Repository navigation
Show the disabled Submit button in the report preview when self-approval is prevented - #101148
Conversation
…val is prevented canSubmit hid the preview's Submit action from the submitter on a prevent-self-approval workspace, so SubmitActionButton never mounted and its disabled state was unreachable. The report header's isSubmitAction qualifies the same check with !isReportSubmitter and shows a disabled Submit instead. Align the preview with the header. Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
melvin-bot
Bot
added
Melvin-Test-Android
Melvin-Test-Web
|
I created this PR from an upstream branch because I don't have push access to your fork. To take ownership of this branch and push updates: Then you can close this PR and open a new one from your fork. |
|
🤖 Android test steps verification results: ✅ pass PR #101148's disabled-Submit-button fix for self-approval prevention works correctly on Android: both the report preview card Submit button and the report header Submit button become disabled when "Prevent self-approvals" is on and the submitter is the sole approver, and both re-enable when the setting is turned off.
No console errors were observed. Verified on standalone Android NewDot (not the HybridApp shell). view run · no recording available |
|
🤖 Web test steps verification results: ✅ pass PR #101148's fix works as intended on dev NewDot web: with "Prevent self-approvals" enabled and the workspace admin as the sole submitter/approver, both the report preview card's Submit button and the report header's Submit button render disabled with a clear "submitting to yourself" warning. Disabling the setting re-enables both buttons.
No JS console errors were surfaced during the run. Note that the web driver has no dedicated console-log capture, so this reflects what appeared through UI and network error surfaces rather than an exhaustive console dump. view run · no recording available |
|
@MelvinBot Could you please merge main and then replace all |
…dSubmitPreventSelfApproval Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
Done:
The Tests, Offline tests, and QA Steps sections still have their |
|
@thelullabyy 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffc31e0d8f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| // The submitter keeps the SUBMIT action on a prevent-self-approval workspace, matching isSubmitAction in | ||
| // ReportPrimaryActionUtils. SubmitActionButton then mounts and renders itself disabled. Returning VIEW here instead | ||
| // would hide the preview's Submit button entirely while the report header still shows a disabled one. | ||
| it('canSubmit should return true for the submitter when the workspace prevents self-approval', async () => { |
There was a problem hiding this comment.
Add the required // Given, // When, and // Then comments to both new self-approval test cases. The explanatory comment above this case does not provide those three structured steps, and the following non-submitter case has none, so the added tests do not meet the repository's mandatory test-documentation convention.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
@DylanDylann this PR just hit main, how redundant is this now? #101845
|
Ahh yes, looks like my PR also fixes this issue. @thelullabyy can you reproduce #100709 on the latest main now? |
|
Submit button is now gone. We are good to close right? @DylanDylann @trjExpensify Screen.Recording.2026-10-04.at.20.39.52.mov |
|
Mhm, why is it "gone"? Shouldn't it be there, disabled, with nextSteps reading like this video shows: 
|
|
@DylanDylann could you please take a look? |
|
@thelullabyy Something may have changed recently. Could you check the codebase again to find out why? |
|
On my list today |
|
@MelvinBot Could you please merge main and retest this PR. Looks like new changes caused the Submit button gone |
|
@madmax330 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] |
|
@madmax330 Sorry, the PR is not yet ready, I will ping u once it requires your review |
|
🤖 Web test steps verification results: ❌ fail I merged main into this PR (merge commit
Why Submit is missing on main: Main still hides it. Nothing new on main removed it. The preview's After the merge, this PR still returns |
Reviewer Checklist
Screenshots/VideosiOS: HybridAppScreen.Recording.2026-10-08.at.01.12.33.moviOS: mWeb SafariScreen.Recording.2026-10-08.at.01.08.45.movMacOS: Chrome / SafariScreen.Recording.2026-10-08.at.01.07.09.mov |
|
@trjExpensify The submit button is back on staging now, however, I think it doesn't include the fix for this issue #100709 So this PR is still needed Screen.Recording.2026-10-08.at.01.13.41.mov |
|
@madmax330 FYI I approved the PR |
|
Fair enough. 👍 |
|
🚧 madmax330 has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/madmax330 in version: 9.5.6-0 🚀
|
|
No help site update is needed. This bug fix makes the report preview show the same disabled Submit button the report header already shows when Prevent self-approvals is on, and no article describes the preview's Submit button in this case. |
|
Any QA steps here? |
|
@IuliiaHerets Could u please follow the test steps in Test section? |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.6-6 🚀
Bundle Size Analysis (Sentry): |
7 participants
Explanation of Change
The report preview never rendered a Submit button on a "Prevent self-approvals" workspace, so there was nothing to disable. The two Submit surfaces resolved their action differently:
isSubmitActioninReportPrimaryActionUtils.tsblocks SUBMIT for prevented self-approval only when the current user is not the submitter (&& !isReportSubmitter). The submitter still gets SUBMIT, andSubmitPrimaryActionrenders it disabled.canSubmitinReportPreviewActionUtils.tshad the same check without the!isSubmitterqualifier, so it returnedfalsefor the submitter.getReportPreviewActiontherefore never returnedSUBMIT,SubmitActionButtonwas never mounted, and the preview fell through to rendering only the View button.That made the disabled-state gate added by #97946 unreachable for the prevent-self-approval branch — the component it lives in is never mounted. Its unit test mocks
shouldBlockSubmitDueToPreventSelfApproval, so it passed while the real app skipped the component, which is why this only surfaced in QA.This PR adds the
!isSubmitterqualifier tocanSubmit, so the two gates agree: prevented self-approval hides Submit from everyone except the submitter, who gets the action and sees the button rendered disabled. No change toSubmitActionButtonwas needed.Two regression tests were added to
tests/actions/ReportPreviewActionUtilsTest.tscovering the submitter (expectsSUBMIT) and a non-submitter (expects notSUBMIT) on a prevent-self-approval workspace. The first test fails onmainand passes with this change.AI Tests run locally
npm run lint-changednpm run typechecknpm run spell-changednpm test—ReportPreviewActionUtilsTest,ReportPrimaryActionUtilsTest,MoneyRequestReportPreview,ReportPreviewActionButtonTest,SubmitActionButtonTest,useReportPreviewActionDecisionTest,ReportWorkflowTestNot run:
npm run prettier(no such script in this repo — formatting is enforced through ESLint),npm run react-compiler-compliance-check(no.tsxchanged),npm run storybook --smoke-test(no component changed).Browser verification was not possible. The automated web session handed to this run was stuck on a blank page and could not be driven, so the behavior above rests on unit-test and code evidence, not a live repro. The human co-author should confirm the disabled button visually before marking this ready for review.
Fixed Issues
$ #100709
PROPOSAL: #100709 (comment)
Tests
// TODO: The human co-author must fill out the tests they ran before marking this PR as "ready for review".
// Suggested starting point, matching the steps in the linked issue:
// 1. Go to Workspace settings > Workflows and enable "Prevent self-approvals".
// 2. Go to the workspace chat and create an expense.
// 3. Verify the report preview card in the chat shows a Submit button that is disabled, next to the View button.
// 4. Open the expense report and verify the header Submit button is disabled too, matching the preview.
// 5. Turn "Prevent self-approvals" off and verify both Submit buttons are enabled again.
Offline tests
// TODO: The human co-author must fill this out before marking this PR as "ready for review".
QA Steps
// TODO: The human co-author must fill out the QA tests before marking this PR as "ready for review".
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari