fix: Prevent self approval still display in expense detail after set approver for workspace by nkdengineer · Pull Request #84700 · Expensify/App

nkdengineer · 2026-03-10T08:57:26Z

Explanation of Change

fix: Prevent self approval still display in expense detail after set approver for workspace

Fixed Issues

$ #80255
PROPOSAL: #80255 (comment)

Tests

Precondition: Create a control workspace, at workflows set Submission manually, enable Prevent self-approvals

At control workspace chat create and submit a manual expense
Navigate to this control workspace setting
Click on Members tab invite an admin
Click on Workflows tab and set admin is approver
Return to Inbox and observe expense report created at step 2
Click on expense report for go to detail page
Verify that the submit button is enabled and the prevent self approval message is not displayed

Verify that no errors appear in the JS console

Offline tests

QA Steps

Same as test

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2026-03-10.at.15.52.37.mov

Android: mWeb Chrome

Screen.Recording.2026-03-10.at.15.52.02.mov

iOS: Native

Screen.Recording.2026-03-10.at.15.51.10.mov

iOS: mWeb Safari

Screen.Recording.2026-03-10.at.15.50.43.mov

MacOS: Chrome / Safari

Screen.Recording.2026-03-10.at.15.48.40.mp4

…approver for workspace

melvin-bot · 2026-03-10T09:10:34Z

@brunovjk 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]

codecov · 2026-03-10T09:13:56Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/libs/NextStepUtils.ts	`72.18% <100.00%> (+12.85%)`	⬆️
src/libs/ReportUtils.ts	`82.51% <100.00%> (+<0.01%)`	⬆️
...yReportHeaderPrimaryAction/SubmitPrimaryAction.tsx	`0.00% <0.00%> (ø)`
... and 156 files with indirect coverage changes

brunovjk · 2026-03-10T14:03:59Z

Great work @nkdengineer, however initially the message "Oops!..." appears briefly and then disappears:

Full test

Screen.Recording.2026-03-10.at.10.56.25.mov

Just the flickering part.

qwer.mov

Do you think this is related to the changes in BE that you mentioned here? Thanks.

nkdengineer · 2026-03-10T14:25:12Z

Do you think this is related to the changes in BE that you mentioned here? Thanks.

@brunovjk Yes, this is the backend problem I mentioned.

joekaufmanexpensify

Good for product

brunovjk · 2026-03-12T20:40:54Z

@luacmartins can you help me check and fix the issue mentioned by nkdengineer here? Thank you very much.

luacmartins · 2026-03-23T16:18:39Z

@nkdengineer from the video, it seems like the BE has the correct value for the nextStep since the correct value is shown after the report loads. Are we sure this is an issue with the BE and we're not just a missing optimistic update in App?

nkdengineer · 2026-03-24T07:47:15Z

@nkdengineer from the video, it seems like the BE has the correct value for the nextStep since the correct value is shown after the report loads. Are we sure this is an issue with the BE and we're not just a missing optimistic update in App?

@luacmartins

Great work @nkdengineer, however initially the message "Oops!..." appears briefly and then disappears:

When we disable Prevent self-approval rule, the next step is still kept as the prevent self-approval next step (the next step from backend), which causes this problem.
Since we already have the logic to display the Prevent self-approval next step which overrides the current next step when it's needed in the App, can we fix it from the backend so that it does not return the Prevent self-approval as the next step

luacmartins · 2026-03-24T20:25:33Z

@nkdengineer I checked the response of UpdateWorkspaceApproval and we don't return the reportNextStep and I don't think we should. Can we just optimistically set it when we change the approval mode?

nkdengineer · 2026-03-25T04:18:06Z

Since we already have the logic to display the Prevent self-approval next step which overrides the current next step when it's needed in the App, can we fix it from the backend so that it does not return the Prevent self-approval as the next step

@luacmartins I mean here is the OpenReport API.

nkdengineer · 2026-03-25T04:19:14Z

Can we just optimistically set it when we change the approval mode?

Also, I don't think it's a good idea because we need to go through all related expense reports and calculate the next step.

luacmartins · 2026-03-25T18:10:13Z

Well, yes but that'd ensure the functionality is correct offline, right?

nkdengineer · 2026-03-25T18:11:50Z

Yes.

nkdengineer · 2026-04-07T16:54:47Z

We should be doing this to optimistically update these, right?

The correct next-step message appears only after a short delay, which feels more like a FE/state sync or optimistic update issue.

If it's expected that the backend will change the report's next step when enabling/disabling prevent self-approval, we need to do that in setPolicyPreventSelfApproval function to prevent this issue.

luacmartins · 2026-04-10T16:11:21Z

@nkdengineer that seems correct

nkdengineer · 2026-04-13T14:21:57Z

The correct next-step message appears only after a short delay, which feels more like a FE/state sync or optimistic update issue.

Fixed this issue

The Submit button is still disabled even after the message changes to Waiting for you to submit the expense.

@brunovjk Can't reproduce

luacmartins · 2026-04-14T17:04:52Z

@nkdengineer is this ready for review again?

nkdengineer · 2026-04-14T17:05:46Z

