[$1000] WS - Default currency field isn't disabled when account have Control Policy

[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!

---

Issue found when executing PR #16449

Action Performed:

1. Launch the app
2. Log in with expensifail account which has Control policy
3. Go to Settings - Workspace - Tap on Avatar
4. Check Workspace Settings

Expected Result:

Verify that the default currency field is disabled and has styles matching this comment.

Actual Result:

The default currency field isn't disabled when the account have Control Policy

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.1

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

Bug5999182_WS.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/~0187a24a9f8cb94f28
• Upwork Job ID: 1643189704464609280
• Last Price Increase: 2023-04-11

[MelvinBot]

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

[allroundexperts]

@laurenreidexpensify @kbecciv Can you also mention an account with control policy enabled?

[MelvinBot]

@laurenreidexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[laurenreidexpensify]

I suspect this is intentional cos any account on a Control or Collect plan is being billed in the currency selected for the default billing. I just tested this in account of mine where the billing is on GBP and the default currency populated as that.

Gonna ask around further now.

[laurenreidexpensify]

I am asking in Slack here https://expensify.slack.com/archives/C049HHMV9SM/p1680517071785079

If that link doesn't work please search for "BUG DISCUSSION: WS - Default currency field isn't disabled when account have Control Policy"

[MelvinBot]

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

[MelvinBot]

Current assignee @laurenreidexpensify is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

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

[MelvinBot]

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

[spantheslayer]

Proposal

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

Currency field is not disabled even when accounts are having Control Policy

What is the root cause of that problem?

hasVBA is a variable used in the render() method in WorkspaceSettingsPage.js file. if hasVBA is true, the Picker component will be disabled. If hasVBA is false, the Picker component will be enabled. But by default the variable is always false even if an account is having control policy. Thus this issue occurs.

App/src/pages/workspace/WorkspaceSettingsPage.js

Lines 139 to 150 in 17ea27f

	<Picker
	inputID="currency"
	label={this.props.translate('workspace.editor.currencyInputLabel')}
	items={this.getCurrencyItems()}
	isDisabled={hasVBA}
	defaultValue={this.props.policy.outputCurrency}
	hintText={
	hasVBA
	? this.props.translate('workspace.editor.currencyInputDisabledText')
	: this.props.translate('workspace.editor.currencyInputHelpText')
	}
	/>

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

If we change the variable from hasVBA in isDisabled option to this.props.policy.name then the expected result happends.

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 143 in 17ea27f

isDisabled={hasVBA}

I have noticed, by default any workspace has a default account policy so when we target the policy name it will be disabled.

What alternative solutions did you explore? (Optional)

if we create a policy.controlPolicies array and see whether the array is empty or not depending on the account type creation and pass that in isDisabled section, it will act accordingly as well.

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 143 in 17ea27f

isDisabled={hasVBA}

[laurenreidexpensify]

@joelbettner @rushatgabhane bump on review ^^

[rushatgabhane]

thanks, on this

[hoangzinh]

It's really hard to make proposal without a control policy account

[rushatgabhane]

@joelbettner @laurenreidexpensify what exactly is an account having control policy?

[rushatgabhane]

@spantheslayer I think the main problem here is how to find if a policy has a control policy and disabling based on that. Your proposal doesn't explain that

[spantheslayer]

@rushatgabhane Yeah, the issue is that when you try to create a new policy from staging.expensify.com , it creates a workspace. which I believe has an account policy I believe (correct me if I am wrong).

But when we create a workspace from our expensify chat dashboard, it also reflects there in the staging.expensify.com section. that means by default every workspace gets created gets assigned a policy right?

That's why I thought of modifying the isDisabled param.

[rushatgabhane]

I don't think that's related. We need to figure out what an account with Control policy means.
Asking on slack https://expensify.slack.com/archives/C01GTK53T8Q/p1680855786767039

[spantheslayer]

Sure thanks that helps. Please let me know, I will raise another updated PR accordingly.

[rushatgabhane]

Alright, we found out that control policy is of type corporate in our codebase.
@laurenreidexpensify Question: should default currency field be disabled for Collect policy as well?

[spantheslayer]

@rushatgabhane so what's your though on this? should I try finding if the account is of corporate or what?
also I read that it is not possible for external contributors to test it out.

[Prince-Mendiratta]

Proposal

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

When visiting the general settings section of the workspace, the default currency field is not disabled.

What is the root cause of that problem?

I believe that this is not a bug/issue, this is simply a case of incomplete tests steps from #16449. The main blocker here is the concept of "Control Policy", which is leading us away from the main problem. The use of a Control Policy is different from a Workspace, a control policy is not even displayed in the workspaces page, as can be seen here:

App/src/libs/PolicyUtils.js

Lines 68 to 77 in e56b9c7

	function shouldShowPolicy(policy, isOffline) {
	return policy
	&& policy.type === CONST.POLICY.TYPE.FREE
	&& policy.role === CONST.POLICY.ROLE.ADMIN
	&& (
	isOffline
	|| policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE
	|| !_.isEmpty(policy.errors)
	);
	}

A Control Policy is a "corporate" policy type while a Workspace is a "free" policy type. The changes made in #16449 is responsible for improving the disabled state for the picker in a workspace. I further verified this by starting a free trial on expensify.com and creating a control policy.

As can be seen in the OP as well, the Control Policy is the email ID name while the Workspace is "Applause's Workspace". The picker is not disabled in the "Applause's Workspace".

Now, let's talk about when exactly the picker is supposed to be disabled. As can be seen in the slack thread created by @laurenreidexpensify here, the default currency picker is disabled only when a bank account is linked to the "Workspace".

The dropdown should only ever be disabled if the account has a verified bank account defaulted for reimbursement.

As far as I can tell in the OP, the tester has not linked a bank account to the WS, thus allowing the picker to be visible. I tried linking a bank account to the workspace myself but it involves verification technicalities from Concierge which I don't an external contributor can test yet.

The hasVBA variable in this file stands for whether a reimbursement account is attached to the Workspace. Since the bank account is not attached by default, the value is always false, as mentioned in @spantheslayer proposal, which is the expected result.

App/src/pages/workspace/WorkspaceSettingsPage.js

Lines 139 to 150 in 17ea27f

	<Picker
	inputID="currency"
	label={this.props.translate('workspace.editor.currencyInputLabel')}
	items={this.getCurrencyItems()}
	isDisabled={hasVBA}
	defaultValue={this.props.policy.outputCurrency}
	hintText={
	hasVBA
	? this.props.translate('workspace.editor.currencyInputDisabledText')
	: this.props.translate('workspace.editor.currencyInputHelpText')
	}
	/>

I propose that we test the #16449 once again but on an account which has a workspace with a reimbursement account linked, which need not be an expensifail account and if it works as expected, do nothing.

cc @rushatgabhane

[laurenreidexpensify]

@joelbettner @rushatgabhane bump ^^

[MelvinBot]

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

[laurenreidexpensify]

@joelbettner @rushatgabhane please address today ^^

Note for myself: C+ offer sent to Rushat

[rushatgabhane]

sorry i forgot about this

[rushatgabhane]

I agree with @Prince-Mendiratta do nothing for this issue.

@kbecciv could you please retest this issue after adding a bank account to the workspace?

[MelvinBot]

@joelbettner @rushatgabhane @laurenreidexpensify 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!

[laurenreidexpensify]

@kbecciv can you please retest this issue after adding a bank account to the workspace?

[MelvinBot]

@joelbettner, @rushatgabhane, @laurenreidexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[laurenreidexpensify]

bump @kbecciv thanks

[kbecciv]

@laurenreidexpensify Checking

[kbecciv]

@laurenreidexpensify After adding a bank account to the workspace - the Default currency field is disabled

[rushatgabhane]

perfect! Thanks for confirming @kbecciv
@laurenreidexpensify we can close this issue :)

[laurenreidexpensify]

Awesome thanks all!