m-natarajan commented 11 months ago

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.13-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/p1702580397915479

Action Performed:

Alice clicks Start chat
Alice clicks Add to group next to Bob
Alice then clicks Cathy's name direct

Expected Result:

It opens a DM with Cathy (ignoring the group with Bob)

Actual Result:

Should open a group with Bob and Cathy

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/24bc0b8f-93dc-40fb-b68b-3d156fd47c52

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b26e760299069d66
Upwork Job ID: 1735411019486609408
Last Price Increase: 2023-12-14

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

neonbhai commented 11 months ago

Proposal

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

When doing Start chat, and someone is added to a group, clicking on a person should add them to the group, not discard the group

What is the root cause of that problem?

This happens as in NewChatPage on selecting a row always opens a DM. https://github.com/Expensify/App/blob/83a89fbabc0a550993e5e4d6b9f8904aee9ff1ca/src/pages/NewChatPage.js#L248 There is no logic to check if user has started adding participants to groups.

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

We should add logic to add to group if there are participants selected already.

onSelectRow={(option) => {
    selectedOptions.length >= 1 ? toggleOption(option) : createChat(option)
}

This makes sure if the user has some selected participants, clicking on row will add to selection, rather than discarding everything and navigating to chat.

Result

Before

Selection not preserved. Group lost.

https://github.com/Expensify/App/assets/64629613/78c00f65-726f-45b7-9446-9e5f06fcbe7e

After

Selection preserved. Start chat feels like in selection mode

https://github.com/Expensify/App/assets/64629613/4713adbd-3578-4f37-89a9-a1a171cb9546

ZhenjaHorbach commented 11 months ago

Proposal

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

When doing Start chat, and someone is added to a group, clicking on a person should add them to the group, not discard the group

What is the root cause of that problem?

When we press on the contact we сhoose only one contact and then create a chat

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

We can update function like

                            onSelectRow={(option) => {
                                createGroup(option);
                            }}

https://github.com/Expensify/App/blob/83a89fbabc0a550993e5e4d6b9f8904aee9ff1ca/src/pages/NewChatPage.js#L248

And update createGroup

    const createGroup = (option) => {
        const newSelectedOptions = option.login && !_.find(selectedOptions, (item) => item.login === option.login) ? [...selectedOptions, option] : [...selectedOptions];
        const logins = _.pluck(newSelectedOptions, 'login');
        if (logins.length < 1) { //Optional. Because I can't think of a single case when we have 0 selected users
            return;
        }
        Report.navigateToAndOpenReport(logins);
    };

https://github.com/Expensify/App/blob/83a89fbabc0a550993e5e4d6b9f8904aee9ff1ca/src/pages/NewChatPage.js#L173-L179

Also we can delete createChat due to unnecessary

We can use createGroup instead createChat because they use the same Report method

What alternative solutions did you explore? (Optional)

NA

Krishna2323 commented 11 months ago

Proposal

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

When doing Start chat, and someone is added to a group, clicking on a person should add them to the group, not discard the group

What is the root cause of that problem?

We start chat on selecting any row with individual.

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

We should add a function to determine if we should toggle and create a group or we should start a chat, we can return the updated selected options from toggleOption function and immediately use it for creating the group. We can pass the updated selected options to createGroup and use it instead of selectedOptions if present.

    const onSelectRow = (option) => {
        if (selectedOptions.length >= 1) {
            const newOptions = toggleOption(option);
            const logins = _.pluck(newOptions, 'login');
            if (logins.length < 1) {
                return;
            }
            Report.navigateToAndOpenReport(logins);
        } else {
            createChat(option);
        }
    };

Result

https://github.com/Expensify/App/assets/85894871/21cbb468-a7dc-482d-9cd9-247952a7e340

neonbhai commented 11 months ago

Proposal

Updated

Added result video demonstrating the solution.

dukenv0307 commented 11 months ago

Proposal

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

Click on the option doesn't add the user to the group

What is the root cause of that problem?

When we don't click on Add to group, we always create a DM chat

https://github.com/Expensify/App/blob/4592a7c53a2e8bbbcb440dce8d07ccf165ce1cd3/src/pages/NewChatPage.js#L248

We have the same problem in MoneyTemporaryForRefactorRequestParticipantsSelector

https://github.com/Expensify/App/blob/4592a7c53a2e8bbbcb440dce8d07ccf165ce1cd3/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js#L311

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

We should check if we already click on the add button, when we select another we will call toggleOption

onSelectRow={(option) => {
    if (selectedOptions.length > 0) {
        toggleOption(option);
        return;
    }
    createChat(option)
}}

https://github.com/Expensify/App/blob/4592a7c53a2e8bbbcb440dce8d07ccf165ce1cd3/src/pages/NewChatPage.js#L248

Fix the same here

onSelectRow={participants.length === 0 ? addSingleParticipant: addParticipantToSelection}

https://github.com/Expensify/App/blob/4592a7c53a2e8bbbcb440dce8d07ccf165ce1cd3/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js#L311

What alternative solutions did you explore? (Optional)

We can create a button which can help us to switch between create a DM chat or a group.

bernhardoj commented 11 months ago

This is the same behavior as request money, I think not a bug.

melvin-bot[bot] commented 11 months ago

@burczu, @adelekennedy Whoops! This issue is 2 days overdue. Let's get this updated quick!

burczu commented 11 months ago

I tend to agree with @bernhardoj's comment that this may not be a bug - group is created once we click on Create group button and clicking on the email just opens a chat with this person. Also, as @bernhardoj pointed out - it works the same with requesting money: when we select with few people to split bill, and then click on one of the selected emails, it choose only this person and moves to the next step.

What do you think @adelekennedy?

adelekennedy commented 11 months ago

let's close - this may be something we decide to change in the future but we can leave that to the split group to decide

Expensify / App

[$500] When doing Start chat, and someone is added to a group, clicking on a person should add them to the group, not discard the group #33119

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Bug0 Triage Checklist (Main S/O)

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?

Result

Before

After

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)

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?

Result

Proposal

Updated

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)