Yes it's ready

luacmartins · 2026-04-14T17:08:31Z

@brunovjk could you please review?

brunovjk · 2026-04-15T13:04:58Z

@nkdengineer I can still reproduce the bug where the old message briefly appears, but only for new users, after the message "disappears" for the first time, the bug is no longer reproducible. The "Submit" button was also disabled within the expense preview. The bug where the create expense modal gets stuck is not related to this PR; the bug is also reproducible in main.

Screen.Recording.2026-04-15.at.09.55.44.mov

Can you reproduce this? Thank you.

nkdengineer · 2026-04-16T06:49:52Z

@luacmartins @brunovjk Again, we have another case here where the Prevent self-approval next step is returned from backend, and changing the approver doesn't change this in optimistic data. I still think it's a simple fix if we remove Prevent self-approval next step calculation from the backend and only handle this next step in the front-end, like a strict policy rule violation. Then we don't need to handle the optimistic data in many cases, like disable prevent-self approval, changing approvers in workflow, changing report approver

luacmartins · 2026-04-17T18:22:37Z

@dangrous tagging you since I believe you worked on the next step changes. What do you think of the comment above?

dangrous · 2026-04-21T18:14:22Z

I think for the moment at least we will still need to calculate that in the backend, since we expect it in Old Dot.

I don't believe we currently recalculate any next steps - or send Onyx updates - based on policy changes, because it would require us to check all reports on the policy. This will eventually be a feature of the new next steps once they're fully migrated into Auth, but we're not there yet. Right now they're only recalculated when a report is newly (re)opened

I guess we could potentially calculate the prevent-self-approval on BOTH front ends, if that would clean up something in App, but not sure if that's necessarily worth the effort?

luacmartins · 2026-04-24T21:47:20Z

@nkdengineer did you get a chance to review the comment above?

nkdengineer · 2026-04-28T02:34:28Z

I don't believe we currently recalculate any next steps - or send Onyx updates - based on policy changes, because it would require us to check all reports on the policy

@luacmartins Based on this, since we don't recalculate the next step on the policy changes and it's calculated in the read time, can we skip this issue instead of trying to update the optimistic next step of all reports on the policy changes since it's waiting for the update from the backend site, it's not something wrong in the frontend

I guess we could potentially calculate the prevent-self-approval on BOTH front ends, if that would clean up something in App, but not sure if that's necessarily worth the effort?

We already calculated the prevent-self approval in the front-end here.

App/src/libs/NextStepUtils.ts

Lines 357 to 390 in 666d9c0

    
           function getReportNextStep( 
        
               currentNextStep: ReportNextStepDeprecated | undefined, 
        
               moneyRequestReport: OnyxEntry<Report>, 
        
               transactions: Array<OnyxEntry<Transaction>>, 
        
               policy: OnyxEntry<Policy>, 
        
               transactionViolations: OnyxCollection<TransactionViolations>, 
        
               currentUserEmail: string, 
        
               currentUserAccountID: number, 
        
           ) { 
        
               const nextApproverAccountID = getNextApproverAccountID(moneyRequestReport); 
        
               if ( 
        
                   isOpenExpenseReport(moneyRequestReport) && 
        
                   transactions.length > 0 && 
        
                   transactions.every( 
        
                       (transaction) => !!transaction && hasSubmissionBlockingViolations(transaction, transactionViolations, currentUserEmail, currentUserAccountID, moneyRequestReport, policy), 
        
                   ) 
        
               ) { 
        
                   // eslint-disable-next-line rulesdir/no-default-id-values -- actorAccountID can be -1 for unspecified owner 
        
                   return buildOptimisticFixIssueNextStep(moneyRequestReport?.ownerAccountID ?? -1); 
        
               } 
        
               const isSubmitterSameAsNextApprover = 
        
                   isReportOwner(moneyRequestReport) && (nextApproverAccountID === moneyRequestReport?.ownerAccountID || moneyRequestReport?.managerID === moneyRequestReport?.ownerAccountID); 
        
               // When prevent self-approval is enabled & the current user is submitter AND they're submitting to themselves, we need to show the optimistic next step 
        
               // We should always show this optimistic message for policies with preventSelfApproval 
        
               // to avoid any flicker during transitions between online/offline states 
        
               if (isSubmitterSameAsNextApprover && policy?.preventSelfApproval) { 
        
                   return buildOptimisticNextStepForPreventSelfApprovalsEnabled(); 
        
               } 
        
               return currentNextStep; 
        
           }

luacmartins · 2026-04-28T15:12:59Z

@nkdengineer yea that seems fine

nkdengineer · 2026-04-29T07:35:39Z

@brunovjk Let skip this one and review again.

brunovjk · 2026-04-29T12:47:10Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

84700_android_web.mov

Android: mWeb Chrome

84700_android_web.mov

iOS: HybridApp

84700_ios_native.mov

iOS: mWeb Safari

84700_ios_web.mov

MacOS: Chrome / Safari

84700_web_chrome.mov

brunovjk

LGMT. Thanks for the hard work here @nkdengineer and @luacmartins.

