[HOLD for payment 2024-02-07] [$500] Room - Unable to create a public room if I don't select a workspace

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.26.1
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: Applause - Internal Team
Slack conversation:

Issue found when executing #33715

Action Performed:

1. Log in with a new expensifail account
2. From the initial open FAB menu, select Start chat
3. Click on the "Room" button
4. Click on the "Learn More" button
5. Create a new workspace
6. Go to FAB - Start chat - Room
7. Type in "123" as the room name (don't select a workspace)
8. Create the room

Expected Result:

I should be receiving an error message that I must select a workspace before creating a room.

Actual Result:

Unable to create a public room. "Unexpected error creating this chat, please try again laterPolicy rooms can not be created on personal policies".

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

Bug6345321_1705500809073.bandicam_2024-01-17_13-45-20-887.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014dfbb95a9411a4d8
• Upwork Job ID: 1747630089482182656
• Last Price Increase: 2024-01-17
• Automatic offers:
• ikevin127 | Contributor | 28106888
• s-alves10 | Contributor | 28107483

Issue Owner

Current Issue Owner: @thesahindia

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014dfbb95a9411a4d8

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

Proposal

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

Unable to create a public room. "Unexpected error creating this chat, please try again laterPolicy rooms can not be created on personal policies".

What is the root cause of that problem?

Recently this PR #34160 introduced the Default workspace rooms to active workspace functionality but did not take into account when validating to make sure the policyID (activePolicyID set at mount) actually exist within workspaceOptions (available workspaces to select).

What this does at mount is to set a policyID that doesn't exist within workspaceOptions, which causes the form to already have a valid workspace assigned which is not within the available options - hence why we can't see any workspace selected in the UI but the validation passes.

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

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 190 in 3033f99

if (!values.policyID) {

In the code block above the if condition we should also check if the current state value of policyID (activePolicyID set at mount) exists within the available workspaceOptions and if not, display workspace not selected error.

Videos

MacOS: Chrome / Safari

Screen.Recording.2024-01-17.at.17.04.47.mov

[mkhutornyi]

Proposal

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

Room - Unable to create a public room if I don't select a workspace

What is the root cause of that problem?

If account has activePolicyID, it's automatically selected as default.
This is problematic when policy linked to that policyID should not be visible in newDot (i.e. control policy)
So UI states that workspace is unselected but actually it's selected and sent to sever when create new room

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

App/src/pages/workspace/WorkspaceNewRoomPage.js

Lines 190 to 192 in 3033f99

	if (!values.policyID) {
	errors.policyID = 'newRoomPage.pleaseSelectWorkspace';
	}

Add more validation whether policy linked to values.policyID exists or not in workspaceOptions using same filter condition used in PolicyUtils.getActivePolicies

[s-alves10]

Proposal

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

Can create a public room without selecting a workspace, but results in errors. When trying to create a public room, error message to select a workspace should be shown to the user

What is the root cause of that problem?

We initialize the policyID here

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 103 in 3033f99

const [policyID, setPolicyID] = useState(props.activePolicyID);

We also initialize the public workspace options here

App/src/pages/workspace/WorkspaceNewRoomPage.js

Lines 199 to 207 in 3033f99

	const workspaceOptions = useMemo(
	() =>
	_.map(PolicyUtils.getActivePolicies(props.policies), (policy) => ({
	label: policy.name,
	key: policy.id,
	value: policy.id,
	})),
	[props.policies],
	);

The problem is that there are cases that activePolicyID isn't included in public workspaces. The workspace options include only public workspaces, but activePolicyID indicates private workspace. So the validation doesn't fails, but it looks like workspace field is empty

This is the root cause

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

We need to clear the policyID when it isn't a public workspace

Update

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 103 in 3033f99

const [policyID, setPolicyID] = useState(props.activePolicyID);

to

    const workspaceOptions = useMemo(
        () =>
            _.map(PolicyUtils.getActivePolicies(props.policies), (policy) => ({
                label: policy.name,
                key: policy.id,
                value: policy.id,
            })),
        [props.policies],
    );
    const [policyID, setPolicyID] = useState(() => {
        if (workspaceOptions.some(option => option.value === props.activePolicyID)) {
            return props.activePolicyID;
        }
        return '';
    });

Update

App/src/pages/workspace/WorkspaceNewRoomPage.js

Lines 145 to 150 in 3033f99

	useEffect(() => {
	if (policyID) {
	return;
	}
	setPolicyID(props.activePolicyID);
	}, [props.activePolicyID, policyID]);

to

    useEffect(() => {
        if (policyID) {
            return;
        }
        if (workspaceOptions.some(opt => opt.value === props.activePolicyID)) {
            setPolicyID(props.activePolicyID);
        } else {
            setPolicyID('');
        }
    }, [props.activePolicyID, policyID, workspaceOptions]);

This works as expected

Result

34647.mp4

What alternative solutions did you explore? (Optional)

[dukenv0307]

Proposal

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

Unable to create a public room. "Unexpected error creating this chat, please try again laterPolicy rooms can not be created on personal policies".

What is the root cause of that problem?

The active policy is the expense default of the account. But it's hidden in workspaceOptions because getActivePolicies only returns true for policy has isPolicyExpenseChatEnabled or areChatRoomsEnabled is true.

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 149 in 3033f99

setPolicyID(props.activePolicyID);

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

We should remove this condition now policy.isPolicyExpenseChatEnabled || policy.areChatRoomsEnabled since we always choose the default policy by default.

App/src/libs/PolicyUtils.ts

Line 19 in 3033f99

policy !== null && policy && (policy.isPolicyExpenseChatEnabled || policy.areChatRoomsEnabled) && policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,

What alternative solutions did you explore? (Optional)

We can check if the active policy of this accout isn't enable now, we can choose the first policy from workspaceOptions

[trjExpensify]

Asking about this one in #vip-vsb here: https://expensify.slack.com/archives/C066HJM2CAZ/p1705526775582279

I'm unsure of:

1. The state of play of rolling this feature out to pre-populate the workspace field with your activePolicyID
2. If handling for the scenario where your activePolicyID is a personal workspace has been scoped and in the works elsewhere.

If not on #2, my suggestion is to show an in-line form error to "Please choose a workspace", as the only options should be group workspace types.

[roryabraham]

So I haven't thoroughly reviewed all the proposals yet, but at a high level I'm proposing a twofold solution:

1. (front-end) Don't let people create rooms without filling out the workspace field. Already, it looks like the workspace field appears as empty if the activePolicyID is a personal policy that isn't in NewDot yet.
2. (back-end) Given that someone's activePolicyID is a personal policy, and they aren't a member of a group policy yet; when they create their first workspace via NewDot, update that to be their activePolicyID.

[roryabraham]

For 1, it looks like most of the proposals here would work, so I'm going to select the first that I think is sufficient.

[melvin-bot]

📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[ikevin127]

#34691 ready for review!

[s-alves10]

@roryabraham

I am so confused about how the selection process goes here. It looks like you didn't check the edit history of proposals. The first proposal was wrong and so I submitted a proposal with correct RCA. The former proposals was updated later. Of course, I would respect the decision of C+/internal team, but I want you to check the edit history at least.

We have similar discussions recently. Please check https://expensify.slack.com/archives/C01GTK53T8Q/p1703083893742679
@ikevin127 you know about that discussion, I think

[mkhutornyi]

This was @ikevin127's proposal before others posted.

[dukenv0307]

@roryabraham Actually activePolicyID is added to always choose the policy by default so I think we need to select another if this policy isn't enabled. That will make consistent when this policy is enable or not and we don't need to validate for the workspace field.

[s-alves10]

@thesahindia PR is ready for review #34796

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

• fix: check if the default workspace is a public workspace #34796

If no regressions arise, payment will be issued on 2024-02-07. 🎊

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

• @thesahindia requires payment through NewDot Manual Requests
• @s-alves10 requires payment automatic offer (Contributor)

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

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

[trjExpensify]

👋 checklist time here!

[trjExpensify]

Same Melv, awaiting the checklist!

[thesahindia]

Steps for test case -

1. Click the FAB icon > Start chat > Room
2. Verify that no workspace is selected
3. Enter room name and click "Create room"
4. Verify that the error message "Please select a workspace" appears

[trjExpensify]

Don't forget about the rest of the checklist? :)

As for the regression test, looks like we're going to be adding one already, so I've suggested a modification to it in that GH.

[trjExpensify]

Regression test sorted, @thesahindia you need to complete the rest of the checklist so we can issue payments and close this one out.

[thesahindia]

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:

• [@thesahindia] The PR that introduced the bug has been identified. Link to the PR:

#34160

• [@thesahindia] 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:

#34160 (comment)

• [@thesahindia] 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:

https://expensify.slack.com/archives/C01GTK53T8Q/p1708594431270969

• [@thesahindia] Determine if we should create a regression test for this bug.

Done

• [@thesahindia] 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.

Done

[trjExpensify]

Thanks! Payment summary as follows:

• @thesahindia $500 for the C+ review (to request on ND).
• @s-alves10 $500 for the fix (Paid).

Closing this out!