Block submission and approval in New Expensify if DEW is enabled on the workspace#73348
Conversation
| title: 'Flujo de aprobación personalizado', | ||
| description: 'Tu empresa tiene un flujo de aprobación personalizado en este espacio de trabajo, por favor realiza esta acción en Expensify Classic', | ||
| goToExpensifyClassic: 'Cambiar a Expensify Classic', | ||
| }, |
| "Selec", | ||
| "setuptools" | ||
| "setuptools", | ||
| "DYNAMICEXTERNAL" |
There was a problem hiding this comment.
Fix spell check issue
|
@abzokhattab 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] |
|
Great job @lorretheboy 🔥, reviewing... |
| /** | ||
| * Checks if policy has Dynamic External Workflow enabled | ||
| */ | ||
| function hasDynamicExternalWorkflow(policy: OnyxEntry<Policy> | SearchPolicy): boolean { |
There was a problem hiding this comment.
Can we add a unit test for this function in the util test file
|
|
||
| const confirmApproval = () => { | ||
| setRequestType(CONST.IOU.REPORT_ACTION_TYPE.APPROVE); | ||
| if (hasDynamicExternalWorkflow(policy)) { |
There was a problem hiding this comment.
I would recommend moving this condition above the setRequestType line... This state update is unnecessary .. what do you think?
There was a problem hiding this comment.
You are correct. I will update accordingly
| if (hasDynamicExternalWorkflow(snapshotPolicy)) { | ||
| onDEWModalOpen?.(); | ||
| return; | ||
| } | ||
| approveMoneyRequestOnSearch(hash, [item.reportID], transactionID, currentSearchKey); | ||
| return; | ||
| case CONST.SEARCH.ACTION_TYPES.SUBMIT: { | ||
| submitMoneyRequestOnSearch(hash, [item], [snapshotPolicy], transactionID, currentSearchKey); | ||
| submitMoneyRequestOnSearch(hash, [item], [snapshotPolicy], transactionID, currentSearchKey, onDEWModalOpen); |
There was a problem hiding this comment.
Just a small improvement here, i would recommend removing the onDEWModalOpen from the submitMoneyRequestOnSearch function and adding the hasDynamicExternalWorkflow condition here, just like how it's done in the approve ... just to keep it consistent with the approve ... what do you think?
There was a problem hiding this comment.
Let's do this in a follow up PR please @lorretheboy
|
Looking at the report (#73348 (comment)), it seems coverage for Ideally, when we modify a component that already has tests, we should update or add tests to cover the new behavior |
|
How we looking guys? Going to spin up a build. |
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
Great almost there, i am taking videos and testing now, i added some comments above for improvement |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Looking good to me on the build! 2025-10-23_23-35-51.mp42025-10-23_23-38-09.mp4 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-10-24.at.00.44.38.movAndroid: mWeb ChromeScreen.Recording.2025-10-24.at.00.41.20.moviOS: HybridAppScreen.Recording.2025-10-24.at.00.27.02.moviOS: mWeb SafariScreen.Recording.2025-10-24.at.00.29.09.movMacOS: Chrome / SafariScreen.Recording.2025-10-24.at.00.17.59.movMacOS: DesktopScreen.Recording.2025-10-24.at.00.45.30.mov |
|
LGTM 🚀 it's working as expected.. i just have some improvement comments above. let us know what you think @lorretheboy |
Block submission and approval in New Expensify if DEW is enabled on the workspace (cherry picked from commit e623fea) (cherry-picked to staging by AndrewGable)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.2.37-6 🚀
|
|
QA'd on staging, works fine! 2025-10-24_02-24-02.mp4 |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.2.37-8 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.2.38-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.2.38-5 🚀
|
Explanation of Change
Fixed Issues
$ #73338
PROPOSAL: #73338 (comment)
Tests
Offline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)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
ANDROID.mov
Android: mWeb Chrome
WEBSITE.ANDROID.mov
iOS: Native
IOS.mov
iOS: mWeb Safari
WEBSITE.IOS.mov
MacOS: Chrome / Safari
WEBSITE.mov
MacOS: Desktop
DESKTOP.mov