[HOLD for payment 2024-09-17][$250] Web - Concierge - Concierge in unavailable workspace when create new account

[lanitochka17]

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
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

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 Owner

Current Issue Owner: @cristipaval

[melvin-bot[bot]]

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]

@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[bot]]

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

[melvin-bot[bot]]

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

[jainilparikh]

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.

[melvin-bot[bot]]

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

[allgandalf]

@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

[jainilparikh]

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

[melvin-bot[bot]]

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

[allgandalf]

@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[bot]]

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

[allgandalf]

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[bot]]

@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]

Issue not reproducible during KI retests. (First week)

[sakluger]

I also was unable to reproduce this. Closing.

[IuliiaHerets]

Issue is still reproducible on the latest build v9.0.19-9, unavailable workspace appears for a moment before the Onboarding modal opens

https://github.com/user-attachments/assets/122db380-4bb0-41d9-b0a4-a8ff4ca1b920

[melvin-bot[bot]]

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

[sakluger]

@jainilparikh are you still interested in working on this one?

[allgandalf]

If they are interested then @jainilparikh can you please try to address this comment, thanks 🙏

[jainilparikh]

Yes, taking a look sorrry for the late response.

[jainilparikh]

So, to answer your query:

@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

This was added as a default response by the original author of the getPolicyName function ( I don't think the author had any other intentions for this string other than acting as a fallback when there are no policies):

https://github.com/Expensify/App/pull/8473/files#diff-82f2652821babb48b600a87486fa1191db7b3cc300845573a5757676e13cfc26R186

The original author just assumed that sending 'workspace.common.unavailable' would be the right response when there is no policy associated with a user. Multiple refactors were done after this, but in all of them, this default response was untouched.

If we were to go futher back in history, this string was called 'unknown Policy': https://github.com/Expensify/App/pull/7957/files#diff-82f2652821babb48b600a87486fa1191db7b3cc300845573a5757676e13cfc26L176 Which was replaced with 'unavailable workspace'.

TLDR: I don't think replacing it will cause a regression.

Please let me know if there are any other questions. Thanks!

[jainilparikh]

@allgandalf , any further questions ?

[allgandalf]

I will review this today ♻️

[melvin-bot[bot]]

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

[sakluger]

@allgandalf any updates?

[allgandalf]

Okay the issue is only for the first time when we login with new account, it doesn't happen when we complete onboarding, it only happens in the transition of log in and the onboarding modal appears:

https://github.com/user-attachments/assets/7346423e-a63a-46e9-9bf1-6256ea72c738

https://github.com/user-attachments/assets/a1792cd6-27e1-44ab-bfd5-726092175d7b

Also @jainilparikh's RCA is not correct, we return noPolicyFound only when the report is empty object and when we create a new account for a second report is not considered as empty object, so I will post on slack for some 👀 and also would like if @jainilparikh can dig deeper into this

[wildan-m]

@allgandalf if someone can better explain the root cause but ended up with similar other proposal solution, which proposal will be selected?

[b4s36t4]

Edited by proposal-police: This proposal was edited at 2023-10-07T14:19:29Z.

Proposal

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

Web - Concierge - Concierge in unavailable workspace when create new account

What is the root cause of that problem?

The real problem relies in the ReportScreen component where we're rendering HeaderView component.

Inside ReportScreen component we're rendering HeaderView with a prop report which is a object derives from object read from onyx

https://github.com/Expensify/App/blob/06f5b66c8fe2d9be62bc008d67a33dcdf1b4484c/src/pages/home/ReportScreen.tsx#L327

https://github.com/Expensify/App/blob/15805221bdd48e88bc39991895cbb26172bb106e/src/pages/home/ReportScreen.tsx#L169-L255

The case which the issue is reproducible is where this object contains all the keys but all the values inside are undefined in which it passes the first if condition inside getReportName function.

Now after the if we have another condition where we're checking the polices from onyx and verifying that the report isn't from one of the policies.

When we create a new account, allReports will be empty and report.policyName is also undefined which qualifies this condition and here we're returning the Unavailable Workspace string.

https://github.com/Expensify/App/blob/ed910fa1cae6597ea317f24bd7d4d60e4f6f4d5e/src/libs/ReportUtils.ts#L746

This happens with-in micro-seconds of time diff on writing the data to onyx and component re-render.

To summarize the first if condition is failing because we're not sending an empty object from ReportScreen whereas the 2nd condition is getting succeed because of the time-diff of getting updated data i.e report and allReports which is giving the unavailableTranslation.

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

We can simply return noPolicyFound from 2nd if condition which will return empty string in this cause because returnEmptyIfNotFound is true.

We need to make sure the returnEmptyIfNotFound is respected everywhere in the codebase to make sure we don't cause any regressions.

What alternative solutions did you explore? (Optional)

Currently we have policyId condition _FAKE_ value in-case of empty policy related to report. We can also do same for policyName and return _FAKE_ value from ReportScreen component and have a condition inside getReportName function to handle it carefully.

We should only send _FAKE_ value if the policyName doesn't exist from report which doesn't exist for conceirge and expensify's report.

[b4s36t4]

Gentle bump @allgandalf for proposal review :)

