[$500] [MEDIUM] Workspace - When adding a member to WS fails, a "Chat Report" conversation is created

[lanitochka17]

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: 1.4.22-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Open New Expensify app
2. Log in with HT account @applause.expensifail.com
3. Create WS
4. Add some nonexistent @applause.expensifail.com members to the created WS
5. Remove the error of adding members

Expected Result:

If an error occurs in adding a participant to WS, the created conversation must be deleted from LHN or archived, preserving its original form

Actual Result:

When removing the error of adding a member to WS, the "Chat Report" conversation remains

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6333446_1704482464611.Recording__1042.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ff2d94edb1b3acf6
• Upwork Job ID: 1743364368062525440
• Last Price Increase: 2024-01-19

Issue Owner

Current Issue Owner: @AndrewGable

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ff2d94edb1b3acf6

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee (External)

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

If we add a WS member and it fails, closing/clearing the error doesn't remove the expense chat.

What is the root cause of that problem?

When we press the clear/close error message, we only remove them from POLICY_MEMBERS and PERSONAL_DETAILS_LIST onyx

App/src/libs/actions/Policy.js

Lines 997 to 1004 in f5a3c02

	function clearAddMemberError(policyID, accountID) {
	Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policyID}`, {
	[accountID]: null,
	});
	Onyx.merge(`${ONYXKEYS.PERSONAL_DETAILS_LIST}`, {
	[accountID]: null,
	});
	}

but we never clear the optimistic expense chat that we add when adding the member.

What changes do you think we should make in order to solve the problem?

Remove the optimistic expense chat in clearAddMemberError.

(we can iterate over all reports that is a policy expense chat with a policy ID of policyID and with an owner account ID of the member accountID)

What alternative solutions did you explore? (Optional)

Or immediately clear the report if the request fails in the failureData

App/src/libs/actions/Policy.js

Lines 527 to 534 in f5a3c02

	workspaceMembersChats.onyxFailureData.push({
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReport.reportID}`,
	value: {
	isLoadingInitialReportActions: false,
	},
	});
	});

(easier and no need to iterate over all reports every time clearing an error)

[tienifr]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When removing the error of adding a member to WS, the "Chat Report" conversation remains

What is the root cause of that problem?

In here, we're not removing the expense that if the members fails to be added. And we also don't clear them when we close the error message, so it remains in Onyx.

What changes do you think we should make in order to solve the problem?

We have a complication here where when adding multiple members, it's possible that some members were added successfully, and some members failed to be added (like the case of nonexistent @applause.expensifail.com above). So we need to:

1. In the AddMembersToWorkspace endpoint, returns something in Onyx data for each correctly added member to indicate that the members were added successfully, for example returning pendingAction: null or success: true (similar to the closeAccount data), if members were not added successfully, return the errors key as usual
2. When Onyx data of policyMembers change here, check if there's any member was successfully added but the batch as a whole failed (we can check for the field in 1 and that the policyMember record has errors, the generic one that was added here when the batch fails. For those members we should clear the errors since they were successfully added.

For the members that truly failed to be added, we can remove the expense chat for them here as well. Because if the member failed to be added, it doesn't make sense to keep the expense chat. In case we don't want to remove the expense chat at this step, we should update clearAddMemberError to remove the expense chat so the expense chat will be cleared once the user closes the error.

3. In UI where the add member error is shown, make sure not to show it for members that were successfully added (based on the condition in 1)

What alternative solutions did you explore? (Optional)

NA

[kadiealexander]

@jjcoffee bump to review these proposals please!

[jjcoffee]

Reviewing tomorrow!

[jjcoffee]

I'm leaning towards the alternative solution in @bernhardoj's proposal, as it sounds like it would be a nice straightforward solution.

I am a bit unsure on the exact expected result here, however. It looks like the report does actually get created on the BE, at least for the emails I tested. Can you confirm @bernhardoj?

[bernhardoj]

It looks like the report does actually get created on the BE, at least for the emails I tested.

I tested by inviting a VoIP phone number and after relogin, the policy expense chat is not available anymore, indicating it's not created on the BE.

[jjcoffee]

Hmm what error do you get when inviting that number? For an "incorrect" email (e.g. [email protected] and insert the current time at the end of the address part) I get 404 Account not found. I'm not really sure what triggers this error, maybe the assigned engineer can clarify.

When logging out and back in and opening the Workspace members list I also get a 404 error and an endless loader (even if I previously dismissed the initial error on adding the member).

The workspace-member chat also persists, and shows up in search, so I suspect we may need some BE changes here too to actually remove the report for this case. That is if this is actually a case we need to handle!

