[HOLD for payment 2024-06-06] [$250] RHP - User remains on the Private Notes page when clicked on back btn after reload

[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.63-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Open https://staging.new.expensify.com/
2. Navigate to any conversation with other user
3. Click on Header to open Profile page
4. Click on Private Notes
5. Reload the page
6. Click on back button
7. Click on back button again

Expected Result:

User should be redirected to Profile page when clicked back button

Actual Result:

User remains on the Private Notes page when clicked on back button after page reload. When user clicked on back button again the RHP will be closed

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

Bug6454034_1713463346120.Screen_Recording_2024-04-18_at_20.55.17.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f0ae1b806ad11050
• Upwork Job ID: 1781084747484880896
• Last Price Increase: 2024-04-18
• Automatic offers:
• shubham1206agra | Reviewer | 0
• nkdengineer | Contributor | 0

Issue Owner

Current Issue Owner: @shubham1206agra

[melvin-bot]

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

[lanitochka17]

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

[abzokhattab]

Proposal

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

RHP closes when clicking on the back button in the private notes list page

What is the root cause of that problem?

we use the default navigation behaviour in the private notes list which dismisses the navigation if no route is found in the navigation history

App/src/pages/PrivateNotes/PrivateNotesListPage.tsx

Line 85 in 0140912

<HeaderWithBackButton

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

since the back navigation for the private notes history is deterministic and always goes to the report details page then we can specify that behavior in the onBackButtonPress

                onBackButtonPress={() => ReportUtils.goBackToDetailsPage(report)}

[shahinyan11]

Proposal

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

RHP - User remains on the Private Notes page when clicked on back btn after reload

What is the root cause of that problem?

Here in the PRIVATE_NOTES_EDIT page we always go to PRIVATE_NOTES_LIST page using Navigation.goBack method by clicking back button. When we pass the fallbackRoute param in the Navigation.goBack method, it only goes to that route if the current route is the first one in the navigation stack, otherwise it goes back to the previous route. And so the current screen becomes first one in navigation stack, we navigate to the fallbackRoute after clicking the back button.

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

1. Add ability to receive backTo param for ROUTES.PRIVATE_NOTES_EDIT.getRoute method. To do this update this line as below

getRoute: (reportID: string, accountID: string | number, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/notes/${accountID}/edit` as const, backTo),

2. Add Navigation.getActiveRoute() as third ( backTo ) param in getRoute methods here and here

3. Update this line like below

onBackButtonPress={() => Navigation.goBack(route.params?.backTo)}

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

Was able to repro, seems like it can be external, @shubham1206agra , can you review the bug and proposals above? Thx

2024-04-18_15-16-59.mp4

[nkdengineer]

Proposal

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

User remains on the Private Notes page when clicked on back button after page reload. When user clicked on back button again the RHP will be closed

What is the root cause of that problem?

We always fallback go back to private note list page here

App/src/pages/PrivateNotes/PrivateNotesEditPage.tsx

Line 119 in 68e6962

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

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

We should use the same check as we do here to go to the private note edit or private note list page.

App/src/libs/ReportUtils.ts

Lines 5622 to 5628 in 68e6962

	const currentUserPrivateNote = report.privateNotes?.[session.accountID]?.note ?? '';
	if (isEmpty(currentUserPrivateNote)) {
	Navigation.navigate(ROUTES.PRIVATE_NOTES_EDIT.getRoute(report.reportID, session.accountID));
	return;
	}
	Navigation.navigate(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID));
	}

1. Create a utils like goBackFromPrivateNotes in ReportUtils. If currentUserPrivateNote is empty we will go back to to detail page by using goBackToDetailsPage function. Otherwise fallback to private note list.

function goBackFromPrivateNotes(report: OnyxEntry<Report>, session: OnyxEntry<Session>) {
    if (isEmpty(report) || isEmpty(session) || !session.accountID) {
        return;
    }
    const currentUserPrivateNote = report.privateNotes?.[session.accountID]?.note ?? '';
    if (isEmpty(currentUserPrivateNote)) {
        goBackToDetailsPage(report);
        return;
    }
    Navigation.goBack(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID));
}

2. In PrivateNotesEditPage, subscribe session from Onyx. And then call goBackFromPrivateNotes function here

onBackButtonPress={() => ReportUtils.goBackFromPrivateNotes(report, session)} 

App/src/pages/PrivateNotes/PrivateNotesEditPage.tsx

Line 119 in 68e6962

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

What alternative solutions did you explore? (Optional)

NA

[dragnoir]

Proposal

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

RHP - User remains on the Private Notes page when clicked on back btn after reload

What is the root cause of that problem?

PrivateNotesEditPage is navigating back to ROUTES.PRIVATE_NOTES_LIST here

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

The page is used on many details page, the back button should be general not specific. As we do on ReportSettingsPage

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

Line 60 in 305f12c

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

We should use ROUTES.REPORT_WITH_ID_DETAILS

- onBackButtonPress={() => Navigation.goBack(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID))}
+ onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID))}

Note

using onBackButtonPress={() => ReportUtils.goBackToDetailsPage(report)} can work only on the profile page but we use notes on many details pages

POC:

20240419_113926.mp4

[mallenexpensify]

@shubham1206agra plz review proposals above, thx.

[kaushiktd]

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

RHP - User remains on the Private Notes page when clicked on back btn after reload

What is the root cause of that problem?

The user remains on the Private Notes page when clicking the back button after a page reload. Subsequently, clicking the back button again results in the Right Hand Pane (RHP) being closed instead of navigating to the expected profile page.

The reason for this behavior is that the PrivateNotesEditPage component is incorrectly navigating back to ROUTES.PRIVATE_NOTES_LIST when the back button is pressed. However, the intended behavior is to navigate back to the profile page (ROUTES.PROFILE).

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

add variable
const participantAccountID = report.participantAccountIDs && report.participantAccountIDs.length > 0
? report.participantAccountIDs[0]
: null;

Replace the existing navigation code:

App/src/pages/PrivateNotes/PrivateNotesEditPage.tsx

Line 119 in 68e6962

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

With the updated navigation code to navigate to the profile page:
onBackButtonPress={() => Navigation.goBack(ROUTES.PROFILE.getRoute(participantAccountID))}

video:-https://drive.google.com/file/d/1YeP1T1ycClNsbVIoWxO9PXRDz6rtRfxw/view?usp=sharing

[shubham1206agra]

Well, I am going to go ahead with @nkdengineer's proposal with one side note, i.e., we should use a modified version of navigateToDetailsPage instead of goBackToDetailsPage.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @shubham1206agra 🎉 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]

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

[shahinyan11]

@shubham1206agra @mountiny Are you sure that whenever currentUserPrivateNote is empty, we should go to the details page.
Could you give your opinion on my proposal? I want to understand what is the advantage of @nkdengineer's propsal compared to mine

[dragnoir]

@shubham1206agra thank you for your review.

I feel that my proposal is simple and effective. When you have time, can you pls share your opinion about t it? Thank you for your time.

[melvin-bot]

This issue has not been updated in over 15 days. @mallenexpensify, @mountiny, @shubham1206agra, @nkdengineer 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!

[mallenexpensify]

Seems to have gone a bit stale but... we're in a merge freeze now so the PR should be deployed soon

• Navigate back to correct page from private note edit page #40951 (comment)

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

• Navigate back to correct page from private note edit page #40951

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

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

• @shubham1206agra requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (Contributor)

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

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

[melvin-bot]

@mallenexpensify, @mountiny, @shubham1206agra, @nkdengineer Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

Contributor: @nkdengineer paid $250 via Upwork
Contributor+: @shubham1206agra paid $250 via Upwork.

@shubham1206agra , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01435fc0aa943015f5

@shubham1206agra can you also fill out the BZ checklist? Thx

Sorry I missed this (not sure how)

[shubham1206agra]

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:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: [TS migration] Migrate 'PrivateNotes' page to TypeScript #34656
• [@shubham1206agra] 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: https://github.com/Expensify/App/pull/34656/files#r1635821755
• [@shubham1206agra] 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: Not Required
• [@shubham1206agra] Determine if we should create a regression test for this bug. No
• [@shubham1206agra] 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.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

@mallenexpensify, @mountiny, @shubham1206agra, @nkdengineer Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@mallenexpensify, @mountiny, @shubham1206agra, @nkdengineer 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[nkdengineer]

@mallenexpensify I think we can process payment now and then we can close this issue.

[melvin-bot]

@mallenexpensify, @mountiny, @shubham1206agra, @nkdengineer 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[mallenexpensify]

Closing, everyone's paid. I created a TestRail GH too

• https://github.com/Expensify/Expensify/issues/406659