[HOLD for Payment 2024-09-06] [$250] Android - Room - Anyone can find this room dialog box is shown briefly after tapping Yes

[IuliiaHerets]

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: 9.0.19
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

1. Launch app
2. Login with new expensifail account
3. Tap fab -- nee workspace
4. Tap fab -- start chat
5. Create a new room
6. Tap header -- settings--visibility -- public
7. Tap yes

Expected Result:

Anyone can find this room dialog box must not be shown briefly after tapping yes.

Actual Result:

Anyone can find this room dialog box is shown briefly after tapping yes.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6570051_1723475967133.Screenrecorder-2024-08-12-20-43-21-442_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a6ad1425f84b08d6
• Upwork Job ID: 1823807191851235765
• Last Price Increase: 2024-08-14
• Automatic offers:
• rayane-djouah | Reviewer | 103606492
• FitseTLT | Contributor | 103606496

Issue Owner

Current Issue Owner: @garrettmknight

[melvin-bot]

Triggered auto assignment to @garrettmknight (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.

[IuliiaHerets]

We think that this bug might be related to #vip-vsb

[IuliiaHerets]

@garrettmknight 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

[FitseTLT]

Proposal

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

Android - Room - Anyone can find this room dialog box is shown briefly after tapping Yes

What is the root cause of that problem?

After change visibility we are navigating back to details page here

App/src/libs/actions/Report.ts

Lines 1752 to 1753 in cc2efc6

	ReportUtils.goBackToDetailsPage(report);
	}

But the navigation happens to soon before the modal is dismissed so the modal reappears after the navigation

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

Just the same as we fixed similar bug here

App/src/pages/ReportDetailsPage.tsx

Lines 796 to 800 in cc2efc6

	if (!navigateBackToAfterDelete.current) {
	Navigation.dismissModal();
	} else {
	ReportUtils.navigateBackAfterDeleteTransaction(navigateBackToAfterDelete.current, true);
	}

In visibility Page we will set a ref navigateToDetails to true after calling updateRoomVisibility then in onModalHide we will call the navigation (if the ref is true) ReportUtils.goBackToDetailsPage(report) we are now calling in updateRoomVisibility and if the ref is false we only call dismissModal

if (navigateToDetails.current && !isEmptyObject(report) && report.reportID) {
            ReportUtils.goBackToDetailsPage(report);
        } else {
 Navigation.dismissModal();
}

What alternative solutions did you explore? (Optional)

[daledah]

Proposal

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

Anyone can find this room dialog box is shown briefly after tapping yes.

What is the root cause of that problem?

• When click on Yes in step 7, we call:
  

  App/src/libs/actions/Report.ts

  Line 1752 in cc2efc6

  ReportUtils.goBackToDetailsPage(report);

  
  then
  

  App/src/pages/settings/Report/VisibilityPage.tsx

  Line 86 in 86da342

  hideModal();

• So if the navigate progress is too fast, the modal is still displayed briefly.

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

• We can hide the modal before navigating back to report setting screen. So:

App/src/pages/settings/Report/VisibilityPage.tsx

Lines 84 to 87 in 86da342

	onConfirm={() => {
	changeVisibility(CONST.REPORT.VISIBILITY.PUBLIC);
	hideModal();
	}}

will be:

                    onConfirm={() => {
                        hideModal();
                        changeVisibility(CONST.REPORT.VISIBILITY.PUBLIC);
                    }}

[melvin-bot]

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

[melvin-bot]

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

[rayane-djouah]

Thank you for your proposals everyone

@FitseTLT a ref was used in https://github.com/Expensify/App/pull/44777/files to determine to which page we navigate, but I think that using a ref for our case is more complex than necessary.

@daledah I tested switching the order of hideModal and changeVisibility and I'm still able to reproduce the bug.

[daledah]

@rayane-djouah Can you reproduce the bug in latest main?

[FitseTLT]

@FitseTLT a ref was used in https://github.com/Expensify/App/pull/44777/files to determine to which page we navigate, but I think that using a ref for our case is more complex than necessary.

@rayane-djouah This PR is the correct one with exactly the same case as the current issue. In our case, we don't need the ref for saving the route we are navigating to but only to determine whether we need to goBackToDetailsPage because the onModalHide can also be called if the user dismisses the modal by click outside it, for instance. That's why we only dismissModal now when navigateBackToAfterDelete is not set

App/src/pages/ReportDetailsPage.tsx

Lines 796 to 800 in cc2efc6

	if (!navigateBackToAfterDelete.current) {
	Navigation.dismissModal();
	} else {
	ReportUtils.navigateBackAfterDeleteTransaction(navigateBackToAfterDelete.current, true);
	}

[rayane-djouah]

@FitseTLT's proposal looks good to me.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @FitseTLT 🎉 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 📖

[rayane-djouah]

⚠️ We just got confirmation on Slack that the Deploy Checklist: New Expensify 2024-08-26 which includes the PR of this issue was only deployed to production in Deploy Checklist: New Expensify 2024-08-28. More context on why this happened can be found in this Slack thread and this Slack comment.

Given the context above, this issue should be on [HOLD for Payment 2024-09-6] according to production deploy from Deploy Checklist: New Expensify 2024-08-28.

cc @garrettmknight

[garrettmknight]

Payment Summary:

• Contributor - @rayane-djouah $250 via Upwork
• Reviewer - @FitseTLT $250 via Upwork

[garrettmknight]

@FitseTLT can you complete the checklist when you get a chance?

[rayane-djouah]

Checklist

• The PR that introduced the bug has been identified. Link to the PR: Fix - Room - Anyone can find this room dialog box is shown briefly after tapping Yes #47756
• 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: Fix: Room visibility can’t be changed after a room is created #35830 (comment)
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression Test Proposal

1. Open any room that is not "Public".
2. Tap header > Settings > Visibility > select "Public".
3. On the confirm modal Tap "Yes".
4. Verify that the modal is closed and you are navigated to the settings page and the confirm modal doesn't appear briefly on the settings page.

Do we agree 👍 or 👎

cc @garrettmknight

[garrettmknight]

Done!