[$250] Workspace - WS chat is created for invalid phone number
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.0.52-5 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): [email protected] **Issue reported by:**Applause Internal Team
Action Performed:
- Login to an account
- create a WS
- Invite a user with invalid Phone number(Invalid Phone: +1 631-791-8378) Notice error is shown and can't invite user.
- Go to inbox
Expected Result:
WS chat with invalid Phone number is not created
Actual Result:
WS chat with invalid Phone number is created
Workaround:
Unknown
Platforms:
- [ ] Android: Standalone
- [ ] Android: HybridApp
- [x] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/5a47eb49-3cfb-4446-8a6d-69da70913a7e
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021849978282313703906
- Upwork Job ID: 1849978282313703906
- Last Price Increase: 2024-10-26
- Automatic offers:
- akinwale | Reviewer | 104685790
Issue Owner
Current Issue Owner: @bernhardoj
Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
@stephanieelliott FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
Edited by proposal-police: This proposal was edited at 2024-10-23 10:33:54 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
WS chat is still available for failed to add workspace members.
What is the root cause of that problem?
When we invite a member, we optimistically create a report for the invited member. However, we don't clear the report when fails, we only set the isLoadingInitialReportActions metadata to false.
https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/libs/actions/Policy/Policy.ts#L1007-L1013
Also, I see another issue. The optimistic member is added to the #announce room but never removed from the room if failed to be added. We already have a failure data that should remove the optimistic member when fails. https://github.com/Expensify/App/blob/4f0ca943a51e340652885c3c1fa3dd4434b91e3f/src/libs/actions/Policy/Member.ts#L149-L165
However, participants is an object, so merging the new participants object that contains the optimistic members with the previous participants object will do nothing.
What changes do you think we should make in order to solve the problem?
We can clear the optimistic report when fails (and it's metadata).
workspaceMembersChats.onyxFailureData.push({
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReport.reportID}`,
value: null,
});
workspaceMembersChats.onyxFailureData.push({
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReport.reportID}`,
value: null,
});
OR
We can show an error instead of clearing it like this: https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/libs/actions/IOU.ts#L804-L809
workspaceMembersChats.onyxFailureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReport.reportID}`,
value: {
errorFields: {
createChat: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('report.genericCreateReportFailureMessage'),
},
},
});
This way, the user will see an error on the chat and can manually remove it.
To fix the #announce participant issue, we need to set null to each member.
value: {
participants: accountIDs.reduce((acc, curr) => {
Object.assign(acc, {[curr]: null});
return acc;
}, {}),
pendingChatMembers: announceReport?.pendingChatMembers ?? null,
},
Job added to Upwork: https://www.upwork.com/jobs/~021849978282313703906
Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)
@akinwale, @stephanieelliott Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Hey @akinwale we have a proposal here, can you please review?
Hey @akinwale we have a proposal here, can you please review?
Sure, I'll review today.
We can move forward with @bernhardoj's proposal here.
@bernhardoj Let's go with the option that is already an established pattern in other parts of the app.
πππ C+ reviewed.
Triggered auto assignment to @marcochavezf, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
Sounds good, thanks @akinwale for the review! Assigning @bernhardoj π
π£ @akinwale π An offer has been automatically sent to your Upwork account for the Reviewer role π Thanks for contributing to the Expensify app!
Let's go with the option that is already an established pattern in other parts of the app.
@akinwale sorry, which one you mean? π The clear report one or show an error?
Let's go with the option that is already an established pattern in other parts of the app.
@akinwale sorry, which one you mean? π The clear report one or show an error?
Display an error.
Ok, thanks!
PR is ready
cc: @akinwale
Hey @akinwale the PR is waiting for your review, can you take a look?
@stephanieelliott I will review today.
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.59-3 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/51923
If no regressions arise, payment will be issued on 2024-11-18. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @akinwale requires payment automatic offer (Reviewer)
- @bernhardoj requires payment through NewDot Manual Requests
@akinwale @stephanieelliott The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
@stephanieelliott I am eligible to be paid through ND on this one so I'll decline the automatic offer.
Summarizing payment on this issue:
- Contributor: @akinwale $250 - please request via ND
- Contributor+: @bernhardoj $250 - please request via ND
Requested in ND.
$250 approved for @akinwale
$250 approved for @bernhardoj