Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
![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: 46a790a5cb
ℹ️ 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".
| if (activeTab === RULES_TAB.REQUIRE_FIELDS) { | ||
| for (const ruleKey of filteredSelectedRequireFieldsRuleKeys) { | ||
| deleteRequireFieldsRule(policyData, ruleKey, isVendorMatchingBetaEnabled); | ||
| deleteRequireFieldsRule(policyData, ruleKey, isVendorMatchingBetaEnabled, getReviewWorkspaceSettingsTaskCompletion()); |
There was a problem hiding this comment.
When an outstanding Review your workspace settings task exists and multiple Require fields or Flag for review rules are selected, each loop iteration calls getReviewWorkspaceSettingsTaskCompletion(), whose hook mints a fresh report action ID on every call. Thus each delete carries a separate optimistic task completion; this produces duplicate “marked as complete” actions, and if a later delete fails its failureData can reopen the task after an earlier delete succeeded. Build the payload once for the bulk action and pass {} after the first emitted write, including the flag-for-review loop below.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is valid.
Bug: Bulk deleting Field requirements or Flag for review rules adds one "marked as complete" action per rule
Preconditions: workspace admin, Categories and Rules enabled, "Review your workspace settings" task still open, two categories each with a field requirements rule (e.g. Require description), desktop, offline.
- Workspaces > your workspace > Rules > Field requirements
- Tick both rules
- Click "2 selected" > Delete rules > Delete
- Open the "Review your workspace settings" task from Concierge
Expected: the task is checked with a single "marked as complete" action.
Actual: the task is checked with two "marked as complete" actions. The Flag for review tab behaves the same.
bug.mov
| const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyId}`); | ||
|
|
||
| const policyHasTags = hasTags(policyTags); | ||
| const getReviewWorkspaceSettingsTaskCompletion = useReviewWorkspaceSettingsTaskCompletion(); |
There was a problem hiding this comment.
When a category is renamed from the editable Categories table rather than its edit page, categoryRows.onRenameName still calls renameCategoryInline, and Policy/InlineEdit.ts calls renamePolicyCategory without the new completion argument. The default {} therefore drops both completedTaskReportActionID and the task Onyx updates, so an outstanding Review task does not tick immediately for that reachable rename flow. Thread this getter's payload through the inline helper as well.
Useful? React with 👍 / 👎.
| } else { | ||
| // The command is per-category, so a bulk selection saves one rule for each category picked. | ||
| setPolicyCategoryTaxes(policy, categoriesToMatch, categoryTaxID); | ||
| setPolicyCategoryTaxes(policy, categoriesToMatch, categoryTaxID, getReviewWorkspaceSettingsTaskCompletion()); |
There was a problem hiding this comment.
When an existing category tax rule is edited by changing its category, this block takes the movePolicyCategoryTax branch instead of the newly wired setPolicyCategoryTaxes branch. The move helper emits two SET_POLICY_CATEGORY_TAX writes without either the task action ID or its Onyx data, so an outstanding Review task has no optimistic completion for the move. Accept the payload in movePolicyCategoryTax and attach it only to its first write.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Please check above comments including #104364 (comment) |
|
Addressed |
This branch has not been deployed
3 participants
Explanation of Change
Several category-related write commands that the backend confirms complete the "Review your workspace settings" onboarding task weren't threading the optimistic
completedTaskReportActionIDthrough to their real, reachable UI entry points — some were wired to orphaned legacy pages instead, and others (category tax rate set/delete, create/rename/enable/disable/delete category) weren't wired at all, so the task's checked state only caught up after the next full sync instead of flipping immediately.This PR wires
reviewWorkspaceSettingsTaskDatathrough each backend-confirmed function and its actual reachable entry point, draining the completion payload to{}after the first write whenever a single save fires more than oneAPI.writecall (bulk actions, multi-field rule saves), so only one optimistic completion ever fires per save.Fixed Issues
$ #67648
PROPOSAL:
Tests
Precondition: use an account/workspace where the Concierge "Review your workspace settings" onboarding task is still outstanding (visible unchecked in the LHN/Concierge chat).
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)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
2026-10-19.16-28-19.mp4