-
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
[Drafts] Show the GBR optimistically to the submitter when Scheduled submit is turned off #33030
Conversation
At the moment needs to be tested with this Web change https://github.com/Expensify/Web-Expensify/pull/40033 |
I will follow up on this tomorrow when travelling, prioritized other issues |
@cubuspl42 @ One of you needs to 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] |
@puneetlath This is ready for a review |
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. Just a couple minor comments. Also, didn't have a chance to test it.
I have verified this behaviour, and is working correctly |
Seems like the test is just running too long now
|
Nope that is not implemented yet @shubham1206agra with the first issue, I think thats fine that seems to be related just to the pusher updates and maybe it was not processed in time or something, right? |
Screen.Recording.2023-12-31.at.5.48.28.PM.movI think pusher is giving updates without refresh |
Oh you are suggesting the you want the colour of the button to change when the scheduled submit property is changed by the pusher? |
I am responding to this
It seems like offline request doesn't seem to pick up right value. |
I have added useMemo, can you retest? I am having some issues with pusher locally, it does not wasnt to authenticate |
Yeah, seems fixed now. Looks like pusher was the culprit after all. |
Reviewer Checklist
Screenshots/VideosScheduled submit onScreen.Recording.2023-12-30.at.7.07.22.PM.movScheduled submit offScreen.Recording.2023-12-30.at.7.12.53.PM.movOptimistic GBR when scheduled submit if offScreen.Recording.2023-12-30.at.7.15.26.PM.movOptimistic GBR when scheduled submit if offScreen.Recording.2023-12-30.at.7.48.21.PM.mov |
@Julesssss 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] |
✋ 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: 1.4.21-0 🚀
|
🚀 Deployed to production by https://github.com/marcaaron in version: 1.4.21-4 🚀
|
1 similar comment
🚀 Deployed to production by https://github.com/marcaaron in version: 1.4.21-4 🚀
|
🚀 Deployed to production by https://github.com/marcaaron in version: 1.4.21-4 🚀
|
Details
When scheduled submit is turned off, then the GBR should show for the submitter in the LHN on the policy expense chat. They need to manually submit. Whether the scheduled submit is on or off is told by the
isHarvestingEnabled
policy property.Secondly this change is also fixing the case where the approver force submits the report and the GBR does not show up for them optimistically. When they submit the report, they are the next person in the approval chain so they should have GBR in LHN.
In this PR, we are also updating the styles of the submit button based on the isHarvestingEnabled property. When the scheduled submit is turned on, the submit button should not show active green for the submitter as the report will autosubmit
Fixed Issues
$ #32920
$ #33443
PROPOSAL:
Tests
isHarvestingEnabled
property set to trueisHarvestingEnabled
is now set to falseOffline tests
N/A
QA Steps
Same as tests
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
Scheduled submit on:
scheduledSubmitOn.mp4
Scheduled submit off
scheduledSubmitOff.mp4
Optimistic GBR when scheduled submit if off
Screen.Recording.2023-12-29.at.14.53.36.mp4
Optimistic GBR when scheduled submit if on - no GBR in this case
Screen.Recording.2023-12-29.at.14.55.25.mp4
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop