[HOLD for payment 2024-03-11] [$500] [Wave Collect] [Ideal Nav] Navigating back from the Room Description page opens the Room Settings page

[hayata-suenaga]

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

---

Version Number: v1.4.38-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Logs: N/A
Expensify/Expensify Issue URL: N/A
Issue reported by: internal employee
Slack conversation: internal slack

Action Performed:

1. Open App on a narrow browser or on mobile
2. Create a new room and open to a room report/chart
3. Click the room name at the top
4. Confirm that the Room Description page appears
5. Click the back bottom

Expected Result:

The app should navigate back to the chat page for the room

Actual Result:

The app navigates to the Room Settings page

Workaround:

N/A

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

room-description.MP4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0102c95bfabb9f4183
• Upwork Job ID: 1755293298126323712
• Last Price Increase: 2024-02-07
• Automatic offers:
• aimane-chnaif | Reviewer | 0
• ZhenjaHorbach | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0102c95bfabb9f4183

[melvin-bot]

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

[melvin-bot]

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

[allgandalf]

Proposal

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

Navigating back from the Room Description page opens the Room Settings page

What is the root cause of that problem?

We pass the wrong route in HeaderWithBackButton

App/src/pages/RoomDescriptionPage.tsx

Lines 71 to 73 in 82e93b8

	<HeaderWithBackButton
	title={translate('reportDescriptionPage.roomDescription')}
	onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID))}

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

Pass The correct route to onBackButtonPress i.e. Navigation.goBack()

What alternative solutions did you explore? (Optional)

N/A

Result

WhatsApp.Video.2024-02-07.at.11.53.46.PM.mp4

[ZhenjaHorbach]

Proposal

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

[Wave 8] [Ideal Nav] Navigating back from the Room Description page opens the Room Settings page

What is the root cause of that problem?

The main problem with issue is that when we press back button we always return on details page

App/src/pages/RoomDescriptionPage.tsx

Lines 71 to 74 in 82e93b8

	<HeaderWithBackButton
	title={translate('reportDescriptionPage.roomDescription')}
	onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID))}
	/>

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

To fix this issue we can update onBackButtonPress like

onBackButtonPress={() => Navigation.goBack()}

in this case we always will return go back on the previous screen

What alternative solutions did you explore? (Optional)

NA

[ZhenjaHorbach]

@GandalfGwaihir
Don't forget about the comment
And before posting a proposition from another problem if you want to be first
At least edit it
Please )

[allgandalf]

Updated Proposal

Added video supporting my proposed solution

[allgandalf]

@ZhenjaHorbach , yes i was about to post a comment stating the updated proposal, uploading the video took some time :) Thanks for the patience

[ZhenjaHorbach]

This is not what you edited )
Also add comment about correct route
Which I first added to my proposal

[allgandalf]

Well, i added the route name later but i had correct RCA before your proposal @ZhenjaHorbach

[aimane-chnaif]

ok so this is not regression from Ideal Nav PR but just follow-up from new feature of room description field.
@ZhenjaHorbach's proposal looks good to me.
@GandalfGwaihir let's try to first post completed solution if possible, not skeleton
🎀 👀 🎀 C+ reviewed

[allgandalf]

Hey @aimane-chnaif but if you check the timings, my RCA was before @ZhenjaHorbach , also my solution stated the exact change i updated the video and detailed route, you can check my history too :)

[muas19]

Seems this is part of this #35626

[melvin-bot]

Current assignee @hayata-suenaga is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[aimane-chnaif]

Seems this is part of this #35626

@muas19 please read my explanation here

[ZhenjaHorbach]

It seems to me that finding a place to fix this is the easiest thing
But the correct changes are also important)
And it's not the correct changes

Hey @aimane-chnaif but if you check the timings, my RCA was before @ZhenjaHorbach , also my solution stated the exact change i updated the video and detailed route

[allgandalf]

I read that the proposed solution need not be detailed but a overview of how we would proceed in the solution and these things will get handled in the PR

[FitseTLT]

@aimane-chnaif Are u sure u are selecting the correct proposal? it looks like a recent change from #34150
it should be reverted to prev state

onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_SETTINGS.getRoute(report.reportID))}

[aimane-chnaif]

@aimane-chnaif Are u sure u are selecting the correct proposal? it looks like a recent change from #34150 it should be reverted to prev state

onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_SETTINGS.getRoute(report.reportID))}

@FitseTLT I considered that already.
Directly navigating to room description and Details > Room description both have the same url. So it's safe to just navigate back without param.

[aimane-chnaif]

Alternatively, we can add backTo param when navigate from Details page but I don't think it's needed.

[FitseTLT]

What about refreshing page while on room description, that's the purpose of fallbackRoutes ? @aimane-chnaif

[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.4.46-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Navigating back from the Room Description page opens the Room Settings page #37440

If no regressions arise, payment will be issued on 2024-03-11. 🎊

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

• @ZhenjaHorbach requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment automatic offer (Reviewer)

[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:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] 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:
• [@aimane-chnaif] 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:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] 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.
• [@kevinksullivan] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[hayata-suenaga]

@aimane-chnaif could you finish the checklist please?

[aimane-chnaif]

Offending PR with comment: https://github.com/Expensify/App/pull/34150/files#r1523929426

Issue reported by: internal employee

Based on this, it looks like we need to add new regression test for this.

Regression Test Proposal

1. Go to any policy room with description
2. Click the room description in header
3. Confirm that the room description page shows
4. Click the back button
5. Verify that it navigates back to room chat page, not room settings

[hayata-suenaga]

waiting for payment

[hayata-suenaga]

@kevinksullivan please handle payment when you have time 🙇

[hayata-suenaga]

waiting for payment

[melvin-bot]

@kevinksullivan, @ZhenjaHorbach, @aimane-chnaif, @hayata-suenaga Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ZhenjaHorbach]

@kevinksullivan
We are ready for payment )

[hayata-suenaga]

@kevinksullivan

@ZhenjaHorbach is the contributor who authored the PR
@aimane-chnaif is the C+ who reviewed the PR

The proposal was approved on Feb 28
The PR was opened on Feb 28
The PR was merged on Feb 29

[kevinksullivan]

all set