[HOLD for payment 2024-01-24] CRITICAL: [$750] Default new rooms to your primary workspace

[m-natarajan]

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: 1.4.7-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1701632317613889

Action Performed:

1. Click fab and select start chat

Expected Result:

workspace field should default to primary workspace

Actual Result:

workspace field is blank

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b604efe47660d3a4
• Upwork Job ID: 1731962570569764864
• Last Price Increase: 2024-01-08
• Automatic offers:
• alitoshmatov | Reviewer | 28003794
• esh-g | Contributor | 28003795

[melvin-bot]

Triggered auto assignment to @dylanexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[dukenv0307]

Proposal

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

When creating a Room, workspace should default to primary workspace

What is the root cause of that problem?

This is a feature request

We set a default value for policyID as null which makes the workspace field is blank

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 103 in 93121be

const [policyID, setPolicyID] = useState(null);

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

We should create a useEffect to set the default value for policyID, we can set policy the first workspace in policies or the special workspace that we want to display by default

What alternative solutions did you explore? (Optional)

[esh-g]

Proposal

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

When creating a Room, workspace should default to primary workspace

What is the root cause of that problem?

Feature request. N/A

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

We should do the following changes:

1. Make the workspace value picker a controlled input by adding the value={policyID} attribute to it here:
   

   App/src/pages/workspace/WorkspaceNewRoomPage.js

   Lines 272 to 277 in 6dbaa4a

   	<ValuePicker
   	inputID="policyID"
   	label={translate('workspace.common.workspace')}
   	items={workspaceOptions}
   	onValueChange={setPolicyID}
   	/>

2. Modify the ValuePicker to prevent a null value from causing an error here by using optional chaining(value?.length):
   

   App/src/components/ValuePicker/index.js

   Line 65 in 6dbaa4a

   const descStyle = value.length === 0 ? StyleUtils.getFontSizeStyle(variables.fontSizeLabel) : null;

3. Add a useEffect to WorkspaceNewRoomPage to set the policyID to the first workspace:

useEffect(() => {
    setPolicyID(workspaceOptions[0].value);
}, [props.policies]);

What alternative solutions did you explore? (Optional)

Currently we don't have the activeWorkspace logic on newDot which is present on oldDot. In this issue, we are going to add a new Onyx key ACTIVE_WORKSPACE_ID which is exactly the use case for this.
After the implementation of the issue mentioned above, we can just access the ACTIVE_WORKSPACE_ID using the withOnyx HOC and initialise the state of the policyID to the passed prop.

Therefore, I think we can hold this issue until the ActiveWorkspace logic is implemented.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01b604efe47660d3a4

[melvin-bot]

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

[dylanexpensify]

Woot, welcome to the jungle @alitoshmatov! We already have a proposal from @dukenv0307 to review! 🎉 Can we make sure we review it today? 🙇‍♂️

[alitoshmatov]

Yes. I will review it today

[yh-0218]

Proposal

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

When creating a Room, workspace should default to primary workspace

What is the root cause of that problem?

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 79 in 48b5a29

const [policyID, setPolicyID] = useState(null);

Because we set policyID = null and reset from only workspace list selecting.

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

1. we need to init policyID

     useEffect(() => {
            if (workspaceOptions && workspaceOptions.length > 0) {
                setPolicyID(workspaceOptions[0].value);  // We can update logic
            } else {
                setPolicyID(null);
            }
      }, [workspaceOptions])

2. we need to set policyID to component as value
   value={policyID} to here
   

   App/src/pages/workspace/WorkspaceNewRoomPage.js

   Line 217 in 48b5a29

   <InputWrapper

3. update one more

const descStyle = value && value.length === 0 ? StyleUtils.getFontSizeStyle(variables.fontSizeLabel) : null;

App/src/components/ValuePicker/index.js

Line 65 in 48b5a29

const descStyle = value.length === 0 ? StyleUtils.getFontSizeStyle(variables.fontSizeLabel) : null;

What alternative solutions did you explore? (Optional)

