[OldDot Rules Migration] Expense report rules#47468
[OldDot Rules Migration] Expense report rules#47468marcaaron merged 80 commits intoExpensify:mainfrom
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
@BrtqKr let's go with |
I think we might be overthinking this one. This feature is mainly used/enabled when a business bank account is connected for reimbursement. In order to even have "auto-reimbursement" you need a bank account and that bank account in many of our customer cases is going to be locked to USD. I would guess that it's highly unusual for someone to put a large value like $20,000 USD for auto-reimbursing reports. So, I think having that error message proposed by @jamesdeanexpensify is fine for now and we shouldn't worry too much about these other currencies. |
marcaaron
left a comment
There was a problem hiding this comment.
Changes look great here. I think we are just waiting for someone to confirm the translations.
|
@BrtqKr Bump on #47468 (comment) and please resolve conflict |
|
@DylanDylann, swapped the condition |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-09-05.at.17.00.22.movAndroid: mWeb ChromeScreen.Recording.2024-09-05.at.17.06.29.moviOS: NativeScreen.Recording.2024-09-05.at.15.25.38.moviOS: mWeb SafariScreen.Recording.2024-09-05.at.15.22.30.movMacOS: Chrome / SafariScreen.Recording.2024-09-05.at.15.26.18.movMacOS: DesktopScreen.Recording.2024-09-05.at.15.21.21.mov |
|
@BrtqKr Lint error |
marcaaron
left a comment
There was a problem hiding this comment.
Great work here! Thanks for the changes 🙇
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
This PR is failing for mWeb and Android because of issue #48894 |
| fieldList: { | ||
| [CONST.POLICY.FIELD_LIST_TITLE_FIELD_ID]: { | ||
| defaultValue: customName, | ||
| pendingFields: {defaultValue: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}, |
There was a problem hiding this comment.
Coming from #61573, we should update optimistic value for fieldList in buildPolicyData
| errorFields: { | ||
| fieldList: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'), | ||
| }, |
There was a problem hiding this comment.
Turns out setting error on the fieldList object like this was setting RBR on the object itself instead of specific fields, which led to this issue:
we fixed this by setting the RBR error specifically on the field matching the error:
errorFields: {
fieldList: {
[CONST.POLICY.FIELDS.FIELD_LIST_TITLE]: {
defaultValue: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),
},
},
},| policyID={policyID} | ||
| accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]} | ||
| featureName={CONST.POLICY.MORE_FEATURES.ARE_RULES_ENABLED} | ||
| shouldBeBlocked={!policy?.shouldShowAutoReimbursementLimitOption || autoPayApprovedReportsUnavailable} |
There was a problem hiding this comment.
The shouldBeBlocked flag should only be used when the feature is enabled. Otherwise, the page will render NotFoundPage and won’t auto-navigate to the Workspace settings page.
More details are available in this proposal: #80703 (comment).
Details
Fixed Issues
$ #47014
PROPOSAL:
Tests
Firstly make sure rules are enabled
Go to the workspaces and select one
Go to the workflows -> check if everything is disabled
Go to the rules -> check if bottom three rules are properly locked are redirecting to the more features section
Enable everything in Workflows
Go to the rules -> verify that every switch is unlocked
Go through each section and verify that editing everything works as expected
A couple of things to note regarding features:
Offline tests
QA Steps
Same as above
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2024-08-23.at.11.39.31.mov
Android: mWeb Chrome
Screen.Recording.2024-08-23.at.11.50.14.mov
iOS: Native
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-08-23.at.11.24.35.mp4
iOS: mWeb Safari
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-08-23.at.11.17.07.mp4
MacOS: Chrome / Safari
Screen.Recording.2024-08-23.at.10.23.45.mov
MacOS: Desktop
Screen.Recording.2024-08-23.at.10.29.43.mov