I've also tried an invalid number [email protected] which gives this error:

This behaves more as you describe, with the chat disappearing on relogin (and it's not accessible via deeplink).

@tienifr's proposal is correct though in that for multiple members the whole AddMembersToWorkspace request fails (on current main) and all members show as having an error. I think your alternative solution won't handle this @bernhardoj.

[bernhardoj]

Hmm what error do you get when inviting that number?

When logging out and back in and opening the Workspace members list I also get a 404 error and an endless loader (even if I previously dismissed the initial error on adding the member).

I see. I can reproduce this with the email.

multiple members the whole AddMembersToWorkspace request fails (on current main) and all members show as having an error

If the request fails, all members won't be added to the workspace (at least I don't see them after refreshing/reopening the page).

[tienifr]

If the request fails, all members won't be added to the workspace (at least I don't see them after refreshing/reopening the page).

@jjcoffee I think this is not ideal UX and should be fixed, imagine if I add 10 users, one of them has issue (eg invalid number). Then all 10 failed and I'd have to add again one by one. Only the user with error should have error, the rest should be added normally.

[jjcoffee]

When logging out and back in and opening the Workspace members list I also get a 404 error and an endless loader (even if I previously dismissed the initial error on adding the member).

Confirmed that this only happens for emails ([email protected] works as expected). Despite the error, and the fact that we hide the member if the error is dismissed, they do actually get added to the workspace on the BE (they also appear in oldDot). I think that needs to be handled with a BE fix.

If the request fails, all members won't be added to the workspace (at least I don't see them after refreshing/reopening the page).

Hmm so it looks like it actually depends on the order in which you invite people, so if the first invite is to a "real" email (that succeeds), followed by any number of fake/bad numbers, then the real email will get added (appears on oldDot, and the report persists after relogin). Another BE fix required there!

In summary, I'm wondering if it might be best to go with a fully BE fix here where the BE returns the relevant Onyx data to remove the optimistically created reports (since there are a couple BE changes required anyway).

If we want a faster fix I'm good with going with the alt. solution in @bernhardoj's proposal which doesn't require any BE changes to work, but with the caveat that it will remove optimistically created expense chats for all invited members if the request fails (even if some of the members did get added). It's not perfect, but it's better than before and those chats would re-appear after relogin!

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @AndrewGable, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[tienifr]

but with the caveat that it will remove optimistically created expense chats for all invited members if the request fails (even if some of the members did get added)

@jjcoffee This might get reported as another bug later. I think we should go with a solution that doesn't lead to another bug if that's possible.

Would you mind giving some feedback on my proposal? Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[luacmartins]

Taking over from Joel

[melvin-bot]

@luacmartins, @jjcoffee, @kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@luacmartins, @jjcoffee, @kadiealexander Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@luacmartins, @jjcoffee, @kadiealexander Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[greg-schroeder]

Cleaning up Wave 6 issues (there are hundreds) - adding users to a Workspace is an admin-only action, so setting this to Wave 8. Will let you decide on priority @zanyrenney!

[melvin-bot]

@luacmartins, @jjcoffee, @kadiealexander 10 days overdue. Is anyone even seeing these? Hello?

[melvin-bot]

@luacmartins, @jjcoffee, @kadiealexander 12 days overdue. Walking. Toward. The. Light...

[melvin-bot]

This issue has not been updated in over 14 days. @luacmartins, @jjcoffee, @kadiealexander eroding to Weekly issue.

[Christinadobrzyn]

Hi there - can we merge this issue into this job?

It seems like both GHs involve the 'Auth GetRestrictedAuthTokenByEmail returned an error' error.

[kadiealexander]

@luacmartins / @jjcoffee could you please check the issue linked above and see if it would have the same fix?

[jjcoffee]

That issue doesn't appear to be related, the error isn't the same and it's an entirely different flow. cc @Christinadobrzyn

[jjcoffee]

#25592 seems to be related, I think we can either hold on that or ask Applause for a retest cc @kadiealexander

[kadiealexander]

Have asked Applause to retest!

[kavimuru]

@kadiealexander Now the error does not appear.
Added members disappear. When navigating away and opening the members page again, all the added members are showing as 'Hidden' and 'chat report' appears in the LHN. Please check this video.

Recording.37.mp4

[melvin-bot]

This issue has not been updated in over 15 days. @luacmartins, @jjcoffee, @kadiealexander eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[luacmartins]

@kavimuru could you please retest this issue?

[kavimuru]

Not reproducible now

Recording.184.mp4

[luacmartins]

Cool, seems like we can close this issue then. Thank you!