Skip to content

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

Closed

Description

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


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 OwnerCurrent Issue Owner: @thesahindia

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jan 17, 2024
  2. changed the title [-]Room - Unable to create a public room if I don't select a workspace[/-] [+][$500] Room - Unable to create a public room if I don't select a workspace[/+] on Jan 17, 2024
  3. melvin-bot commented on Jan 17, 2024

    @melvin-bot
  4. melvin-bot commented on Jan 17, 2024

    @melvin-bot

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

  5. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jan 17, 2024
  6. melvin-bot commented on Jan 17, 2024

    @melvin-bot

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

  7. ikevin127 commented on Jan 17, 2024

    @ikevin127
    Contributor

    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?

    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
  8. mkhutornyi commented on Jan 17, 2024

    @mkhutornyi
    Contributor

    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?

    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

  9. dummy-1111 commented on Jan 17, 2024

    @dummy-1111
    Contributor

    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

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

    We also initialize the public workspace options here

    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

    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

    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)

  10. dukenv0307 commented on Jan 17, 2024

    @dukenv0307
    Contributor

    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.

    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.

    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

  11. trjExpensify commented on Jan 17, 2024

    @trjExpensify
    Contributor

    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.

  12. 58 remaining items

  13. trjExpensify commented on Feb 12, 2024

    @trjExpensify
    Contributor

    Same Melv, awaiting the checklist!

  14. thesahindia commented on Feb 12, 2024

    @thesahindia
    Member

    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
  15. trjExpensify commented on Feb 14, 2024

    @trjExpensify
    Contributor

    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.

  16. trjExpensify commented on Feb 19, 2024

    @trjExpensify
    Contributor

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

  17. thesahindia commented on Feb 22, 2024

    @thesahindia
    Member

    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

  18. trjExpensify commented on Feb 22, 2024

    @trjExpensify
    Contributor

    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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions