-
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
Remove old receipt selector #32764
Remove old receipt selector #32764
Conversation
@johnmlee101 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] |
Testing, but also cc @ntdiary if you want to do the checklist for this PR since I see you're assigned on the parent issue |
@johnmlee101 what's the URL of that page (and also the URL right before you get that)? I'll test that flow out again on my end. |
@johnmlee101, please feel free to take over it, I'm still busy with other issues. 😂 |
If this requires C+ review, I can do on behalf of @ntdiary 🙂 |
@situchan, it sounds like it would be helpful for you to take over the review of this. Thank you! |
@johnmlee101 I was able to reproduce the bug you found. I'll have that fixed here in a few minutes I think |
All right, I found the bug and fixed it so the replace flow should work again. Updated! |
retesting that flow real quick! |
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.
Nice, works 👍
Wow, nice that those were caught. I am guessing they cropped up from some other PR that got merged... Fixed! |
quick! merge before something else conflicts! |
✋ 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/johnmlee101 in version: 1.4.24-4 🚀
|
@tgolen, I think this may have caused this blocker (And likely this one, although I can't reproduce it). I'm investigating what's going on. It is a pretty corner case flow, so I don't think we need to block deploy on it |
yes, they're same bug and main test flow in this PR and we demoted as happening on production (not reliably reproducible) |
Oh, gotcha. I haven't been able to reproduce in prod, but it consistently happens in main/staging. Good to know it is not a blocker, tho |
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.24-8 🚀
|
BUG/REGRESSION @tgolen In this PR, we use Screen.Recording.2024-01-29.at.22.53.51.movIt was caused by we missed updating the route to use Lines 396 to 400 in ef151c3
And we also need to update new route for IOURequestStartPage
cc @situchan |
Details
The old receipt selector is no longer in use since the flows have been refactored. This PR removes the old files and incorporates the current receipt selector in the pages still referencing the old one.
Fixed Issues
This is part of #29107
Tests
manual
money request (need to test this by requesting money from workspaces, existing users, and brand new users)scan
money requestmanual
money request without any receipt imageOffline tests
Same as the above
QA Steps
Same as the above
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.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 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
Android: mWeb Chrome
Unable to test due to broken emulator
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop