[HOLD for payment 2023-07-17] [$1000] Back Button is shown on desktop in Report Screen

[kavimuru]

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 app
2. Edit the last two digits of a report to some letter

Expected Result:

There should be no back button for desktop web. It should be only for mWeb.

Actual Result:

There is a back button even for desktop web.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.32-5
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

Recording.5163.mp4

Screen.Recording.2023-06-21.at.11.43.51.AM.mov

Expensify/Expensify Issue URL:
Issue reported by: @esh-g
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687328203617079

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cf5f3f1131fc4f89
• Upwork Job ID: 1676619009155428352
• Last Price Increase: 2023-07-05

[melvin-bot]

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

[kavimuru]

Proposal

Please re-state the problem

Back button is shown for desktop web on FullPageNotFoundView

What is the root cause of the problem?

We are passing the shouldShowBackButton prop to FullPageNotFoundView, but FullPageNotFoundView has no prop defined for shouldShowBackButton so it is being ignored.

App/src/pages/home/ReportScreen.js

Line 256 in 342d4fd

shouldShowBackButton={this.props.isSmallScreenWidth}

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

We should add the shouldShowBackButton prop to FullPageNotFoundView and pass it to its child component HeaderWithBackButton so that the expected behaviour can be achieved.

<HeaderWithBackButton onBackButtonPress={props.onBackButtonPress} shouldShowBackButton={props.shouldShowBackButton} />

Result
https://github.com/Expensify/App/assets/77237602/f9b4e920-9b87-4a02-97c1-d5a94b2c71cf

What other alternatives did you explore

None

[melvin-bot]

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[anmurali]

Why is this a bug. Why should it not be on web? It seems like useful functionality that is working as it should.

[esh-g]

@anmurali
The expected behaviour is to hide the back button. We can be sure of that because in ReportScreen.js, we are passing the prop shouldShowBackButton only when the screen is small size.

Basically this code means that we should only show back button when the screen is small size like on mobile.

[melvin-bot]

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

[melvin-bot]

Current assignee @anmurali is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[multijump]

Proposal

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

Back Button is shown on desktop in Report Screen

What is the root cause of that problem?

Didn't pass shouldShowBackButton as props to HeaderWithBackButton component in FullPageNotFoundView component even though shouldShowBackButton as isSmallScreenWidth was passed in ReportScreen component.
In the HeaderWithBackButton component, shouldShowBackButton's default value is true so back button is always showing in FullPageNotFoundView component.

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

We need to pass shouldShowBackButton as props to HeaderWithBackButton component in FullPageNotFoundView.
Like this.

<HeaderWithBackButton onBackButtonPress={props.onBackButtonPress} shouldShowBackButton={props.shouldShowBackButton} />

As expected result is to show it in only mWeb, isSmallScreenWidth will work correctly.

What alternative solutions did you explore? (Optional)

N/A

[sobitneupane]

Proposal from @esh-g looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Upwork job

[melvin-bot]

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

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]

📣 @esh! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

The BZ member will need to manually hire esh for the Reporter role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[anmurali]

Ah reporting bonus as in that $250. Gotcha. I will create a new offer for that.

[sobitneupane]

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:

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

#18402

• [@sobitneupane] 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/18402/files#r1272367467

• [@sobitneupane] 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:

This was a case missed in the PR review. This should have been caught earlier. But the PR that caused this issue was a huge one involving navigation refactor. So, could have been missed.

• [@sobitneupane] Determine if we should create a regression test for this bug.

Yes.

• [@sobitneupane] 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.

[sobitneupane]

Regression Test Proposal

1. Go to any report (chat).
2. Change the reportID in the web URL to an invalid one.
3. Verify that the back button on the header of "Not Found" page appears on small screen (i.e., mWeb) and doesn't appear on wide screen (i.e., Web)

Do we agree 👍 or 👎

[anmurali]

https://www.upwork.com/nx/wm/offer/25721215 @esh-g

[anmurali]

@techievivek - do you agree with the regression test proposal?

[esh-g]

@anmurali Accepted the offer!

[sobitneupane]

Payment requested on newDot.

[techievivek]

@anmurali

do you agree with the regression test proposal?

Yes, looks good to me.

[JmillsExpensify]

@anmurali Can you please summarize the appropriate individual payments for all parties involved in this issue? This is holding up @sobitneupane's NewDot payments. More information on this compliance process in Slack.

[techievivek]

Gentle bump @anmurali for the above comment.

[JmillsExpensify]

Reached out via DM.

[anmurali]

• Issue Reporter @esh-g $250
• Contributor @esh-g $1000 + $500 (speed bonus)
• Contributor plus @sobitneupane $1000 + $500 (speed bonus)

[JmillsExpensify]

Reviewed details for @sobitneupane. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot.

[melvin-bot]

@anmurali, @sobitneupane, @techievivek, @esh-g Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[techievivek]

Not overdue, waiting for the payment to complete.

[sobitneupane]

I think we are good to close this issue. I received payment through newDot.

[techievivek]

Ok great, closing this now. Thanks