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] Web - Login - Error page is displayed after changing the long URL to staging and go to it #29849

Closed
1 of 6 tasks
izarutskaya opened this issue Oct 18, 2023 · 22 comments
Closed
1 of 6 tasks
Assignees
Labels
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 Overdue

Comments

@izarutskaya
Copy link

izarutskaya commented Oct 18, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR: #28984

Version Number: 1.3.86-1

Reproducible in staging?: Y

Reproducible in production?: N

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

Expensify/Expensify Issue URL:

Issue reported by: Applause-Internal Team

Slack conversation: @

Action Performed:

Pre-requisite: user must be logged in with account A on expensify.com and account B on new.expensify.com

  1. With account B on expensify.com go to Support and select Concierge.
  2. A new tab will open with a long URL, copy it.
  3. Change the URL to staging.new.expensify.com and go to it.

Expected Result:

User A should be successfully logged in to ND with no error pages displayed.

Actual Result:

User A is logged in but there is an error page displayed.

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

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Bug6241329_1697603277658.bandicam_2023-10-17_21-59-49-598.mp4
MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b109095a56a932b9
  • Upwork Job ID: 1714580146381455361
  • Last Price Increase: 2023-10-18
  • Automatic offers:
    • cubuspl42 | Reviewer | 27258677
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 18, 2023
@melvin-bot melvin-bot bot changed the title Web - Login - Error page is displayed after changing the long URL to staging and go to it [$500] Web - Login - Error page is displayed after changing the long URL to staging and go to it Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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 melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@izarutskaya
Copy link
Author

Not reproducible in production

20231018_114714.mp4

@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@hungvu193
Copy link
Contributor

It was intended to be fixed by this PR #28984
However I still can reproduce this issue. So I think it can be regression from it.

@koko57
Copy link
Contributor

koko57 commented Oct 18, 2023

yes, it's also reproducible locally. Not sure why the id for Concierge chat changes to the id for Concierge chat from the previously log out account
Screenshot 2023-10-18 at 14 05 02
Screenshot 2023-10-18 at 14 03 48
Screenshot 2023-10-18 at 14 03 33
Screenshot 2023-10-18 at 14 03 39

it tested well before merging my PR, so maybe some other changes were introduced that caused the issue with mine

I'll investigate it

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@mountiny
Copy link
Contributor

Thanks for investigating, maybe we can also confirm if revert fixes this, there could be some other change

@mountiny
Copy link
Contributor

I think the steps should say With account A on expensify.com go to Support and select Concierge. right?

@koko57
Copy link
Contributor

koko57 commented Oct 18, 2023

reverting my changes gives me totally black screen - not like in the issue I tried to fix (an empty sidebar and green background)
Screenshot 2023-10-18 at 15 03 45

@Beamanator
Copy link
Contributor

Hey @koko57 - has there been any more investigation in the last 4 hours? 😅 I didn't see that I am assigned to look into this as well 😳

@koko57
Copy link
Contributor

koko57 commented Oct 18, 2023

When I removed demoInfo key from withOnyx in AuthScreens (with my changes reverted) I got the same outcome as in the issue I was fixing (green background, empty sidebar). Then I restored the changes and noticed that at first the user is redirected to the correct chat (the one with id from the transition url) but right afterwards there's a redirection to this improper chat. I haven't found the reason why this happens. I'll continue investigating it tomorrow

@francoisl francoisl removed the DeployBlockerCash This issue or pull request should block deployment label Oct 18, 2023
@francoisl
Copy link
Contributor

Discussed in Slack in this thread, this is an edge case so we'll not block the deploy for this.
I also got a crash on production when I tried to reproduce this, so there might be an issue there anyway.

@francoisl francoisl added Daily KSv2 and removed Hourly KSv2 labels Oct 18, 2023
@koko57
Copy link
Contributor

koko57 commented Oct 19, 2023

Reverting this empty string to null in this line fixes the issue 😃

return String(lodashGet(route, 'params.reportID', ''));

and seems that it doesn't introduce any regression.
However, it only unveiled some other problems with the transition. Starting working on my proposal

@koko57
Copy link
Contributor

koko57 commented Oct 20, 2023

I've merged with the latest main and '' in this line

// // The reportID is used inside a collection key and should not be empty, as an empty reportID will result in the entire collection being returned.
return String(lodashGet(route, 'params.reportID', null));
was changed back to null so locally there's no problem with the transtition.

Screen.Recording.2023-10-20.at.17.48.50.mov

for the transition to staging there's still a problem, but it's because this PR hasn't been deployed to staging yet.

@mountiny
Copy link
Contributor

@koko57 thank you! would you be able to make a new PR?

@koko57
Copy link
Contributor

koko57 commented Oct 20, 2023

@mountiny do we need a new PR here? This PR fixes this

@mountiny
Copy link
Contributor

ah yes sorry I forgot we did not end up reverting your PR, so then we are all good

@Beamanator
Copy link
Contributor

Ok so we can close this one out, yeah?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 Overdue
Projects
None yet
Development

No branches or pull requests

9 participants