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-10-25] [$250] Onboarding - New user lands on workspace chat when 'Manage my team's expenses' selected in modal #50578

Open
2 of 6 tasks
lanitochka17 opened this issue Oct 10, 2024 · 25 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 10, 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: 9.0.47-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5070660
Issue reported by: Applause - Internal Team

Action Performed:

  1. Open New Expensify app
  2. Log in with new gmail user
  3. Select 'Manage my team's expenses' in onboarding modal
  4. Enter workspace name, user name and submit modal

Expected Result:

Either Concierge chat or LHN should be displayed

Actual Result:

Workspace chat is 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

Add any screenshot/video evidence
Bug6630791_1728569605765.OHBK2619.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021844427822228964480
  • Upwork Job ID: 1844427822228964480
  • Last Price Increase: 2024-10-10
Issue OwnerCurrent Issue Owner: @ikevin127
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Oct 10, 2024
Copy link

melvin-bot bot commented Oct 10, 2024

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

Copy link

melvin-bot bot commented Oct 10, 2024

💬 A slack conversation has been started in #expensify-open-source

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.

@allgandalf
Copy link
Contributor

allgandalf commented Oct 10, 2024

Our PR touched the onboarding flow recently, looking into it.. will update in sometime! @MarioExpensify

@allgandalf
Copy link
Contributor

@twilight294 , please check on your end too, if this is a regression from our PR, you need to fix it too

@allgandalf
Copy link
Contributor

@MarioExpensify I reverted our PR and tested it here, but the bug is still reproducible, so not related to our PR, but i would love to help you on this one, I will stay subscribed to this thread:

Screen.Recording.2024-10-10.at.8.08.46.PM.mov

@MarioExpensify
Copy link
Contributor

Trying to track down which PR caused the issue right now

@allgandalf
Copy link
Contributor

Cool, our washttps://github.com/Expensify/App/pull/50359, not linking cause it makes melvin to notify 😅

@allgandalf
Copy link
Contributor

Crap, it did anyway 💢

@MarioExpensify
Copy link
Contributor

Still haven't been able to come up with a fix for this one, @allgandalf feel free to hop in

@MarioExpensify MarioExpensify added the External Added to denote the issue can be worked on by a contributor label Oct 10, 2024
@melvin-bot melvin-bot bot changed the title Onboarding - New user lands on workspace chat when 'Manage my team's expenses' selected in modal [$250] Onboarding - New user lands on workspace chat when 'Manage my team's expenses' selected in modal Oct 10, 2024
Copy link

melvin-bot bot commented Oct 10, 2024

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

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

melvin-bot bot commented Oct 10, 2024

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

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 10, 2024

Edited by proposal-police: This proposal was edited at 2024-10-10 17:56:10 UTC.

Proposal

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

Workspace chat is displayed

What is the root cause of that problem?

This comes from #50278

We always navigate to the last accessed report here. In this case when we select Manage my team's expense, the policy expense chat is created and it's the last access report

const lastAccessedReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID)?.reportID;
if (!lastAccessedReportID) {
return;
}

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

I checked and this logic is used when the user signs in with a public room. Then we can check if the lastAccessedReport is the report from the onboardingPolicyID we will navigate to concierge chat otherwise navigate to the last access report

const lastAccessedReport = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID);
const lastAccessedReportID = lastAccessedReport?.reportID
if (!lastAccessedReportID || lastAccessedReport?.policyID === onboardingPolicyID) {
    // We can return early or navigate to concierge here
    Report.navigateToConciergeChat();
    return;
}
const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? '-1');
Navigation.navigate(lastAccessedReportRoute);

const lastAccessedReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID)?.reportID;
if (!lastAccessedReportID) {
return;
}

What alternative solutions did you explore? (Optional)

Or we can always navigate to the concierge chat as we do in the past

@allgandalf
Copy link
Contributor

We are reverting the offending PR here: #50596

@ikevin127
Copy link
Contributor

Given that the offending PR was reverted, this issue should be fixed - not an issue anymore.

@MarioExpensify MarioExpensify removed 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 labels Oct 10, 2024
@thienlnam thienlnam removed the DeployBlockerCash This issue or pull request should block deployment label Oct 10, 2024
@MarioExpensify MarioExpensify added Daily KSv2 and removed Hourly KSv2 labels Oct 10, 2024
@mvtglobally
Copy link

@wildan-m
Copy link
Contributor

Hi, does anyone know why I have different flow?

I've overridden canUseAllBetas, but it remains different.

cc: @thesahindia, @lakchote

Kapture.2024-10-11.at.10.23.32.mp4

@lakchote
Copy link
Contributor

Hi, does anyone know why I have different flow?

I've overridden canUseAllBetas, but it remains different.

cc: @thesahindia, @lakchote

You didn't use a gmail account to sign up.

It's written in the issue's reproduction steps.

@wildan-m
Copy link
Contributor

@lakchote I've tried using gmail, but its flow still different compared to the reproduction steps above. And can't reproduce the issue even when my prev PR applied.

Kapture.2024-10-11.at.15.22.35.mp4

@MarioExpensify
Copy link
Contributor

I'll close the issue, feel free to continue the discussion in it, it is just to clear up my K2.

@lakchote
Copy link
Contributor

lakchote commented Oct 11, 2024

I'll close the issue, feel free to continue the discussion in it, it is just to clear up my K2.

@MarioExpensify please do not close it, as we still have a discussion ongoing and the problem hasn't found resolution yet.

If it's to clear up your K2, just unassign yourself. Thanks!

@lakchote lakchote reopened this Oct 11, 2024
@lakchote
Copy link
Contributor

@wildan-m yes I've been able to reproduce (on your original PR's branch):

Screen.Recording.2024-10-11.at.18.10.24.mov

@thesahindia
Copy link
Member

Ok so it is reproducible when we test on the original PR branch, but after the new changes made by #49161 it's not reproducible anymore.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 Weekly KSv2 labels Oct 12, 2024
@melvin-bot melvin-bot bot changed the title [$250] Onboarding - New user lands on workspace chat when 'Manage my team's expenses' selected in modal [HOLD for payment 2024-10-25] [$250] Onboarding - New user lands on workspace chat when 'Manage my team's expenses' selected in modal Oct 18, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 18, 2024
Copy link

melvin-bot bot commented Oct 18, 2024

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

Copy link

melvin-bot bot commented Oct 18, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.50-8 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-10-25. 🎊

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

  • @wildan-m requires payment (Needs manual offer from BZ)
  • @thesahindia requires payment through NewDot Manual Requests

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 Engineering Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants