Optimistic update for moving tasks for "Manage Team's Expenses" to #admins room#52314
Optimistic update for moving tasks for "Manage Team's Expenses" to #admins room#52314yuwenmemon merged 35 commits intoExpensify:mainfrom
Conversation
moveTasksToAdminsRoom.mp4 |
src/libs/actions/Report.ts
Outdated
| const integrationName = userReportedIntegration ? CONST.ONBOARDING_ACCOUNTING_MAPPING[userReportedIntegration] : ''; | ||
| const actorAccountID = CONST.ACCOUNT_ID.CONCIERGE; | ||
| const targetChatReport = ReportUtils.getChatByParticipants([actorAccountID, currentUserAccountID]); | ||
| const actorAccountID = engagementChoice === CONST.ONBOARDING_CHOICES.MANAGE_TEAM ? CONST.ACCOUNT_ID.QA_GUIDE : CONST.ACCOUNT_ID.CONCIERGE; |
There was a problem hiding this comment.
We don't use this, as discussed here https://expensify.slack.com/archives/C07HPDRELLD/p1731436691482209?thread_ts=1730777288.450589&cid=C07HPDRELLD
There was a problem hiding this comment.
These are required for generating optimistic actions.
However, I will remove it from the params passed to CompleteGuidedSetup if it is not used in the backend.
src/CONST.ts
Outdated
| NOTIFICATIONS: Number(Config?.EXPENSIFY_ACCOUNT_ID_NOTIFICATIONS ?? 11665625), | ||
| PAYROLL: Number(Config?.EXPENSIFY_ACCOUNT_ID_PAYROLL ?? 9679724), | ||
| QA: Number(Config?.EXPENSIFY_ACCOUNT_ID_QA ?? 3126513), | ||
| QA_GUIDE: Number(Config?.EXPENSIFY_ACCOUNT_ID_QA_GUIDE ?? 14365522), |
There was a problem hiding this comment.
This seems to be not a problem anymore. CreateWorkspace does not still return a personal detail for qa.guide. So, I retained this for the optimistic detail update.
We can use Concierge or currentUserAccountID instead of qa.guide for the optimistic report action generation. Backend overwrites these with qa.guide as the actor.
There was a problem hiding this comment.
Can you try to use the guide's details that are sent along with their intro message following the call to CreatePolicy earlier in the guided setup flow?
There was a problem hiding this comment.
When I checked this last time, CreateWorkspace was not returning qa.guide details. Let me check now again.
There was a problem hiding this comment.
CompleteGuidedSetup returns the assigned guide details. These optimistic task report actions etc. are created just after clicking on the accounting step and the assigned guide details are not available at that time.
The sender appears for the tasks as qa.guide briefly before they are replaced with the actual guide details sent in the response for CompleteGuidedSetup.
There was a problem hiding this comment.
guideDetailOnCompleteGuidedSetup.mov
|
|
||
| const [allPersonalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); | ||
|
|
||
| const setOptimticQAGuidePersonalDetail = () => { |
There was a problem hiding this comment.
This too is for the optimistic generation of report actions because CreateWorkspace's response still does not have the personal detail value for qa.guide. This can be omitted if we use Concierge or currentUserAccountID as the actor for the optimistic report actions. The actor is overwritten with qa.guide in backend's response.
src/libs/actions/Report.ts
Outdated
| const integrationName = userReportedIntegration ? CONST.ONBOARDING_ACCOUNTING_MAPPING[userReportedIntegration] : ''; | ||
| const actorAccountID = CONST.ACCOUNT_ID.CONCIERGE; | ||
| const targetChatReport = ReportUtils.getChatByParticipants([actorAccountID, currentUserAccountID]); | ||
| const actorAccountID = engagementChoice === CONST.ONBOARDING_CHOICES.MANAGE_TEAM ? CONST.ACCOUNT_ID.QA_GUIDE : CONST.ACCOUNT_ID.CONCIERGE; |
|
@c3024 Is this ready for testing online or should I only be testing offline for now? |
|
This is ready for testing online too! |
This comment was marked as resolved.
This comment was marked as resolved.
|
@yuwenmemon @jjcoffee |
|
I guess the only thing outstanding now is waiting for the BE change to accept whatever we send from FE to avoid duplication?
|
|
Let's send only the tasks in order to not have duplicates. Let the guide message come from the backend. |
| const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED); | ||
| const session = useSession(); | ||
|
|
||
| // A guide is assigned to the user if they choose the 'MANAGE_TEAM' onboarding option, except for users with emails containing '+'. |
There was a problem hiding this comment.
@c3024 I was thinking of something a bit more concise like "Guides are assigned for the MANAGE_TEAM onboarding action, except for emails that have +.". I think the rest is self-explanatory/easy to understand from a quick read of the code.
src/libs/actions/Report.ts
Outdated
| } | ||
| } | ||
|
|
||
| // A guide is assigned to the user if they choose the 'MANAGE_TEAM' onboarding option, except for users with emails containing '+'. |
There was a problem hiding this comment.
@c3024 Also could you make this one similarly more concise.
|
@yuwenmemon Just to check the expected result, if we're offline before |
If we go offline before receiving the |
jjcoffee
left a comment
There was a problem hiding this comment.
All good from my side, just this comment for you to check @yuwenmemon.
|
Think that's fine for now yeah! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/yuwenmemon in version: 9.0.70-0 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.70-9 🚀
|




Explanation of Change
This PR moves the onboarding tasks to #admins room.
Fixed Issues
$ #51443
PROPOSAL: NA
Tests
Offline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
guideAndroid.mp4
Android: mWeb Chrome
Screenrecorder-2024-11-26-14-13-37-829.mp4
iOS: Native
guideiOS.mp4
iOS: mWeb Safari
guideiOSmWeb-compressed.mp4
MacOS: Chrome / Safari
guideChrome.mp4
MacOS: Desktop
guideDesktop.mp4