[HOLD] [$500] Web - Leaving and revisiting #teachers-unite page displays 'Hmm its not here' page for sometime

kbecciv commented 1 year ago

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:

Open the app
Click on plus and click save the world
Click on 'I know a teacher'
Fill in details and click 'Lets do this!'
Visit #teachers-unite page and copy the URL
Click on header and leave the thread
Paste the URL and revisit, observe that it displays 'Hmm its not here' page for some time before rejoining

Expected Result:

App should either display 'skeleton view' or instantly rejoin when we leave thread and again visit #teachers-unite page

Actual Result:

App displays 'Hmm its not here' page for some time when we leave thread and again visit #teachers-unite page

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.62.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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/93399543/14dd5ebc-8c71-4f99-950b-2b37ec2687bf

https://github.com/Expensify/App/assets/93399543/25123bdc-47a8-4bb9-8cb9-42c013a95003

Expensify/Expensify Issue URL: Issue reported by: @dhanashree-sawant Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693374999062339

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0116c116dcad14c52c
Upwork Job ID: 1698576378656542720
Last Price Increase: 2023-09-04

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

[x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[x] 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
[x] 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.
[x] 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

sophiepintoraetz commented 1 year ago

2023-09-04_17-56-02 (1)

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~0116c116dcad14c52c

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

marlisajp commented 1 year ago

Hi my name is Marlisa from Upwork and I would like to submit a proposal for this issue.

Proposed Solution:

Initial Investigation: Clone the Expensify repository and reproduce the issue in a local environment. Check browser console logs and network requests to identify any errors or delays during navigation.
Potential Causes: Delays in data fetching from the backend causing the temporary display. Incorrect route handling or caching issues. Component's lifecycle events causing an unintended re-render or delay.
Addressing Data Fetching: If data fetching is causing the delay, implement a loading spinner or a more user-friendly message instead of the "Hmm its not here" placeholder. Consider prefetching data or employing optimistic UI techniques to make navigation seem instantaneous.
Route and Navigation: Ensure that routes are set up correctly, especially for the #teachers-unite page. Implement client-side caching or utilize service workers to provide faster page loads when revisiting.
Component Optimization Review the lifecycle events or hooks associated with the #teachers-unite page. Ensure unnecessary renders or delays are minimized. Use React's performance tools or browser dev tools to profile component renders and optimize accordingly.
Testing Test the fix on multiple browsers to ensure cross-browser compatibility. Ensure that revisiting the #teachers-unite page is seamless, without any noticeable delays or placeholder messages. Consider adding end-to-end tests using tools like Cypress to catch similar issues in the future.

melvin-bot[bot] commented 1 year ago

📣 @marlisajp! 📣 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:

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.
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.
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>
```

rafi16jan commented 1 year ago

Proposal

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

A placeholder of 'Hmm its not here' is displayed seconds before re-joining the teachers-unite page

What is the root cause of that problem?

It seems that there is some kind of race condition or data retrieve failure when leaving and re-joining teachers-unite

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

Either to debug the race condition and directly fix the order of execution, or implement an await that defer authentication tokens (if that's the root cause) or any other dependency before retrieving the said page itself

What alternative solutions did you explore? (Optional)

An ugly alternative solution would be hiding the Hmm its not here placeholder or replacing it with a loading indicator before the real page loads.

melvin-bot[bot] commented 1 year ago

📣 @rafi16jan! 📣 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:

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.
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.
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>
```

rafi16jan commented 1 year ago

Contributor details Your Expensify account email: abdurrafi16jan@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~016fcec68abaa3bc96

melvin-bot[bot] commented 1 year ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

marlisajp commented 1 year ago

Contributor details Expensify account email: marlisajp@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01545b41c7fab75f4b

ankitsehgal94 commented 1 year ago

Contributor details Your Expensify account email: codingmunk94@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01c1048ef2c2e1cb54

melvin-bot[bot] commented 1 year ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

rmm-fl commented 1 year ago

Proposal

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

Leaving and revisiting #teachers-unite page displays 'Hmm its not here' page for sometime

What is the root cause of that problem?

https://github.com/Expensify/App/blob/91ff78db32f0284a73db13b2e2372583d71cb27f/src/pages/home/ReportScreen.js#L298-L301

The condition for determining whether to show the NotFoundView or not is incorrect, because of that when user leaves the room the reportID can be undefined for short period of time when rejoining while the room is being re-opened. Because of that the FullPageNotFoundView is shown for a short time period.

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

Correct conditions for FullPageNotFoundView.shouldShow should be as follows:

First it should wait for the report to load by checking if report object is empty or not (using _.isEmpty)
- if it is empty it means we are still loading, so FullPageNotFoundView.shouldShow should be false
- if not empty then it should check if reportID is undefined or not
- if it is undefined it means that report doesn't exists, so FullPageNotFoundView.shouldShow should be true for this case
- if it is not undefined then we should pass shouldHideReport as-is.

What alternative solutions did you explore? (Optional)

-

Result

Video shows the issue is fixed and shows the loading animation when opening the #teachers-unite page, and also shows that it displays the FullPageNotFoundView when opening an invalid report link (link created with arbitary non-existent reportID)

https://github.com/Expensify/App/assets/143930462/e67e15a2-7d38-4ea7-bb37-496683dafef6

mollfpr commented 1 year ago

I think this has the root cause as https://github.com/Expensify/App/issues/25698, and we already have the PR.

rmm-fl commented 1 year ago

Yeah seems about right, https://github.com/Expensify/App/pull/26602 fixes the issue. In that case this issue should be closed.

sophiepintoraetz commented 1 year ago

Ah, nice find @mollfpr. In which case, let's wait until https://github.com/Expensify/App/pull/26602 is merged and we can confirm (and close).

sophiepintoraetz commented 1 year ago

Waiting for review of linked PR.

mollfpr commented 1 year ago

@sophiepintoraetz I think we can put this on HOLD.

melvin-bot[bot] commented 1 year ago

@mollfpr, @sophiepintoraetz Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 1 year ago

@mollfpr, @sophiepintoraetz Eep! 4 days overdue now. Issues have feelings too...

sophiepintoraetz commented 1 year ago

@mollfpr - are we good to close this issue out?

sophiepintoraetz commented 1 year ago

@mollfpr - we're still holding?

mollfpr commented 1 year ago

Sorry for the delay! We finished discussing the expected result in the PR https://github.com/Expensify/App/pull/26602. I will try to reproduce this while completing the test, and let you know the result.

sophiepintoraetz commented 1 year ago

Just waiting on the results of testing - not overdue.

mollfpr commented 1 year ago

@sophiepintoraetz The PR #26602 is merged now, and tested the current main the issue is not reproduced.

sophiepintoraetz commented 1 year ago

Thank you @mollfpr - let me doublecheck what needs to happen here!

sophiepintoraetz commented 1 year ago

Unnngh, have not found the time to do so - I think I need to double-check this https://docs.google.com/document/d/1BA9A1ZouVgUlR6mTGRYSPOqsgFks5-ErHmPjw64AL-U/edit

melvin-bot[bot] commented 1 year ago

@mollfpr, @sophiepintoraetz Whoops! This issue is 2 days overdue. Let's get this updated quick!

sophiepintoraetz commented 1 year ago

Apologies, this is still with me - hopefully I'll have some time tomorrow to confirm what needs to happen here.

sophiepintoraetz commented 1 year ago

Okay, @mollfpr and @dhanashree-sawant are due for payments here - offers sent!

mollfpr commented 1 year ago

@sophiepintoraetz I think I'm not due for any payment here.

sophiepintoraetz commented 1 year ago

You actually are here - https://docs.google.com/document/d/1BvohU05MTaHnjOD_vwJv_aqDAirv-ChkyRnKCAvOVyQ/edit#bookmark=id.wfm6lkgp0c6 though I think I might want to double check this.

Expensify / App