Repository navigation
Add workspace setting to control marking non-reimbursable reports as paid - #100569
stitesExpensify merged 27 commits into
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@FitseTLT This cannot be tested yet (BE PRs not deployed yet) but you can start reviewing code |
|
Joe owns the parent issue, switching the product review. |
|
I will test this once the backend is out 👍 |
|
Backend still in review, but we seem to be getting closer. |
|
@s77rt this is off hold now, correct? nothing remains from the backend? |
| ((nonReimbursableSpend !== 0 || hasSettledZeroReimbursableSpend(spendBreakdown, report, transactions)) && | ||
| !(policy?.preventPayoutNonReimbursableReports && hasOnlyNonReimbursableTransactions(report?.reportID, transactions)))) |
There was a problem hiding this comment.
I don't think this condition is correct it is mixing up our rule to hasSettledZeroReimbursableSpend. I feel like I got your intention you don't want hasSettledZeroReimbursableSpend unintentionally enabling our case but from the intention of their pr I think they want to target reimbursable expenses cumulating to zero. So we should apply our rule for the all non-reimbursable case and then apply !hasOnlyNonReimbursableTransactions condition beside hasSettledZeroReimbursableSpend.
There was a problem hiding this comment.
I'm not sure I got what you mean. The new condition does keep the intention of the original PR and extend to support this one.
- If a report has any expenses that as whole cancel each other, also show the pay button
- If a report has non reimbursable expenses then show the pay button
However if that report only has non-reimbursable expenses and the policy settings prevent paying those, then do not show the pay button
Am I missing some case?
There was a problem hiding this comment.
Hmmm. I am not sure how practical it is because now hasSettledZeroReimbursableSpend case is also intervening with the all non-reimbursable case but there are at least two problems.
- non-zero
nonReimbursableSpendvalue was supposed to only allow pay only for the case of all non-reimbursable, now there is no logic limiting that. - A zero all non-reimbursable report is also allowed to be paid which also happens on main of course but I don't think is correct it is unintentionally slipping through hasSettledZeroReimbursableSpend.
There was a problem hiding this comment.
non-zero nonReimbursableSpend value was supposed to only allow pay only for the case of all non-reimbursable, now there is no logic limiting that.
This was pointless because if a report has nonReimbursableSpend > 0 but it also contain reimbursable expenses, then the pay button will show up regardless due to either reimbursableSpend > 0 or the new hasSettledZeroReimbursableSpend
A zero all non-reimbursable report is also allowed to be paid which also happens on main of course but I don't think is correct it is unintentionally slipping through hasSettledZeroReimbursableSpend.
it can be marked as paid still, the only thing preventing this is this new preventPayoutNonReimbursableReports flag
|
bump @s77rt we are close! |
|
Addressed comments |
|
I will review this tomorrow |
|
🚧 stitesExpensify 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/stitesExpensify in version: 9.5.14-0 🚀
|
|
A help site update is needed. I opened a draft docs PR: Docs updates for E/A#100569 Add Prevent marking non-reimbursable reports as paid setting. This PR adds a new Prevent marking non-reimbursable reports as paid toggle to Workflows > Advanced, and it isn't behind a beta. This sentence in Workspace Rules lists every report-level rule and leaves out the new one:
The docs PR makes two small edits:
I checked the label and its position against the live web UI. @s77rt, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Deploy Blocker #104388 was identified to be related to this PR. |
|
Deploy Blocker #104395 was identified to be related to this PR. |
|
Deploy Blocker #104409 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.14-3 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Added new policy rule
preventPayoutNonReimbursableReports, when enabled, non-reimbursable reports cannot be marked as paid.Fixed Issues
$ #100427
PROPOSAL:
Tests
Prevent marking non-reimbursable reports as paidtoggleViewScreen.Recording.2026-10-02.at.9.35.59.AM.mov
Offline tests
Same as Tests
QA Steps
Same as Tests
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