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

[HOLD for payment 2024-03-07] [$500] Room - User is navigated to Concierge report when creating room #37234

Closed
6 tasks
kbecciv opened this issue Feb 26, 2024 · 20 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production DeployBlockerCash This issue or pull request should block deployment Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Feb 26, 2024

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


Version Number: v1.4.44-0
Reproducible in staging?: y
Reproducible in production?: n
Issue reported by: Applause - Internal Team

Action Performed:

  1. Create a workspace if you don't have any
  2. Click on 'FAB' > Start chat > Room tab
  3. Enter room name and click on 'Create room' button

Expected Result:

User should be navigated to the newly created room

Actual Result:

User is navigated to Concierge report

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

Add any screenshot/video evidence

Bug6393358_1708984329114.Screen_Recording_2024-02-27_at_12.26.11_at_night.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0146027f79fc978115
  • Upwork Job ID: 1762246166149222400
  • Last Price Increase: 2024-02-26
@kbecciv kbecciv added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor labels Feb 26, 2024
@melvin-bot melvin-bot bot changed the title Room - User is navigated to Concierge report when creating room [$500] Room - User is navigated to Concierge report when creating room Feb 26, 2024
Copy link

melvin-bot bot commented Feb 26, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0146027f79fc978115

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 26, 2024
Copy link

melvin-bot bot commented Feb 26, 2024

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Feb 26, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Feb 26, 2024
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.

@kbecciv
Copy link
Author

kbecciv commented Feb 26, 2024

We think that this bug might be related to #vip-vsb
|CC @quinthar

Copy link

melvin-bot bot commented Feb 26, 2024

Triggered auto assignment to @blimpich (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@blimpich
Copy link
Contributor

Looking into this

@dukenv0307
Copy link
Contributor

dukenv0307 commented Feb 26, 2024

Proposal

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

  • Room - User is navigated to Concierge report when creating room

What is the root cause of that problem?

  • We navigate user to concierge in this line because of the condition is true.
  • It is because after creating room, we navigate to report and we call the openReport API which is redundant (this does not appear in prod). When calling openReport, we set the optimistic report to something like {reportName: ChatReport}
  • But why openReport is called again? It is because we render the report screen when the report data is not fetched successfully

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

  • In here, we can remove allowStaleData. So it can make sure the report data is fetched before rendering the report screen

Alternative solution

  • Also, we just need to remove !report.statusNum && !prevReport.parentReportID && prevReport.chatType === CONST.REPORT.CHAT_TYPE.POLICY_ROOM

@blimpich
Copy link
Contributor

@dukenv0307 This is a regression though, right? What PR caused this?

@blimpich
Copy link
Contributor

blimpich commented Feb 27, 2024

My guess is something happened such that the props getting passed down into ReportScreen are wonky now, though I'm having a hard time finding which PR caused this.

@blimpich
Copy link
Contributor

@dukenv0307
Copy link
Contributor

dukenv0307 commented Feb 27, 2024

@blimpich based on my proposal, we render the report screen when the report data is not fetched successfully, that leads to the bug. What do you think about it? Also, it is very difficult to find the PR that caused the regression on my side

@blimpich
Copy link
Contributor

blimpich commented Feb 27, 2024

@dukenv0307 This is a regression though, right? What PR caused this?

This again. Something broke the app, and in order to accept your proposal I would want to know why this started happening in the first place. This isn't happening in production so this is a recent bug.

@s77rt
Copy link
Contributor

s77rt commented Feb 27, 2024

Timeline after we press the create room button

  1. Policy.addPolicyReport is called and it sets the optimistic report
  2. Report.openReport is called and it overrides the optimistic report <- This should not happen

The because chain:

  • Report.openReport is called in ReportScreen fetchReportIfNeeded because isLoadingInitialReportActions is true.
  • isLoadingInitialReportActions is true because reportActions are empty.
  • reportActions are empty because withOnyx subscription returns undefined
  • withOnyx subscription returns undefined due to a recent change Use initialValue when cached value is undefined react-native-onyx#470 I'm not sure why exactly but it seems to contradicts with the selector

Offending PRs:

@bernhardoj
Copy link
Contributor

@s77rt is on point. This has the same root cause as this issue

@melvin-bot melvin-bot bot added the Weekly KSv2 label Feb 27, 2024
Copy link

melvin-bot bot commented Feb 27, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@bernhardoj
Copy link
Contributor

The PR is here #37269

@blimpich
Copy link
Contributor

Reverted the react Onyx change by first reverting the react-onyx package and then upgrading the App's package to the latest version. Thank you @s77rt for the explanation!

And I'm sorry @bernhardoj the timing didn't work out with your fix. Its morning over here and by the time I started work we really wanted to get the deploy out, so it makes sense to do this revert given the time / risk involved. It always sucks when making seemingly straightforward changes has these unforeseeable cascading effects.

Closing as #37323 fixes this.

@luacmartins
Copy link
Contributor

Seems like this was also the root cause for #37300

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Feb 29, 2024
@melvin-bot melvin-bot bot changed the title [$500] Room - User is navigated to Concierge report when creating room [HOLD for payment 2024-03-07] [$500] Room - User is navigated to Concierge report when creating room Feb 29, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 29, 2024
Copy link

melvin-bot bot commented Feb 29, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Feb 29, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.45-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-03-07. 🎊

For reference, here are some details about the assignees on this issue:

  • @Pujan92 requires payment (Needs manual offer from BZ)

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 DeployBlockerCash This issue or pull request should block deployment Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants