Fixed money request optimistic violations#94105
Conversation
- Expensify#93932: re-enabling categories now writes the missingCategory violation for existing "Uncategorized" expenses, so the error shows on the preview immediately instead of only after opening the expense. The sentinel is normalized to empty in pushTransactionViolationsOnyxData, scoped to category-requirement updates so unrelated toggles are unaffected. - Expensify#93929: a disabled-but-not-deleted tax rate is now treated as out of policy, so deleting a tag offline no longer drops the "tax no longer valid" violation. getIsViolationFixed is aligned to the same check. - Expensify#93945: multi-level (independent/dependent) tag fields now show the delete-confirmation modal after Tags is disabled, matching single-level tags, instead of opening the Tag RHP.
![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: 294e800be0
ℹ️ 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".
| pushTransactionViolationsOnyxData(onyxData, policyData, {...policyOptimisticData, requiresTag: false}, {}, policyTagsOptimisticData, autoSelections); | ||
| } else { | ||
| pushTransactionViolationsOnyxData(onyxData, policyData, policyOptimisticData); | ||
| const policyTag = PolicyUtils.getTagLists(policyData.tags).at(0); |
There was a problem hiding this comment.
In multi-level tag policies where the Tags feature was disabled, Onyx can contain every tag list with enabled: false. This branch only builds optimistic data for getTagLists(...).at(0), so after toggling the feature back on, especially offline or on a slow connection, only the first level becomes selectable while later levels remain disabled; the recompute below can still leave tagOutOfPolicy violations for those selected levels and dependent tag pickers have no valid child choices until the server response arrives. Build the optimistic and failure updates for every tag list, not just the first one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@lakchote this comment expects to show violation in every tag of dependant tags. And I think it makes sense. But BE sends only first:
"transactionViolations_3030744692880219138": [
{ "name": "tagOutOfPolicy", "data": { "tagName": "State" }, "showInReview": true, "type": "violation" }
]State is orderWeight: 0 — the first level. One violation for the whole multi-level tag, labeled with the first level.
So which is expected? cc @trjExpensify
REC-20260620055314.mp4
|
@lakchote I think we need another help in BE:
When a tax rate is disabled, the response sends only the tax violation for the affected transaction. Since Onyx merges arrays by replacement, this overwrites the transaction's full violation set and drops every other still-valid violation. Repro
Response payload (the bug) { "key": "transactionViolations_<id>", "onyxMethod": "merge",
"value": [{ "name": "taxOutOfPolicy", "type": "violation", "showInReview": true }] }The transaction still has no category, so Expected: return the transaction's full current violation set (e.g. REC-20260621112707.mp4 |
1 participant
Explanation of Change
Fixed Issues
$ #93945
#93929
#93932
PROPOSAL:
Tests
#93945:
Precondition:
• Workspace has independant or dependant tags.
#93929:
• Prerequisite: Account has at least one workspace.
• Prerequisite 2: "Tags" and "Taxes" enabled.
• Prerequisite 3: One tag created and one tax available apart from default.
#93932:
Verify The "Missing category" error should be displayed on the expense preview without opening the expense details.
Verify that no errors appear in the JS console
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
android_app.mp4
Android: mWeb Chrome
android_web.mp4
iOS: Native
IOS_app.mp4
iOS: mWeb Safari
IOS_web.mp4
MacOS: Chrome / Safari
web.mp4