Screen.Recording.2023-12-05.at.3.20.29.PM.mov

[alitoshmatov]

@dukenv0307 Your solution won't update initial value of ValuePicker.

[dukenv0307]

@alitoshmatov Of course we will have to pass value for policy picker if we want to change the value by using useEffect. It's the same way as we do for Visibility picker. My proposal has the main ideal and implementation can be detailed in the PR

[alitoshmatov]

@dukenv0307 I much prefer to turn ValuePicker into controlled component, I don't think providing defaultValue prop separately and setting default value for policyID separately is a good idea. We are much better off providing value prop to ValuePicker component

[alitoshmatov]

@esh-g 's proposal looks great and solve the issue.

We can go with that.

C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[dukenv0307]

@alitoshmatov I mean we will pass value prop here.

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 222 in 48b5a29

onValueChange={setPolicyID}

[alitoshmatov]

@yh-0218 Your proposal is similar to @esh-g 's proposal

[esh-g]

@alitoshmatov Would you like to wait for the active workspace to get implemented before we work on this? (Described in the alternate solution section of the proposal)

[alitoshmatov]

@dukenv0307 I understand what you mean, but your proposal wasn't clear enough. I would have understood if you had missed small detail, but it wasn't. In the first look, I didn't even assumed you meant this.

[roryabraham]

Great, I left a few minor comments on the test PR, then I think we can merge that one and do a second follow-up PR to finish this issue out.

[roryabraham]

Upping the bounty for this issue per this comment

[melvin-bot]

Upwork job price has been updated to $750

[roryabraham]

Merged the PR to introduce tests 🚀

[roryabraham]

@esh-g all the back-end pieces are deployed, so E/App should have an activePolicyID field under the Account object in Onyx now. Looking forward to seeing the main PR discussed above ASAP. Thanks!

[esh-g]

@roryabraham Is the activePolicyID always set in the backend? If we create a new workspace, does it get set to that workspace? Because while adding testing steps, I need to make sure an activePolicyID exists...

Also what tests should I add in the policyTests for the activePolicyID?

[roryabraham]

Is the activePolicyID always set in the backend?

Currently it's controlled in the policy settings editor in OldDot and always set when creating a policy on expensify.com, but I don't know if it's set for new workspaces in NewDot. Let me check. If not another 1 or 2 back-end PRs may be needed.

However, that doesn't need to block this issue to default new rooms to your primary workspace. If you need to make sure activePolicyID is set, sign into https://staging.expensify.com (instead of https://staging.new.expensify.com) with your test account, go to Settings -> Workspaces -> settings cog on the workspace you want to make your primary -> make primary

Also what tests should I add in the policyTests for the activePolicyID?

I suppose the main thing we would want to test in this is actually a UI thing – that when you open the new room page from the global create menu that the workspace is pre-populated by your activePolicyID. I don't think that fits in the test you already created though.

I'll get back to you if there's anything we want to add to the policy test you created.

[esh-g]

@roryabraham As per my understanding, then I don't need to add any additional test for now right? And could you please point out any other thing that might be missing in the PR?

[roryabraham]

Yeah, I think no additional tests necessary for now

[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.4.25-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Default workspace rooms to active workspace #34160

If no regressions arise, payment will be issued on 2024-01-24. 🎊

For reference, here are some details about the assignees on this issue:

• @alitoshmatov requires payment automatic offer (Reviewer)
• @esh-g requires payment automatic offer (Contributor)

[melvin-bot]

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

• [@alitoshmatov] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[dylanexpensify]

coming up!

[dylanexpensify]

tomorrow!

[alitoshmatov]

Regression Test Proposal

1. Make sure you have a workspace
2. If you don't have a workspace create one and logout and login again
3. Click on FAB
4. Click on Start chat
5. Choose Room tab
6. Make sure you have a default workspace selected on workspace selection

Do we agree 👍 or 👎

[dylanexpensify]

Payment summary:

• Contributor: @esh-g $750
• Contributor+: @alitoshmatov $750

Please apply or request!

[dylanexpensify]

done