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

kbecciv · 2023-03-30T16:41:25Z

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:

Launch the app
Log in with expensifail account which has Control policy
Go to Settings - Workspace - Tap on Avatar
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?

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 · 2023-03-30T16:41:32Z

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

MelvinBot · 2023-03-30T16:41:37Z

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 · 2023-03-31T22:53:27Z

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

MelvinBot · 2023-04-03T09:41:09Z

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

laurenreidexpensify · 2023-04-03T10:16:44Z

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 · 2023-04-03T10:21:59Z

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 · 2023-04-04T09:52:12Z

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

MelvinBot · 2023-04-04T09:52:15Z

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

MelvinBot · 2023-04-04T09:52:24Z

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

MelvinBot · 2023-04-04T09:52:30Z

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

spantheslayer · 2023-04-04T14:25:50Z

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 · 2023-04-06T11:35:58Z

@joelbettner @rushatgabhane bump on review ^^

rushatgabhane · 2023-04-06T19:44:11Z

thanks, on this

hoangzinh · 2023-04-07T01:44:05Z

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

rushatgabhane · 2023-04-07T08:09:57Z

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

rushatgabhane · 2023-04-07T08:11:32Z

@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 · 2023-04-07T08:19:41Z

@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 · 2023-04-07T08:21:10Z

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 · 2023-04-07T08:22:29Z

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

rushatgabhane · 2023-04-07T12:36:41Z

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 · 2023-04-07T12:41:37Z

@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 · 2023-04-07T13:25:06Z

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 · 2023-04-11T09:22:37Z

@joelbettner @rushatgabhane bump ^^

MelvinBot · 2023-04-11T16:00:16Z

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

laurenreidexpensify · 2023-04-12T11:56:45Z

@joelbettner @rushatgabhane please address today ^^

Note for myself: C+ offer sent to Rushat

rushatgabhane · 2023-04-12T12:08:40Z

sorry i forgot about this

rushatgabhane · 2023-04-12T12:09:29Z

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 · 2023-04-13T10:04:59Z

@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 · 2023-04-14T07:50:48Z

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

MelvinBot · 2023-04-17T10:07:31Z

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

laurenreidexpensify · 2023-04-17T10:18:17Z

bump @kbecciv thanks

kbecciv · 2023-04-17T14:52:11Z

@laurenreidexpensify Checking

kbecciv · 2023-04-17T16:33:34Z

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

rushatgabhane · 2023-04-18T02:26:48Z

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

laurenreidexpensify · 2023-04-18T12:33:04Z

Awesome thanks all!

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 30, 2023

melvin-bot bot assigned laurenreidexpensify Mar 30, 2023

mvtglobally mentioned this issue Mar 30, 2023

Deploy Checklist: New Expensify 2023-03-29 #16701

Closed

21 tasks

melvin-bot bot added the Overdue label Apr 3, 2023

melvin-bot bot removed the Overdue label Apr 3, 2023

laurenreidexpensify added the External Added to denote the issue can be worked on by a contributor label Apr 4, 2023

melvin-bot bot changed the title ~~WS - Default currency field isn't disabled when account have Control Policy~~ [$1000] WS - Default currency field isn't disabled when account have Control Policy Apr 4, 2023

melvin-bot bot assigned rushatgabhane Apr 4, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 4, 2023

melvin-bot bot assigned joelbettner Apr 4, 2023

melvin-bot bot added the Overdue label Apr 6, 2023

melvin-bot bot removed the Overdue label Apr 6, 2023

melvin-bot bot added the Overdue label Apr 10, 2023

melvin-bot bot removed the Overdue label Apr 11, 2023

melvin-bot bot added the Overdue label Apr 17, 2023

melvin-bot bot removed the Overdue label Apr 17, 2023

laurenreidexpensify closed this as completed Apr 18, 2023

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

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

Comments

kbecciv commented Mar 30, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

MelvinBot commented Mar 30, 2023

MelvinBot commented Mar 30, 2023 • edited by laurenreidexpensify Loading

Bug0 Triage Checklist (Main S/O)

allroundexperts commented Mar 31, 2023

MelvinBot commented Apr 3, 2023

laurenreidexpensify commented Apr 3, 2023

laurenreidexpensify commented Apr 3, 2023

MelvinBot commented Apr 4, 2023

MelvinBot commented Apr 4, 2023

MelvinBot commented Apr 4, 2023

MelvinBot commented Apr 4, 2023

spantheslayer commented Apr 4, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

laurenreidexpensify commented Apr 6, 2023

rushatgabhane commented Apr 6, 2023

hoangzinh commented Apr 7, 2023

rushatgabhane commented Apr 7, 2023 • edited Loading

rushatgabhane commented Apr 7, 2023 • edited Loading

spantheslayer commented Apr 7, 2023 • edited Loading

rushatgabhane commented Apr 7, 2023 • edited Loading

spantheslayer commented Apr 7, 2023

rushatgabhane commented Apr 7, 2023

spantheslayer commented Apr 7, 2023 • edited Loading

Prince-Mendiratta commented Apr 7, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

laurenreidexpensify commented Apr 11, 2023

MelvinBot commented Apr 11, 2023

laurenreidexpensify commented Apr 12, 2023

rushatgabhane commented Apr 12, 2023

rushatgabhane commented Apr 12, 2023 • edited Loading

MelvinBot commented Apr 13, 2023

laurenreidexpensify commented Apr 14, 2023

MelvinBot commented Apr 17, 2023

laurenreidexpensify commented Apr 17, 2023

kbecciv commented Apr 17, 2023

kbecciv commented Apr 17, 2023

rushatgabhane commented Apr 18, 2023

laurenreidexpensify commented Apr 18, 2023

kbecciv commented Mar 30, 2023 •

edited by melvin-bot bot

Loading

MelvinBot commented Mar 30, 2023 •

edited by laurenreidexpensify

Loading

rushatgabhane commented Apr 7, 2023 •

edited

Loading

rushatgabhane commented Apr 7, 2023 •

edited

Loading

spantheslayer commented Apr 7, 2023 •

edited

Loading

rushatgabhane commented Apr 7, 2023 •

edited

Loading

spantheslayer commented Apr 7, 2023 •

edited

Loading

Prince-Mendiratta commented Apr 7, 2023 •

edited

Loading

rushatgabhane commented Apr 12, 2023 •

edited

Loading