-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feat] Make it possible to submit expenses to any workspace, whether or not there is a workspace chat #49412
Conversation
not overdue |
We need to update the RequestMoney API command to pass the policyID. That way if we're using an optimistic policyExpenseChat, we know which policy to create the chat on. |
@eVoloshchak Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@eVoloshchak This is ready for review code wise, but BE is not deployed yet. I'll write test steps. but for now it can be tested only offline for now |
FYI BE PR is deployed on staging |
For test 2 it needs to be a Control workspace. Collect workspaces created on OldDot are workspace chat enabled. |
Updated! |
Reviewer Checklist
Screenshots/VideosAndroid: NativeUnable to test on Android native because Flipper doesn't allow me to run herms debugger on my android device. Android: mWeb ChromeScreen.Recording.2024-10-21.at.3.46.00.AM.moviOS: NativeScreen.Recording.2024-10-21.at.3.41.15.AM.moviOS: mWeb SafariScreen.Recording.2024-10-21.at.3.35.27.AM.movMacOS: Chrome / SafariScreen.Recording.2024-10-21.at.3.25.49.AM.movMacOS: DesktopScreen.Recording.2024-10-21.at.3.31.39.AM.mov |
BUGThe onboarding model re-appears after refreshing the page. Screen.Recording.2024-10-18.at.3.54.13.AM.mov |
BUGThe created expense report switches its position in the LHN thrice. Screen.Recording.2024-10-18.at.3.55.07.AM.mov |
I am not able to reproduce, is this happening consistently? |
@ishpaul777 I was able to consistently reproduce it with new accounts. Maybe merge main and try? |
Still cant reproduce Screen.Recording.2024-10-18.at.8.29.22.PM.mov@allroundexperts Have you checked if this is still repro on main? |
On Android mwb, Screen.Recording.2024-10-21.at.3.46.00.AM.mov |
Upon clearing my onyx data, I was not able to reproduce #49412 (comment) as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One potential bug report, but I think its related to another bug that we'll be handling from the backend. As such, I am approving it.
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.52-0 🚀
|
New feature, so not reverting |
hi! I'm VERY not confident about this - but is there a chance that this caused #51256 ? I am having trouble reproducing it consistently on my local (it occurred once, and never again) but it does happen on the actual staging site. This PR looks like it modified tags in a way that MIGHT have caused it? |
THis PR caused this DeployBlocker: #51249 |
I'm proposing we revert the PR: https://expensify.slack.com/archives/C01GTK53T8Q/p1729612151981119 |
Decided to revert this PR to fix this deploy blocker: #51286 |
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.52-5 🚀
|
Details
Fixed Issues
$ #49344
PROPOSAL: N/A
Tests
Offline tests
QA Steps
Test 1
report_<reportid>
.Test 2
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Record_2024-10-18-03-41-41.mp4
Android: mWeb Chrome
iOS: Native
Screen.Recording.2024-10-18.at.3.19.32.AM.mov
iOS: mWeb Safari
Screen.Recording.2024-10-18.at.2.52.34.AM.mov
MacOS: Chrome / Safari
Screen.Recording.2024-10-18.at.2.12.46.AM.mov
Screen.Recording.2024-10-18.at.2.34.01.AM.mov
MacOS: Desktop
Screen.Recording.2024-10-18.at.3.55.50.AM.mov