Conversation
…a single updates object for employee properties
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
situchan
left a comment
There was a problem hiding this comment.
Optimistic data is not enough. Still issue after logout & re-login. We need backend fix as well. Otherwise find different solution.
|
@situchan This change only addresses the frontend side. We’ll still need a backend fix as well |
|
@francoisl should we hold frontend PR until backend is done? |
|
Yeah let's hold on a second. I'll try to finish the backend portion this week. |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Nice product polish
|
Hey team, sorry for the delay here. The backend changes are merged but we've had a deploy freeze that's been delaying things a little. In the meantime, can we update the PR and change the I'll let you know once the command is deployed (hopefully tomorrow). |
|
Ok the API command |
|
The test failed because the action exceeded the timeout |
|
@situchan The PR is ready for review 👍 |
|
I reran the failed test but it failed again, can you pull |
|
@cretadn22 I think the failing test is caused by this PR. Can you check? |
|
@situchan I updated tests |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Still good with these changes.
|
Tests are passing now, @situchan can you review and fill out the checklist please? |
|
|
||
| expect(apiWriteSpy).toHaveBeenCalledWith( | ||
| WRITE_COMMANDS.SET_WORKSPACE_APPROVAL_MODE, | ||
| WRITE_COMMANDS.DISABLE_POLICY_APPROVALS, |
There was a problem hiding this comment.
Please also add test case of SET_WORKSPACE_APPROVAL_MODE
There was a problem hiding this comment.
We already have test cases covering both SET_WORKSPACE_APPROVAL_MODE and DISABLE_POLICY_APPROVALS. In this test, the goal is to verify the behavior of the next step when it’s changed to optional, so we need to adjust the corresponding API call accordingly
There was a problem hiding this comment.
I mean the case where WRITE_COMMANDS.SET_WORKSPACE_APPROVAL_MODE is expected to be called. (approvalMode is not optional)
After this PR, only DISABLE_POLICY_APPROVALS case is added (as it's replaced from SET_WORKSPACE_APPROVAL_MODE)
There was a problem hiding this comment.
Could you please check again the code change? I’ve added test cases to ensure the SET_WORKSPACE_APPROVAL_MODE call is properly called
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
| Policy.setWorkspaceApprovalMode(policyID, ESH_EMAIL, CONST.POLICY.APPROVAL_MODE.BASIC); | ||
| await waitForBatchedUpdates(); | ||
|
|
||
| expect(apiWriteSpy).toHaveBeenCalledWith(WRITE_COMMANDS.SET_WORKSPACE_APPROVAL_MODE, expect.objectContaining({policyID}), expect.anything()); |
There was a problem hiding this comment.
Ah I see this now. Thanks
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @francoisl has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/francoisl in version: 9.3.21-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.21-4 🚀
|
Explanation of Change
Fixed Issues
$ #72109
PROPOSAL: #72109 (comment)
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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))npm run compress-svg)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.2026-01-22.at.19.22.28.mov
Android: mWeb Chrome
Screen.Recording.2026-01-22.at.19.20.47.mov
iOS: Native
Screen.Recording.2026-01-22.at.19.21.16.mov
iOS: mWeb Safari
Screen.Recording.2026-01-22.at.19.19.51.mov
MacOS: Chrome / Safari
Screen.Recording.2026-01-22.at.19.18.46.mov