[HOLD for C+ payment and bugzero checklist] [$500] Workspace - App crashes on member's side when deleting a workspace

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open two new accounts
2. Create a workspace on account A
3. Invite account B as a member on account A's workspace
4. Go to announce room on account B
5. Send a message
6. Hover over the last sent message and click on reply in thread
7. Send a message inside the thread
8. Go to account A and click on the workspace settings and delete the workspace
9. Observe account B

Expected Result:

App should not crash

Actual Result:

App crashes

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.74-2

Reproducible in staging?: Y

Reproducible in production?: Y

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

Notes/Photos/Videos: Any additional supporting documentation

2023-09-22.18.47.19.mp4

Recording.1662.mp4

Expensify/Expensify Issue URL:

Issue reported by: @Nathan-Mulugeta

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695397800777089

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0198d8205c8f8f5131
• Upwork Job ID: 1707090926479659008
• Last Price Increase: 2023-09-27
• Automatic offers:
• c3024 | Contributor | 26964208
• Nathan-Mulugeta | Reporter | 26964210

[c3024]

Proposal

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

App crashes on other User B side when a User A deletes a workspace.

What is the root cause of that problem?

When User A deletes the workspace, the report of rooms of the workspace are removed so in this here

App/src/libs/ReportUtils.js

Lines 854 to 855 in b57f6d2

	const parentReport = lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID}`]);
	if (hasSingleParticipant(parentReport)) {

parentReport is undefined so this here

App/src/libs/ReportUtils.js

Lines 658 to 660 in b57f6d2

	function hasSingleParticipant(report) {
	return report.participantAccountIDs && report.participantAccountIDs.length === 1;
	}

fails because report is undefined.

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

Add a check for report in the above function

return report && report.participantAccountIDs && report.participantAccountIDs.length === 1;

What alternative solutions did you explore? (Optional)

Result

screen-recording-2023-09-30-at-83839-pm_d5z0Rihh.mp4

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0198d8205c8f8f5131

[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 - @allroundexperts (External)

[allroundexperts]

@c3024 Can you also post a screen recording of the end result of your proposal as well?

[c3024]

@allroundexperts Updated the proposal with video.

[allroundexperts]

Thanks for the video @c3024. That makes sense. Let's move forward with @c3024's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

📣 @c3024 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @Nathan-Mulugeta 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[c3024]

PR ready for review @allroundexperts

[melvin-bot]

🎯 ⚡️ Woah @allroundexperts / @c3024, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @c3024 got assigned: 2023-10-01 08:38:36 Z
• when the PR got merged: 2023-10-02 21:54:29 UTC

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.77-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• prevent thread report crash for a member when workspace is deleted by owner #28559

If no regressions arise, payment will be issued on 2023-10-12. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @allroundexperts does not require payment (Eligible for Manual Requests)
• @c3024 requires payment offer (Contributor)
• @Nathan-Mulugeta requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
• [@allroundexperts] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@allroundexperts] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@allroundexperts] Determine if we should create a regression test for this bug.
• [@allroundexperts] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[kadiealexander]

Payouts due:

• Issue Reporter: $50 @Nathan-Mulugeta
• Contributor: $500 + $250 bonus @c3024
• Contributor+: $500 + $250 bonus @allroundexperts

Eligible for 50% #urgency bonus? Yes

Upwork job is here.

[kadiealexander]

@allroundexperts please don't forget the checklist!

[allroundexperts]

Checklist

1. I wasn't able to find any particular PR that caused this. I think this is an edge case that we never thought about.
2. N/A
3. N/A
4. A regression test would be useful.

Regression test

1. Create two new accounts (Account A and Account B)
2. Create a workspace on account A and Invite account B as a member on account A's workspace
3. Go to announce room on account B and Send a message
4. Hover over the last sent message and click on reply in thread
5. Send a message inside the thread
6. Go to account A and click on the workspace settings and delete the workspace
7. Observe account B and verify that the app doesn't crash.

Do we 👍 or 👎 ?

[JmillsExpensify]

$750 payment approved for @allroundexperts based on summary above.

[kadiealexander]

https://github.com/Expensify/Expensify/issues/330534