[HOLD for payment 2023-10-10] Web - Private note - Private notes closed when clicking save button after refreshing the page

[kbecciv]

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 the website
2. Open any group chat from LHN if there is a group chat and then go to step 7. If there is no group chat, go to Step 3
3. Click the + icon from LHN
4. Click Send message
5. Add users by clicking Add to group
6. Click Create group
7. Click the header from the group chat and open detail RHN
8. Click on the user from the group members
9. Click Private notes
10. Click my notes
11. Click Notes
12. Add some value and Click save
13. Do step 11 again
14. Reload the page
15. Click the save button
16. Observe the result

Expected Result:

When clicking save the value should be saved and goes to Notes RHN

Actual Result:

When clicking the save button the RHN gets closed

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: 1.3.72-10
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

Screenshare.-.2023-09-20.1_41_48.PM.1.mp4

bandicam.2023-09-22.17-46-20-788.mp4

Expensify/Expensify Issue URL:
Issue reported by: @misgana96
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695206404845639

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017083ab1bb282119f
• Upwork Job ID: 1705205888707555328
• Last Price Increase: 2023-09-22
• Automatic offers:
• ntdiary | Reviewer | 26881269
• BhuvaneshPatil | Contributor | 26881270
• misgana96 | Reporter | 26881274

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017083ab1bb282119f

[melvin-bot]

Triggered auto assignment to @mallenexpensify (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 @joekaufmanexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[BhuvaneshPatil]

Proposal

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

Private note - Private notes closed when clicking save button after refreshing the page

What is the root cause of that problem?

After refreshing, the navigation stack clears, and we go default HOME route

App/src/pages/PrivateNotes/PrivateNotesEditPage.js

Line 94 in 7bebfd3

Navigation.goBack(ROUTES.HOME);

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

HOME routes is being used as fallback here, we can use
getPrivateNotesViewRoute function to get the route.and use instead of ROUTES.HOME.
account ID can be grabbed using - route.params.accountID

What alternative solutions did you explore? (Optional)

Also I want to point out about other pages,
After refresh, when we save the note and got notes list page. Upon clicking on back button in header, we are taken to HOME route.
The reason being same, after refresh the stack is cleared.
For fixing this we need to tweak the HeaderWithBackButton and provide new prop, fallbackRoute
And assign it default value as ROUTES.HOME.
And modify the onBackButtonPress method.

The new prop will be the string which takes user back to expected route.

[joekaufmanexpensify]

@mallenexpensify going to unassign here as I was the duplicate assignee here, and this doesn't need two BZ assignees. LMK if there's anything I can help with!

[mallenexpensify]

@ntdiary please review @BhuvaneshPatil 's proposal above
#28011 (comment)

[ntdiary]

Sorry for the delay, reviewing now. : )

[ntdiary]

@mallenexpensify, I think @BhuvaneshPatil's proposal makes sense. It looks like we didn't pay more attention to handling navigation fallback for the private note feature during development (just use ROUTES.HOME). I think it's okay to use onBackButtonPress and Navigation.goBack like other flows (e.g. workspace member), and it would be better to fix it for all three pages:

App/src/pages/workspace/WorkspaceInvitePage.js

Lines 218 to 221 in 0477b33

	onBackButtonPress={() => {
	Policy.clearErrors(props.route.params.policyID);
	Navigation.goBack(ROUTES.WORKSPACE_MEMBERS.getRoute(props.route.params.policyID));
	}}

1. List page
   
2. View page
   
3. Edit page
   

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

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

📣 @misgana96 🎉 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

[techievivek]

@BhuvaneshPatil Do we usually add the fallback param explicitly when we call goBack? I missed it in original implementation and wasn't aware that it's a thing.

[ntdiary]

Yeah, we have explicitly specified the fallback param in many places.
Related PR and slack:
#26710
https://expensify.slack.com/archives/C049HHMV9SM/p1690915949087489

[BhuvaneshPatil]

@techievivek Where do we show the private notes, is it related only to user accounts or other places as well
example - I can see the private notes when I open the details of workspace and user account.

Because if we want to redirect user to correct details component there are two choices -

• using account ID
• using report ID

[BhuvaneshPatil]

@ntdiary raised the PR, donno why it didn't added you automatically.

[ntdiary]

example - I can see the private notes when I open the details of workspace and user account.

@BhuvaneshPatil, Ah, I see. Scenario: there are multiple flows that can open the same page.

1. workspace report -> click header -> open private notes page.
2. 1v1 report -> click header -> open private notes page.
3. group reprot -> click header -> select one member -> open private notes page.

Personally, I think for this case, it's still okay to just use ROUTES.HOME.
@techievivek, If you have different thoughts, please feel free to let me know.

demo.mp4

[BhuvaneshPatil]

Yes, I think for workspace list page, we shall keep fallback route as HOME.

[BhuvaneshPatil]

@techievivek
Shall we keep the HOME as fallback route for Private Notes list page because we can go to that route through many possibilities.

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @BhuvaneshPatil got assigned: 2023-09-26 10:30:23 Z
• when the PR got merged: 2023-10-02 06:10:41 UTC
• days elapsed: 3

On to the next one 🚀

[mallenexpensify]

@techievivek , when you have a moment (he might be traveling right now)

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

• fix: add proper fallback route for private notes pages #28273

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

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:

• @ntdiary requires payment offer (Reviewer)
• @BhuvaneshPatil requires payment offer (Contributor)
• @misgana96 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:

• [@ntdiary] The PR that introduced the bug has been identified. Link to the PR:
• [@ntdiary] 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:
• [@ntdiary] 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:
• [@ntdiary] Determine if we should create a regression test for this bug.
• [@ntdiary] 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:

[BhuvaneshPatil]

@mallenexpensify

Can you please consider timings for bonus, the delay happened because of weekends and timezone mismatch.
as you have mentioned

@techievivek , when you have a moment (he might be traveling right now)

Thanks

[mallenexpensify]

@BhuvaneshPatil I think that's fair, espec since Melv says 3 days ¯_(ツ)_/¯

Issue reporter: @misgana96 paid $50 via Upwork
Contributor: @BhuvaneshPatil paid $750 via Upwork, inc. 50% urgency bonus
Contributor+: @ntdiary paid $750 via Upwork.

@ntdiary can you please accept the job and reply here once you have? Can you also fill out the BZ checklist above? Thx
https://www.upwork.com/jobs/~017083ab1bb282119f

[ntdiary]

@mallenexpensify, sure, I will fill out the checklist soon. Also, I just accepted the offer from the service account, and then invited you in the room, not sure if this works. 😂

[ntdiary]

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:

• [@ntdiary] The PR that introduced the bug has been identified. Link to the PR: Add private notes feature to newDot #25761
• [@ntdiary] 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/25761/files#r1353906729
• [@ntdiary] 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: No need
• [@ntdiary] Determine if we should create a regression test for this bug. No need
• [@ntdiary] 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. No need
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Just a minor feature polish, not a regression. : )

[mallenexpensify]

Thanks @ntdiary , I updated the post above to denote you're paid.
Closing! Thanks for the help!