Workspace - Inconsistency between workspace default currency and explanation message below

[kbecciv]

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

---

Action Performed:

1. Go to https://staging.new.expensify.com/
   and login
2. Login with applausetester+1014pronin@applause.expensifail.com
3. Go to workspaces and select each available one by one
4. Compare default currencies with explanation message below

Expected Result:

User would expect the if explanation message below says "The default currency can't be changed because this workspace is linked to a USD bank account.", then it has something to do with USD and most probably default currency should be USD

Actual Result:

Inconsistency between workspace default currency and explanation message below
DO NOT DELETE Long workspace has Default currency EUR, which cannot be changed because "workspace is linked to a USD bank account."

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.92.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

Bug5998286_video_70.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01543b8dc1e5674c99
• Upwork Job ID: 1651528735022252032
• Last Price Increase: 2023-04-27

[MelvinBot]

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

[MelvinBot]

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

[greg-schroeder]

Hey @kbecciv - can you clarify - does this issue need to be reproduced specifically with the applause account listed in the OP, or can it be reproduced with any account? I'm not sure how to try reproducing w/ an applause account if it's the former. 🤔

[kbecciv]

@greg-schroeder It can be reproduce with any expensifail accounts.

[Prince-Mendiratta]

Proposal

Posting proposal early as per new guidelines

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

In this issue, the default currency for a workspace with a linked bank account might not be USD but the hint below mentions that it is in USD, even when it might not be.

What is the root cause of that problem?

This might be a missed case from #15877 with the detail that the linked bank account will always be in USD being an assumption.

The text workspace.editor.currencyInputDisabledText has the currency value USD hard coded, which does not take into account the default value of the currency.

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

We should convert workspace.editor.currencyInputDisabledText to a function takes the default currency as the input and displays the hint accordingly. The default value can be this.props.policy.outputCurrency.

What alternative solutions did you explore? (Optional)

[twisterdotcom]

I'm unable to reproduce this. My Workspaces in non-USD currencies don't show that message:

[Prince-Mendiratta]

@twisterdotcom to reproduce this specifc issue, you will have to set the default currency to GBP, save it, then link a bank account and it'll show up like that

[twisterdotcom]

Ah, I think we should block adding a VBA for currencies we don't support reimbursement for then. That's what we do on oldDot.

[Prince-Mendiratta]

I'm not too sure if this is because of linking a VBA with a currency other than USD. I think this is an issue of stale data.

Assume that a bank account has not been linked. I set the default currency to INR and that is saved in the backend. Without changing the default currency, I add a VBA that uses USD. However, now that I have already saved the default currency as INR, the field is disabled and I cannot change default currency to USD. This is the issue of stale data.

I can't test it though since I do not have the ability to add VBAs but I think we can infer that based on which VBA is linked to the workspace in the OP. I believe @kbecciv should be able to help with that and we can proceed accordingly.

[dukenv0307]

Proposal

We updated CONTRIBUTING.md to allow posting proposals before help wanted label this PR and this PR is merged so contributors can post proposal before help wanted label
Please re-state the problem that we are trying to solve in this issue.

Inconsistency between workspace default currency and explanation message below

What is the root cause of that problem?

currencyInputDisabledText is a varibale that is explanation message in WorkspaceSettingsPage.js. currencyInputDisabledText is constant, it doesn't depend on default currency. So, with any default currency the explanation message below always is "The default currency can't be changed because this workspace is linked to a USD bank account.". Thus this issue occurs.

App/src/languages/en.js

Line 1054 in 56c27de

currencyInputDisabledText: 'The default currency can\'t be changed because this workspace is linked to a USD bank account.',

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

Change currencyInputDisabledText to function. This function has parameter outputCurrency that is default currency of the workspace and return string that is the explanation message. In the resulting string, replace "USD" with outputCurrency passed in. And then in WorkspaceSettingsPage.js pass a object has key is outputCurrency and value is this.props.policy.outputCurrency to this.props.translate('workspace.editor.currencyInputDisabledText').

currencyInputDisabledText: ({ outputCurrency }) => `The default currency can't be changed because this workspace is linked to a ${outputCurrency} bank account.`,

currencyInputDisabledText: ({ outputCurrency }) => `TLa moneda predeterminada no se puede cambiar porque este espacio de trabajo está vinculado a una cuenta bancaria en ${outputCurrency}`,

What alternative solutions did you explore? (Optional)

[twisterdotcom]

Missed this comment but I also think this shouldn't be possible. If your default currency isn't USD, you should not be able to add a VBA to the workspace to get into this situation. You can't do anything with the VBA on that workspace because you can't reimburse reports that aren't in USD.

Without changing the default currency, I add a VBA that uses USD

[greg-schroeder]

Looks like part 1 is merged, part 2 is up next

[melvin-bot]

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

[melvin-bot]

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

• Ask non-USD Workspaces to update currency before adding VBBA #19638

If no regressions arise, payment will be issued on 2023-06-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

https://www.upwork.com/jobs/~01f2f82a91ed7fcc33

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@rushatgabhane] The PR that introduced the bug has been identified. Link to the PR:
• [@rushatgabhane] 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:
• [@rushatgabhane] 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:
• [@rushatgabhane] Determine if we should create a regression test for this bug.
• [@rushatgabhane] 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.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[greg-schroeder]

Sent you an offer on Upwork @rushatgabhane

[greg-schroeder]

Pending offer acceptance and checklist completion

[rushatgabhane]

Hi @greg-schroeder, we can settle this over new dot now.
We've made 2 successful manual requests. We just have to wait 1 more week to see all goes well.

Then I'll raise a manual request for it. Meanwhile, you could make this issue a weekly? So it doesn't bother you everyday

[greg-schroeder]

Ohhh. I heard about that. I wasn't sure if we were actually live with that yet?

[rushatgabhane]

we're live!

https://expensify.slack.com/archives/C02NK2DQWUX/p1687970684314839?thread_ts=1686871517.189609&cid=C02NK2DQWUX

[rushatgabhane]

Actually, we should remove payment labels. There's a 2nd PR coming for this issue #16744 (comment)

[greg-schroeder]

Okay, holding for second PR. I'll cancel the offer in Upwork as well

[greg-schroeder]

hey @luacmartins - just checking in - is part 2 for this one still on the radar?

[luacmartins]

Yes, I've been focused on the manual requests delete functionality. I just need to squash some more bugs there and then this is next on my list.

[luacmartins]

I was thinking of closing this issue and creating a new one though, since the scope of part 2 is quite different than this OP. I also don't think we need to hold payment here since part 1 is done.

[rushatgabhane]

Okay sounds good.

@greg-schroeder no action required by you for payment. Anu will settle it on new dot

[luacmartins]

Created an issue for part 2 here. We can move this one along and close it once payments have been issued

[greg-schroeder]

Okay closing per above comment!