Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Page goes to home screen if press chat and refresh the page - reported by @phivh #8126

Closed
mvtglobally opened this issue Mar 14, 2022 · 143 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@mvtglobally
Copy link

mvtglobally commented Mar 14, 2022

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. Launch the URL https://staging.new.expensify.com/
  2. Log in with any account.
  3. Open any chat
  4. Refresh the page

Expected Result:

After page refresh user should stay on the chat conversation with recent user

Solution hints:

So if we break down the problem into steps considering #8126 (comment).

Manage the drawer state somehow #8126 (comment).
Able to navigate the state of the drawer via URL ?? (look at linkingConfig if you can find something).
we can think of A hybrid solution using react-navigation and Onyx.

Actual Result:

User redirected back to LHN

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.1.42-2
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Linked to #5027

Simulator.Screen.Recording.-.iPhone.13.-.2022-01-21.at.22.09.20.mp4

Expensify/Expensify Issue URL:
Issue reported by: @phivh
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1646153934144159

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @danieldoglas (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@danieldoglas danieldoglas added Improvement Item broken or needs improvement. External Added to denote the issue can be worked on by a contributor labels Mar 14, 2022
@MelvinBot
Copy link

Triggered auto assignment to @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@danieldoglas danieldoglas removed their assignment Mar 14, 2022
@danieldoglas
Copy link
Contributor

Seems like something related to the router.

@laurenreidexpensify laurenreidexpensify removed their assignment Mar 14, 2022
@laurenreidexpensify laurenreidexpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Mar 14, 2022
@MelvinBot
Copy link

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@laurenreidexpensify
Copy link
Contributor

@NicMendonca I'm out for a couple days, wanted to make sure someone picks this up before then - just saw this was I was logging off for day 😅

@NicMendonca
Copy link
Contributor

@MelvinBot
Copy link

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

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 14, 2022
@MelvinBot
Copy link

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

@mdneyazahmad
Copy link
Contributor

I reported this issue earlier but it did not get any response then https://expensify.slack.com/archives/C01GTK53T8Q/p1643463262081129

@parasharrajat

This comment was marked as off-topic.

@mollfpr
Copy link
Contributor

mollfpr commented Mar 16, 2022

Proposal

Make the drawer defaultStatus to closed on mWeb will prevent LHN to show. To make sure that this only applied to mWeb I add some condition below.

+    if (isSmallScreenWidth && Platform.OS === 'web') {
+        return 'closed';
+    }

function getDefaultDrawerState(isSmallScreenWidth) {
if (didTapNotificationBeforeReady) {
return 'closed';
}
return isSmallScreenWidth ? 'open' : 'closed';
}

Result

RPReplay_Final1647452539.MP4

@parasharrajat
Copy link
Member

Thanks for the proposal @mollfpr. Could you please tell me what the would be the output of? LHN or report screen?

  1. Open the App via localhost:8080 URL.
  2. Do 1 and refresh.
  3. Do 1 and click on any report, refresh the page.
  4. Open the app via localhost:8080/r/anyreportid
  5. Do 4 and refresh the app.
  6. Do 4 and use the back arrow to go back to LHN and refresh the app.

@mollfpr
Copy link
Contributor

mollfpr commented Mar 16, 2022

@parasharrajat

  1. It's open the recent report screen with reportID 90013490.
  2. It's show report screen with reportID 90013490.
  3. Number 1 shows the recent report screen with reportID 90013490, so I press the back button and select another report list which is reportID 90012313. After refresh is shown the recent report screen with reportID 90012313.
  4. It's open the report screen with reportID 90012313.
  5. It's show the report screen with reportID 90012313
  6. Show the latest report screen shown which is reportID 90012313

@JmillsExpensify
Copy link

Same

@parasharrajat
Copy link
Member

A lot of things have changed around the navigation in the App, and this is no more reproducible. @JmillsExpensify Can we please do the final settlement here and close this issue? Thank you.

@mountiny mountiny added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Monthly KSv2 Not a priority labels Mar 15, 2024
@mountiny mountiny changed the title [HOLD #11768][$8,000] Page goes to home screen if press chat and refresh the page - reported by @phivh [$500] Page goes to home screen if press chat and refresh the page - reported by @phivh Mar 15, 2024
Copy link

melvin-bot bot commented Mar 15, 2024

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

@mountiny
Copy link
Contributor

I think this is not repro anymore and we should close this out, i dont know if we need to pay anything out here

@mallenexpensify
Copy link
Contributor

Wow, this bug is over two years old and @mollfpr was hired close to two years ago 🤯
I'm going to close as it's not reproducible. @parasharrajat , @mollfpr , if you believe you're due compensation, can you please propose the amount, based on 25, 50 or 100% of the job price? Please provide reasoning too (ie. link the PR if one was created). We'll reopen this if needed.

@parasharrajat
Copy link
Member

parasharrajat commented Mar 22, 2024

I might be eligible for a C+ payment on this. I think 25-50% of the old amount would be fine.

Reasons:

  1. Proposals were reviewed.
  2. Testing on proposal updates was done multiple times.
  3. PR was reviewed.
  4. It required reviewing multiple navigation discussions to understand the feasibility of the code and tracking navigation refactors. This is the reason I held the PR.

cc: @mallenexpensify

@mollfpr
Copy link
Contributor

mollfpr commented Mar 22, 2024

I don't remember much of the detail but considering the length of the discussion on the proposal and the PR and how long the issue is held, 75% would be great! This was the highest job I ever got, so I would expect that I could finish the job at that time.

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Mar 24, 2024

Lot to unpack here. @JmillsExpensify , tagging you since you're the BZ and might have more context.

It looks like

I'm leaning towards 50% for each @mollfpr and @parasharrajat . It's quite a lot of money, $4k each, but we were increasing prices frequently a year+ ago and I don't think either should be penalized because this one stayed open for so long (and we're paying less now). @JmillsExpensify , @thienlnam, what do you think?

@parasharrajat
Copy link
Member

@JmillsExpensify Thoughts?

@melvin-bot melvin-bot bot added the Overdue label Apr 1, 2024
@JmillsExpensify
Copy link

I'm fine with @mallenexpensify's summary, though Matt can you please type out the payment summary. I can't approve my own summaries and both request in Expensify.

@melvin-bot melvin-bot bot removed the Overdue label Apr 3, 2024
@parasharrajat
Copy link
Member

@mallenexpensify Could you please look into above request and close it? Thanks.

@melvin-bot melvin-bot bot added the Overdue label Apr 5, 2024
@mallenexpensify
Copy link
Contributor

Contributor: @mollfpr due $4000 via NewDot
Contributor+: @parasharrajat due $4000 via NewDot

Thanks!

@parasharrajat
Copy link
Member

Payment requested as per #8126 (comment)

@JmillsExpensify
Copy link

$4,000 approved for @parasharrajat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests