-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Simplified Collect][Workflows] Create AutoReporting page and commands #37213
Conversation
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.
Left an early review
src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx
Outdated
Show resolved
Hide resolved
@rushatgabhane I think those policies got updated to Daily, so that's expected |
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.
LGTM on my end. I think we just have the offline comment from @rushatgabhane now
Well then it's normal that Daily was shown. It defaults to |
Please see my comment here regarding the offline behavior |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-02-29.at.22.15.54.movAndroid: mWeb ChromeScreen.Recording.2024-02-29.at.21.55.50.moviOS: mWeb SafariScreen.Recording.2024-02-29.at.21.38.03.movMacOS: Chrome / SafariScreen.Recording.2024-02-29.at.21.35.53.movMacOS: Desktop |
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.
@lakchote noticed a crash on iOS and Android
- Go to frequency page
- Select monthly
- Select
Last day of month
Screen.Recording.2024-02-29.at.21.47.23.mov
@rushatgabhane It should be fixed now. We can't use |
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.
@luacmartins LGTM
@lakchote thanks for the quick fix :)
🎯 @rushatgabhane, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #37602. |
✋ 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/luacmartins in version: 1.4.47-0 🚀
|
This PR is failing because of issue #37764 The issue is reproducible in: Web 310172501-a0b3c17e-9462-472f-8a22-8e55ddae43c6.mp4 |
🚀 Deployed to production by https://github.com/roryabraham in version: 1.4.47-10 🚀
|
Hi team, coming from this issue #41326, it appears that this is a missing piece when we implemented this page to be displayed well in offline mode |
Details
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/368332
PROPOSAL:
Tests
Create a new Collect policy on OldDot
Follow the steps outlined here to enable the Collect workspace to show up on NewDot
Login to NewDot, then navigate to Settings > Workspaces. You should see your Collect workspace, click on it.
On the LHP, you should only see the menu items below, specific to paid policies:
Weekly
as the default value.7.Click on it, a RHP should open with the following options:
Monthly
), and verify the response status code is200
andautoReportingFrequency
in your Network browser's tab is congruent:You should also be redirected back to the main workflows screen.
Repeat it for all options (except
Monthly
).Click on
Monthly
, a sub item row should appear withDate of month
and1st
as the default value (if it's a new Collect policy without autoreporting previously set).1st
a new RHP should open, with the following:st
,nd
,rd
,th
), along withLast day of the month
andLast business day of the month
.a. The search should be functional:
b. It should set the appropriate values as the offset, example for a number:
c.
Last day of the month
:d.
Last business day of the month
:Refresh the page, the value you selected (auto reporting frequency - and monthly offset if you chose the
Monthly
one) should be persisted.Verify the ordinal translations for numbers (
1st
,2nd
etc) appear correctly.a. Set your locale as Spanish
b. Go to workflows screen, and select the monthly frequency
c. You should see something like this below
Offline tests
When you change the autoreporting (submission) frequency, it should be optimistically set in Onyx and follow pattern B.
When you look at Onyx data in your browser's inspector, you should see the
pendingFields
actions related to your change:QA Steps
Same as in
Tests
section abovePR 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
iOS: Native
ios.mov
iOS: mWeb Safari
mios.mov
MacOS: Chrome / Safari
autoreporting.mov
MacOS: Desktop
desktop.mov