github-actions · 2026-04-29T14:21:10Z

🚧 @luacmartins has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-04-29T14:25:58Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84700.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-c99f4df-7486324-39044c4e210d5c43ae5a494411b8747e654c9099/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-c99f4df-7486324-c9f8cada02b21c89cb6e392b404bd838185a9320/index.html

Web 🕸️
https://84700.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-04-29T14:27:29Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2026-04-30T23:04:49Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.3.65-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-04-30T23:07:44Z

I reviewed the changes in this PR against the help site files under App/docs/articles.

No help site changes are required.

This PR is a bug fix that corrects when the "prevent self-approval" block is applied — it now properly considers report state (open vs processing) and unblocks submission when a different approver has been set. The existing docs already describe the feature correctly at the right level of abstraction:

Workspace-Rules.md — "Prevent self-approvals – Block users from approving their own reports."
Approve-Expenses.md — "If Prevent Self-Approval is enabled, an admin cannot bypass approvals to approve their own report."
Expense-and-Report-Actions.md — Same description as above.

The fix makes the code match the already-documented behavior, so no documentation updates are needed.

OSBotify · 2026-05-04T13:25:23Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.3.65-6 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

melvin-bot Bot assigned nkdengineer Mar 10, 2026

fix: Prevent self approval still display in expense detail after set …

d32fe61

…approver for workspace

nkdengineer force-pushed the fix/80255 branch from 7e5ba49 to d32fe61 Compare March 10, 2026 09:05

nkdengineer marked this pull request as ready for review March 10, 2026 09:10

nkdengineer requested review from a team as code owners March 10, 2026 09:10

melvin-bot Bot requested review from brunovjk and removed request for a team March 10, 2026 09:10

melvin-bot Bot requested review from joekaufmanexpensify and removed request for a team March 10, 2026 09:10

github-actions Bot reviewed Mar 10, 2026

View reviewed changes

Comment thread src/libs/ReportUtils.ts

joekaufmanexpensify previously approved these changes Mar 10, 2026

View reviewed changes

nkdengineer added 2 commits March 13, 2026 12:24

Merge branch 'main' into fix/80255

bc8f0e8

improve peft and add unit test

874f56b

nkdengineer dismissed joekaufmanexpensify’s stale review via 874f56b March 13, 2026 06:24

laurenreidexpensify mentioned this pull request Mar 23, 2026

[Due for payment 2026-05-11] [$250] Expense - Prevent self approval still display in expense detail after set approver for workspace #80255

Closed

7 tasks

luacmartins self-requested a review March 23, 2026 16:11

resolve conflict

c78e1d9

resolve conflict

56605f4

merge main

fae8558

nkdengineer force-pushed the fix/80255 branch from 972055f to fae8558 Compare April 29, 2026 07:31

update submit primary action

9424642

fix ts check

cbe28fa

brunovjk approved these changes Apr 29, 2026

View reviewed changes

luacmartins approved these changes Apr 29, 2026

View reviewed changes

luacmartins merged commit c99f4df into Expensify:main Apr 29, 2026
38 checks passed

OSBotify mentioned this pull request Apr 30, 2026

Deploy Checklist: New Expensify 2026-04-30 #89315

Closed

Conversation

nkdengineer commented Mar 10, 2026

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Mar 10, 2026

Uh oh!

codecov Bot commented Mar 10, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

brunovjk commented Mar 10, 2026

Uh oh!

nkdengineer commented Mar 10, 2026

Uh oh!

joekaufmanexpensify left a comment

Choose a reason for hiding this comment

Uh oh!

brunovjk commented Mar 12, 2026

Uh oh!

luacmartins commented Mar 23, 2026

Uh oh!

nkdengineer commented Mar 24, 2026

Uh oh!

luacmartins commented Mar 24, 2026

Uh oh!

nkdengineer commented Mar 25, 2026

Uh oh!

nkdengineer commented Mar 25, 2026

Uh oh!

luacmartins commented Mar 25, 2026

Uh oh!

nkdengineer commented Mar 25, 2026

Uh oh!

nkdengineer commented Apr 7, 2026

Uh oh!

luacmartins commented Apr 10, 2026

Uh oh!

nkdengineer commented Apr 13, 2026

Uh oh!

luacmartins commented Apr 14, 2026

Uh oh!

nkdengineer commented Apr 14, 2026

Uh oh!

luacmartins commented Apr 14, 2026

Uh oh!

brunovjk commented Apr 15, 2026

Uh oh!

nkdengineer commented Apr 16, 2026

Uh oh!

luacmartins commented Apr 17, 2026

Uh oh!

dangrous commented Apr 21, 2026

Uh oh!

luacmartins commented Apr 24, 2026

Uh oh!

nkdengineer commented Apr 28, 2026

Uh oh!

luacmartins commented Apr 28, 2026

Uh oh!

nkdengineer commented Apr 29, 2026

Uh oh!

brunovjk commented Apr 29, 2026

Reviewer Checklist

Screenshots/Videos

Uh oh!

brunovjk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

codecov Bot commented Mar 10, 2026 •

edited by github-actions Bot

Loading