[wildan-m]

Proposal

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

Web Concierge is unavailable for a brief moment when creating a new account.

What is the root cause of that problem?

On the initial login, there is a moment that report and policies are being loaded and we call getPolicyName

When the policies empty and loading then it will enter this check:

https://github.com/Expensify/App/blob/844c6bf01829b88b7392cdc330a787a6366c8cec/src/libs/ReportUtils.ts#L745-L747

This can happen for new account or existing one.

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

Add !report.reportID || isEmptyObject(allPolicies) to this check:

https://github.com/Expensify/App/blob/844c6bf01829b88b7392cdc330a787a6366c8cec/src/libs/ReportUtils.ts#L741

We can also refactor (!allPolicies || Object.keys(allPolicies).length === 0) to isEmptyObject(allPolicies) here:

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

What alternative solutions did you explore? (Optional)

Alternative 1 define isConcierge chat as one of kind of chat that not need to display renderAdditionalText Chang this code to:

    const renderAdditionalText = () => {
        if (isConciergeChat || shouldShowSubtitle() || isPersonalExpenseChat || !policyName || !isEmptyObject(parentNavigationSubtitleData) || isSelfDM) {            
            return null;
        }

Alternative 2 I am unsure when the unavailable workspace is supposed to appear and cannot find sufficient information, but I suggest removing that option entirely and displaying an empty string for all cases where it is not found.

Alternative 3 Similar to prev proposals but with optimization to condition

in getPolicyName

    const noPolicyFound = returnEmptyIfNotFound ? '' : unavailableTranslation;
    if (isEmptyObject(report)||(isEmptyObject(allPolicies) && !report?.policyName) ) {
        return noPolicyFound;
    }

    const finalPolicy = policy ?? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`];
    const parentReport = getRootParentReport(report);

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[sakluger]

Hey @allgandalf could you please review the outstanding proposals?

I'm also going to add another BZ member to monitor while i'm OOO (Back 8/30).

[melvin-bot[bot]]

Triggered auto assignment to @lschurr (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.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[lschurr]

Any update here @allgandalf?

[lschurr]

Bumped in Slack as well - https://expensify.slack.com/archives/C01GTK53T8Q/p1724859115299129

[allgandalf]

Sorry, didn't show up on K2, I will review this one today/tomorrow

[allgandalf]

Long overdue, reviewing today ♻️

[allgandalf]

Thanks for the proposals everyone,

@jainilparikh:

Thanks for the proposal, but your RCA was not clear and correct from the start.

---

@b4s36t4 & @wildan-m , I appreciate you guys digging deep into the RCA, the root cause is much clear now.

Although both @b4s36t4 and @wildan-m proposed solution would solve the bug, I would like to go with @wildan-m main solution here, the proposed condition makes more sense to me

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[allgandalf]

bump to @cristipaval 🙇

[melvin-bot[bot]]

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

[allgandalf]

friendly bump @cristipaval

[allgandalf]

NO melvin, not overdue 😭

[lschurr]

@cristipaval is out on leave - going to put this in Slack to get another Engineer added.

[sakluger]

Let's just auto-assign a new engineer.

[melvin-bot[bot]]

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

[sakluger]

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

[allgandalf]

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

🎀👀🎀 C+ reviewed