-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
"Waiting for you to pay these expenses" is briefly displayed for approver #38244
Conversation
@fedirjh , Having some issue creating a non-reimburser approver on OD, do you have any steps documented to create one? I will try again to create from my side too |
cc @GandalfGwaihir Are you able to create a collect policy ? For creating a collect check, the QA steps in this PR: #34127
First, you have to create collect policy : Choose Control policy plan Set Approval Mode as advanced Finally add new member as an approver |
cc @GandalfGwaihir Were you able to set up a non-reimburser approver on OD ? |
yes I was!! thanks for the help, will update the videos by EOD today :) @fedirjh |
@fedirjh , I was finally able to reproduce after a lot of efforts 🥳, thanks for the initial heads up , can you review this PR please? Also, there are some other configurations too while setting up, so if you find any problem ping me up here, i will try to help |
@GandalfGwaihir Could you please add testing steps ? |
My bad, sorry, have updated the tests :) |
Reviewer Checklist
Screenshots/VideosAndroid: NativeCleanShot.2024-03-18.at.22.30.22.mp4Android: mWeb ChromeCleanShot.2024-03-18.at.22.33.28.mp4iOS: NativeSimulator.Screen.Recording.-.iPhone.Xs.-.2024-03-17.at.15.47.38.mp4iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.Xs.-.2024-03-18.at.22.58.08.mp4MacOS: Chrome / SafariCleanShot.2024-03-18.at.22.06.02.mp4MacOS: DesktopCleanShot.2024-03-18.at.22.51.20.mp4 |
We did not find an internal engineer to review this PR, trying to assign a random engineer to #37866 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
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.
Looks good to me, asking @Gonals to take a look too since he's now assigned to the issue
@Gonals looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Tests are fine |
🚀 Deployed to staging by https://github.com/Gonals in version: 1.4.56-0 🚀
|
As i can see in the video this is a self This is what is intended actually Also in the video you have set the setting to submit and approve, so any report approved would not have to wait for reimbursement:) Also, there is no approver in your policy we only have the admin: |
🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.4.56-8 🚀
|
Details
Wrong optimistic message is being displayed when an approver approves messages, this PR fixes that issue
Fixed Issues
$ #37866
PROPOSAL: #37866 (comment)
Tests
Same as QA
Offline tests
Cannot be reproduced offline as you need the employee to send request to the approver
QA Steps
Precondition:
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
iOS: mWeb Safari
WhatsApp.Video.2024-03-16.at.5.53.40.AM.mp4
MacOS: Desktop
issue37866-MacOS.mp4
Android: Native
issue37866-Android.mp4
Android: mWeb Chrome
issue37866-AndroidWeb.mp4
iOS: Native
WhatsApp.Video.2024-03-16.at.5.57.27.AM.mp4
MacOS: Chrome / Safari
issue37866-MacOSWeb.mp4