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-09-17][$250] Web - Concierge - Concierge in unavailable workspace when create new account #44480

Closed
1 of 6 tasks
lanitochka17 opened this issue Jun 26, 2024 · 64 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 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: 9.0.2-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: N/A
Email or phone of affected tester (no customers): gocemate+a444@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Login with a new Gmail account
  3. Complete the onboarding modal requests
  4. Take a look at the Concierge chat header

Expected Result:

There should be only Concierge label on Concierge chat header

Actual Result:

"Concierge in unavailable workspace" can be seen on the Concierge chat header after creating a new account

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

14!image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013152861e0170f045
  • Upwork Job ID: 1806787876528252448
  • Last Price Increase: 2024-09-03
  • Automatic offers:
    • allgandalf | Reviewer | 103818556
    • wildan-m | Contributor | 103818557
Issue OwnerCurrent Issue Owner: @cristipaval
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@lanitochka17
Copy link
Author

@sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@melvin-bot melvin-bot bot added the Overdue label Jun 28, 2024
@sakluger sakluger added the External Added to denote the issue can be worked on by a contributor label Jun 28, 2024
@melvin-bot melvin-bot bot changed the title Web - Concierge - Concierge in unavailable workspace when create new account [$250] Web - Concierge - Concierge in unavailable workspace when create new account Jun 28, 2024
Copy link

melvin-bot bot commented Jun 28, 2024

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

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

melvin-bot bot commented Jun 28, 2024

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

@jainilparikh
Copy link

jainilparikh commented Jun 30, 2024

Edited by proposal-police: This proposal was edited at 2024-08-15 07:29:48 UTC.

Proposal

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

'Unavailable workspace ' appears when a User attempts to login with a fresh Gmail account.

What is the root cause of that problem?

Within HeaderView.ts we invoke 1getPolicyName function to get the sub-heading. Note: We send true for returnEmptyIfNotFound flag.

https://github.com/Expensify/App/blob/main/src/pages/home/HeaderView.tsx#L113

Within the getPolicyName function, when there report has no policies (This happens when it's a new user), we don't respect the returnEmptyIfNotFound flag.

https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.ts#L745

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

Replace

if ((!allPolicies || Object.keys(allPolicies).length === 0) && !report?.policyName) {
        return unavailableTranslation;
    }

With

if ((!allPolicies || Object.keys(allPolicies).length === 0) && !report?.policyName) {
        return noPilicyFound;
}

The noPolicyFound variable (link) take's care of respecting the returnEmptyIfNotFound flag.

Copy link

melvin-bot bot commented Jul 2, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jul 2, 2024
@allgandalf
Copy link
Contributor

@jainilparikh can you dig in more? I don't think your solution is correct, maybe git blame and check why that text was added there at the first place? thanks

@melvin-bot melvin-bot bot removed the Overdue label Jul 2, 2024
@jainilparikh
Copy link

Hi @allgandalf, Sorry if my proposal is confusing, but to be clear, I am not asking to remove the text, my solution states that we replace the text with a variable noPolicyFound . That variable decides which text to use based on returnEmptyIfNotFound :

https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.ts#L659

Copy link

melvin-bot bot commented Jul 5, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Jul 5, 2024
@allgandalf
Copy link
Contributor

@jainilparikh i am asking why do we return Localize.translateLocal('workspace.common.unavailable') at the first place? can you check why that check was added at the first place

@melvin-bot melvin-bot bot removed the Overdue label Jul 5, 2024
Copy link

melvin-bot bot commented Jul 8, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jul 8, 2024
@allgandalf
Copy link
Contributor

bump to @jainilparikh to address the comment ^, I will post on the Open Source channel if i don't get any proposal next time melvin add overdue

@melvin-bot melvin-bot bot removed the Overdue label Jul 9, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

@sakluger @allgandalf this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@sakluger
Copy link
Contributor

sakluger commented Sep 4, 2024

@allgandalf do you mind posting "🎀👀🎀 C+ reviewed" again to get a new engineer auto-assigned? Thanks!

@allgandalf
Copy link
Contributor

sure thing @sakluger ! was about to suggest that….

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Sep 4, 2024

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

@allgandalf
Copy link
Contributor

allgandalf commented Sep 4, 2024

sweet, I guess this is our first interaction @mjasikowski , hope to work on more PR’s ❤️

You can find the assignment summary here

@lschurr
Copy link
Contributor

lschurr commented Sep 4, 2024

Gonna remove myself since you're back Sash :)

@lschurr lschurr removed their assignment Sep 4, 2024
@mjasikowski
Copy link
Contributor

Thank you for the summary @allgandalf. The proposal by @wildan-m looks good, we can go ahead with this one.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

Copy link

melvin-bot bot commented Sep 4, 2024

📣 @wildan-m 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@wildan-m
Copy link
Contributor

wildan-m commented Sep 5, 2024

@allgandalf The PR is ready #48621

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Sep 5, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week)

@allgandalf
Copy link
Contributor

Can you add payment awaiting label here @sakluger , the PR hit production long back

@sakluger sakluger changed the title [$250] Web - Concierge - Concierge in unavailable workspace when create new account [HOLD for payment 2024-09-17][$250] Web - Concierge - Concierge in unavailable workspace when create new account Sep 20, 2024
@sakluger
Copy link
Contributor

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@allgandalf] The PR that introduced the bug has been identified. Link to the PR:
  • [@allgandalf] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@allgandalf] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@allgandalf] Determine if we should create a regression test for this bug.
  • [@allgandalf] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@sakluger
Copy link
Contributor

Summarizing payment on this issue:

Contributor: @wildan-m $250, paid via Upwork
Contributor+: @allgandalf $250, paid via Upwork

@allgandalf
Copy link
Contributor

allgandalf commented Sep 21, 2024

  • The PR that introduced the bug has been identified. Link to the PR: Implement report approvals #24639
  • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/24639/files#r1769583401
  • A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
  • If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

  1. Login with a new Gmail account
  2. Complete the onboarding modal requests
  3. Take a look at the Concierge chat header
  • Verify that there is only Concierge label on Concierge chat header

Do we agree 👍 or 👎

@sakluger
Copy link
Contributor

Thanks!

@m-natarajan m-natarajan self-assigned this Oct 14, 2024
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. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Status: No status
Development

No branches or pull requests