Part 1 - Fix review workspace settings onboarding task completion#95766
Part 1 - Fix review workspace settings onboarding task completion#95766FitseTLT wants to merge 4 commits into
Conversation
|
@jayeshmangwani 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] |
FitseTLT
changed the title
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| function WorkspaceNamePage({policy}: Props) { | ||
| const styles = useThemeStyles(); | ||
| const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This exact three-line boilerplate — useCurrentUserPersonalDetails(), useOnboardingTaskInformation(CONST.ONBOARDING_TASK_TYPE.REVIEW_WORKSPACE_SETTINGS), and then getReviewWorkspaceSettingsTaskCompletionData(reviewWorkspaceSettingsTaskInformation, currentUserAccountID) — is duplicated verbatim across 12 page components (WorkspaceNamePage, WorkspaceOverviewCurrencyPage, WorkspaceOverviewDescriptionPage, ReportsDefaultTitle, WorkspaceReportsPage, RulesBillableDefaultPage, RulesMaxExpenseAgePage, RulesMaxExpenseAmountPage, RulesReceiptRequiredAmountPage, RulesRequireReceiptsPage, WorkspaceUpgradePage, WorkspaceAutoReportingFrequencyPage, WorkspaceAutoReportingMonthlyOffsetPage). Every consumer must repeat the same wiring, and any future change to how the task-completion data is resolved has to be made in 13 places.
Extract the shared wiring into a single hook and return the ready-to-pass onyxData, so each page calls one hook:
// src/hooks/useReviewWorkspaceSettingsTaskCompletionData.ts
function useReviewWorkspaceSettingsTaskCompletionData() {
const {accountID} = useCurrentUserPersonalDetails();
const taskInformation = useOnboardingTaskInformation(CONST.ONBOARDING_TASK_TYPE.REVIEW_WORKSPACE_SETTINGS);
return getReviewWorkspaceSettingsTaskCompletionData(taskInformation, accountID);
}
// in each page:
const reviewWorkspaceSettingsTaskData = useReviewWorkspaceSettingsTaskCompletionData();
...
updateGeneralSettings(policy, values.name.trim(), policy.outputCurrency, reviewWorkspaceSettingsTaskData);Reviewed at: 56ce09d | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![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: 56ce09dba7
ℹ️ 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".
| hasOutstandingChildTask, | ||
| parentReportAction, | ||
| undefined, | ||
| false, |
There was a problem hiding this comment.
When a workspace-settings command succeeds for a user with this onboarding task, this path creates a local TASK_COMPLETED report action but passes shouldSendCompleteTaskRequest=false, so the completedTaskReportActionID from buildTaskData() is never sent in either CompleteTask or the settings command params. Because the backend side-effect completion that this change relies on cannot know that optimistic action ID, its returned Onyx update will use a different action while this request's successData only clears the local pending action, leaving duplicate “marked as complete” actions for the task. Please either send the optimistic action ID with the side-effect command or avoid creating a client action that the server cannot reconcile.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yeah @dukenv0307 I remebered this was the BE change we haven't fixed. Currently optimisitc data creates completed actions but the report action id is not communicated with the BE command for our cases here so duplicate actions are created. We will need to send the reportActionIDs from FE let me know what I should do from FE.
Full list of workspace-settings APIs (58)
POLICYCHANGELOG_UPDATE_NAME
POLICYCHANGELOG_UPDATE_DESCRIPTION
POLICYCHANGELOG_UPDATE_CURRENCY
POLICYCHANGELOG_UPDATE_FIELD
POLICYCHANGELOG_ADD_CATEGORY
POLICYCHANGELOG_UPDATE_CATEGORIES
POLICYCHANGELOG_UPDATE_CATEGORY
POLICYCHANGELOG_DELETE_CATEGORY
POLICYCHANGELOG_DELETE_CATEGORIES
POLICYCHANGELOG_SET_CATEGORY_NAME
POLICYCHANGELOG_REPLACE_CATEGORIES
POLICYCHANGELOG_ADD_TAX
POLICYCHANGELOG_UPDATE_TAX
POLICYCHANGELOG_DELETE_TAX
POLICYCHANGELOG_ADD_TAG
POLICYCHANGELOG_UPDATE_TAG
POLICYCHANGELOG_UPDATE_TAG_ENABLED
POLICYCHANGELOG_UPDATE_TAG_NAME
POLICYCHANGELOG_UPDATE_TAG_LIST
POLICYCHANGELOG_UPDATE_TAG_LIST_NAME
POLICYCHANGELOG_UPDATE_TAG_LIST_REQUIRED
POLICYCHANGELOG_DELETE_TAG
POLICYCHANGELOG_DELETE_MULTIPLE_TAGS
POLICYCHANGELOG_IMPORT_TAGS
POLICYCHANGELOG_ADD_EMPLOYEE
POLICYCHANGELOG_DELETE_EMPLOYEE
POLICYCHANGELOG_UPDATE_EMPLOYEE
POLICYCHANGELOG_UPDATE_OWNERSHIP
POLICYCHANGELOG_ADD_CUSTOM_UNIT_RATE
POLICYCHANGELOG_DELETE_CUSTOM_UNIT_RATE
POLICYCHANGELOG_UPDATE_CUSTOM_UNIT
POLICYCHANGELOG_UPDATE_CUSTOM_UNIT_RATE
POLICYCHANGELOG_UPDATE_CUSTOM_UNIT_SUB_RATE
POLICYCHANGELOG_DELETE_CUSTOM_UNIT_SUB_RATE
POLICYCHANGELOG_IMPORT_CUSTOM_UNIT_RATES
POLICYCHANGELOG_ADD_REPORT_FIELD
POLICYCHANGELOG_UPDATE_REPORT_FIELD
POLICYCHANGELOG_DELETE_REPORT_FIELD
POLICYCHANGELOG_ADD_INTEGRATION
POLICYCHANGELOG_DELETE_INTEGRATION
POLICYCHANGELOG_UPDATE_AUTOREIMBURSEMENT
POLICYCHANGELOG_UPDATE_REIMBURSEMENT_CHOICE
POLICYCHANGELOG_UPDATE_REIMBURSEMENT_ENABLED
POLICYCHANGELOG_UPDATE_MANUAL_APPROVAL_THRESHOLD
POLICYCHANGELOG_UPDATE_MAX_EXPENSE_AMOUNT
POLICYCHANGELOG_UPDATE_MAX_EXPENSE_AMOUNT_NO_RECEIPT
POLICYCHANGELOG_UPDATE_MAX_EXPENSE_AGE
POLICYCHANGELOG_UPDATE_DEFAULT_BILLABLE
POLICYCHANGELOG_UPDATE_DEFAULT_REIMBURSABLE
POLICYCHANGELOG_UPDATE_DEFAULT_TITLE
POLICYCHANGELOG_UPDATE_DEFAULT_TITLE_ENFORCED
POLICYCHANGELOG_UPDATE_AUTOREPORTING_FREQUENCY
POLICYCHANGELOG_UPDATE_AUTOHARVESTING
POLICYCHANGELOG_UPDATE_AUDIT_RATE
POLICYCHANGELOG_UPDATE_ACH_ACCOUNT
POLICYCHANGELOG_UPDATE_DISABLED_FIELDS
POLICYCHANGELOG_CORPORATE_UPGRADE
POLICYCHANGELOG_TEAM_DOWNGRADE
2026-07-10.00-25-36.mp4
|
@FitseTLT BE PRs are merged, can you please check again? |
2 participants
Explanation of Change
This PR adds the optimistic Onyx data that completes the "Review your workspace settings" onboarding task when a workspace setting is changed. The backend already completes the task when it processes these commands, so the client only mirrors that completion optimistically — no
CompleteTaskrequest is sent.Wired for 10 workspace-settings commands:
UPDATE_WORKSPACE_DESCRIPTIONUPDATE_WORKSPACE_GENERAL_SETTINGSSET_WORKSPACE_AUTO_REPORTING_FREQUENCYSET_WORKSPACE_AUTO_REPORTING_MONTHLY_OFFSETSET_POLICY_EXPENSE_MAX_AGESET_POLICY_EXPENSE_MAX_AMOUNTSET_POLICY_EXPENSE_MAX_AMOUNT_NO_RECEIPTSET_POLICY_BILLABLE_MODESET_POLICY_DEFAULT_REPORT_TITLESET_POLICY_PREVENT_MEMBER_CREATED_TITLEFixed Issues
$ #67648
PROPOSAL: #67648 (comment)
Tests
Precondition: sign in with an account onboarded as
newDotAdmin(invited to a workspace as an admin) so the "Review your workspace settings" task exists in the onboarding/Concierge chat.UpdateWorkspaceDescription) — there must be noCompleteTaskrequest.Overview → Name; Overview → Currency; Workflows → Report frequency; Workflows → Monthly offset; Rules → Max expense age; Rules → Max expense amount; Rules → Receipt required amount; Rules → Require receipts; Rules → Billable default; Reports → Default title; Reports → Enforce title toggle.
newDotManageTeam). Change a workspace setting and verify the setting saves normally and no task-completed action is created.CompleteTask: complete a task from the task header/preview, finish the Test Drive task, and create a workspace category/tag during onboarding. Verify each still completes and still sends itsCompleteTaskrequest.Offline 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