Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread - Unable to invite user to thread when the thread originates from group chat #39332

Closed
6 tasks done
lanitochka17 opened this issue Mar 31, 2024 · 12 comments
Closed
6 tasks done
Assignees
Labels
Planning Changes still in the thought process Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Mar 31, 2024

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.58-4
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Start chat
  3. Create a group chat with two users
  4. Send a message in group chat
  5. Right click on message > Reply in thread
  6. Click on the thread header > Members
  7. Invite any user

Expected Result:

User is invited successfully

Actual Result:

User is not invited to the thread when the thread originates from group chat. The member count changes from 2 back to 1.

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

Bug6432864_1711831103828.20240331_043315.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Mar 31, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Mar 31, 2024

Triggered auto assignment to @arosiclair (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@lanitochka17
Copy link
Author

@arosiclair FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@rayane-d
Copy link
Contributor

rayane-d commented Apr 1, 2024

Response from backend:

{"jsonCode":401,"message":"401 Group Chats should use InviteToGroupChat command","onyxData":[],"requestID":"xxx"}

@abzokhattab
Copy link
Contributor

Proposal

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

Unable to invite user to thread when the thread originates from group chat

What is the root cause of that problem?

The backend returns 401 Group Chats should use InviteToGroupChat command when inviting members in a thread that originates from a group chat.

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

we need to make the invite room command conditional on whether the parent report is a group chat or not, if its a group chat then we need to send InviteToGroupChat instead of InviteToRoom.

to achieve that we need to add a new write command InviteToGroupChat here:

    INVITE_TO_GROUP_CHAT: 'InviteToGroupChat',

then inside the inviteToRoom function in the Report.ts file, we need to change this to:

    const parentReport = currentReportData?.[report.parentReportID];
    const isParentReportGroupChat = ReportUtils.isGroupChat(parentReport);
    API.write(isParentReportGroupChat ? WRITE_COMMANDS.INVITE_TO_GROUP_CHAT : WRITE_COMMANDS.INVITE_TO_ROOM, parameters, {optimisticData, successData, failureData});

@arosiclair
Copy link
Contributor

I reproduced on staging and prod v1.4.57-5 so this is not a blocker

@arosiclair arosiclair added Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment Engineering Hourly KSv2 labels Apr 1, 2024
Copy link

melvin-bot bot commented Apr 1, 2024

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

@arosiclair
Copy link
Contributor

I think this probably should be internal but I'm not sure. @marcaaron I saw your name on InviteToRoom and InviteToGroupChat. What do you think? Not sure if those can be used for threads too.

@marcaaron
Copy link
Contributor

This depends on the outcome of https://expensify.slack.com/archives/C066HJM2CAZ/p1712005980413999

I'm not sure if we intentionally designed this feature to work any kind of way, but proposed a couple of paths forward in that thread.

Gonna stay assigned here so I don't lose the context here. But maybe let's remove the Bug label for now as well until we have chosen a direction.

@marcaaron marcaaron self-assigned this Apr 1, 2024
@marcaaron marcaaron added Planning Changes still in the thought process and removed Bug Something is broken. Auto assigns a BugZero manager. labels Apr 1, 2024
@adelekennedy adelekennedy removed their assignment Apr 2, 2024
Copy link

melvin-bot bot commented Apr 5, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Apr 5, 2024
@marcaaron marcaaron added Weekly KSv2 and removed Daily KSv2 labels Apr 6, 2024
@melvin-bot melvin-bot bot removed the Overdue label Apr 6, 2024
@marcaaron marcaaron assigned marcaaron and s77rt and unassigned marcaaron Apr 6, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 22, 2024
@marcaaron
Copy link
Contributor

I'm gonna close this one in favor of https://github.com/Expensify/Expensify/issues/384761

That lays out in clearer terms how this feature should work.

@melvin-bot melvin-bot bot removed the Overdue label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Planning Changes still in the thought process Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants