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

[$250] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list #35391

Closed
waterim opened this issue Jan 30, 2024 · 78 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2

Comments

@waterim
Copy link
Contributor

waterim commented Jan 30, 2024

Problem:

Right now there are no possibilities to add anyone to the automatically created workspace chats, but it should be possible to add by "mention"; it's similarly not possible to remove someone who has been thus added.

Solution:

As discussed here, we would like to add this possibility to:

  1. Allow anyone to be added to the workspace chat via @mention
  2. Allow anyone to be added to the workspace chat via the Invite button of the members page
  3. Allow anybody who wasn't auto-added (ie, admins or the employee) to be removed via the members page

IMPORTANT details for workspace chats:

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013623b6c6ce0881ae
  • Upwork Job ID: 1753488042510364672
  • Last Price Increase: 2024-06-28
  • Automatic offers:
    • brunovjk | Contributor | 0
Issue OwnerCurrent Issue Owner: @johncschuster
@quinthar quinthar moved this to MEDIUM in [#whatsnext] #vip-vsb Feb 1, 2024
@quinthar quinthar changed the title Add users to workspace chat by mentioning them. Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list Feb 1, 2024
@quinthar
Copy link
Contributor

quinthar commented Feb 1, 2024

optimistically assigning to @waterim

@melvin-bot melvin-bot bot added the Monthly KSv2 label Feb 2, 2024
@waterim
Copy link
Contributor Author

waterim commented Feb 2, 2024

@quinthar Yes, I can take this one
Im still OOO(sick leave) today and can work in this one from Monday.

@jasperhuangg
Copy link
Contributor

This has to be fixed internally, I can take this

@jasperhuangg jasperhuangg self-assigned this Feb 2, 2024
@jasperhuangg jasperhuangg added the Internal Requires API changes or must be handled by Expensify staff label Feb 2, 2024
Copy link

melvin-bot bot commented Feb 2, 2024

Job added to Upwork: https://www.upwork.com/jobs/~013623b6c6ce0881ae

Copy link

melvin-bot bot commented Feb 2, 2024

Triggered auto assignment to Contributor Plus for review of internal employee PR - @thesahindia (Internal)

@jasperhuangg
Copy link
Contributor

Won't need C+ reviews for these PRs, at least for now

@jasperhuangg jasperhuangg added Weekly KSv2 and removed Monthly KSv2 labels Feb 2, 2024
@quinthar quinthar changed the title Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list Feb 4, 2024
@quinthar
Copy link
Contributor

quinthar commented Feb 8, 2024

What's the next step here? Who is doing it and when?

@jasperhuangg
Copy link
Contributor

@quinthar sorry, been bogged down with Wave work. The next step is to update the Auth checks that dictate who can be invited to a workspace chat. We can then update the front-end to handle the changes involving allowing only the people who weren't automatically added to be removed.

One of my wave issues was deprioritized, so I can get cracking with this tomorrow!

@melvin-bot melvin-bot bot removed the Overdue label Feb 14, 2024
@jasperhuangg
Copy link
Contributor

First thing I noticed is that policy expense chats don't give the user/approvers "read, write, share" permissions by default, so I'm going to open a PR to fix that since those permissions are needed in order to invite to the report.

@jasperhuangg
Copy link
Contributor

Just a quick update on this, I have a PR up and I'm figuring out some MergeAccount tests that it broke.

@jasperhuangg
Copy link
Contributor

Gonna continue pushing this PR forward today.

@jasperhuangg
Copy link
Contributor

Quick update, Auth PR is on HOLD for the Web-E^ to go out to production. It's on staging right now

@melvin-bot melvin-bot bot added the Overdue label Mar 5, 2024
@jasperhuangg
Copy link
Contributor

Gonna continue pushing this forward today. Had to deprioritize it for other items.

@melvin-bot melvin-bot bot added the Weekly KSv2 label Jun 8, 2024
@brunovjk
Copy link
Contributor

Update: PR being reviewed

Copy link

melvin-bot bot commented Jun 17, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jun 17, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-06-06] [$500] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list [HOLD for payment 2024-06-24] [HOLD for payment 2024-06-06] [$500] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list Jun 17, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jun 17, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.84-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

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

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

Copy link

melvin-bot bot commented Jun 17, 2024

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:

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

@johncschuster
Copy link
Contributor

Acknowledged! I'll issue payment on the 24th!

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jun 21, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-06-24] [HOLD for payment 2024-06-06] [$500] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list [HOLD for payment 2024-06-28] [HOLD for payment 2024-06-24] [HOLD for payment 2024-06-06] [$500] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list Jun 21, 2024
Copy link

melvin-bot bot commented Jun 21, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.85-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

Copy link

melvin-bot bot commented Jun 21, 2024

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:

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

@brunovjk
Copy link
Contributor

Acknowledged! I'll issue payment on the 24th!

@johncschuster Could we get an increase in the bounty here? Mainly to cover regression. Extensive testing and adjustments were necessary during and after the PR was merged. Thank you 🙏

cc: @jasperhuangg

@johncschuster
Copy link
Contributor

@brunovjk Let me check in with the team about your request. I'll follow up in a bit!

@brunovjk
Copy link
Contributor

Thank you :D

@johncschuster
Copy link
Contributor

johncschuster commented Jun 25, 2024

@brunovjk, I chatted with the team, and it seems like a regression was introduced here, which would result in a payment reduction, not an increase. Can you tell me a bit more about why you feel there should be an increase?

@brunovjk
Copy link
Contributor

Thank you @johncschuster. Here's an explanation:

The initial task here was Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list PR, then we had to update "who can" select/add/remove and which users can be selected/removed on workspace chats PR. That's why I asked for an increase in the reward, since this second PR involved a lot of testing and two regressions, #43828 and #42928 .
My idea was for the bounty to be $1000, minus the two regressions totalizing $250. But, I am in favor of any of your decisions.

@jasperhuangg do we need to wait for #42928 to proceed with payment here?

@johncschuster
Copy link
Contributor

johncschuster commented Jun 28, 2024

Payment Summary:

Contributor: @brunovjk - $1000 $250 (reduced for 2x regressions) - paid via Upwork - PAID
Contributor+: @mananjadhav - $1000 $250 (reduced for 2x regressions) - requires payment through NewDot Manual Requests

@johncschuster johncschuster changed the title [HOLD for payment 2024-06-28] [HOLD for payment 2024-06-24] [HOLD for payment 2024-06-06] [$500] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list [$250] MEDIUM: Allow arbitrary users to be added/removed to/from workspace chat via @mention or members list Jun 28, 2024
Copy link

melvin-bot bot commented Jun 28, 2024

Upwork job price has been updated to $250

@johncschuster
Copy link
Contributor

@mananjadhav, can you propose a regression test checklist?

@mananjadhav
Copy link
Collaborator

mananjadhav commented Jun 28, 2024

@johncschuster I think the test steps from the PR here. I are good for the checklist. Can we please use that?

Pre-condition:

1. We need 4 accounts for this test:

  • Workspace Creator
  • Workspace Admin
  • Workspace Member
  • No-workspace member (invited)

2. Login in the app as "Workspace Creator"
3. Create a workspace
4. Go to the automatically created "Workspace Chat"
5. Go to the "Report Details"
6. Go to "Members" page Click "Invite"
7. Invite all the other 3 accounts (admin, member, and invited) to the "Workspace Chat"
8. Still on the "Members" page, change the admin role to Administrator 8. Click on the "Global Fab" "Start Chat" Select "#Room" tab
9. Create a "workspace-room"
10. Invite all the other 3 accounts (admin, member, and invited) to the "workspace-room"

Tests:

1. Logged as the "Workspace Creator":

i. Go to the "Workspace Chat" Report Details page
ii. Verify that the "Leave" button doesn't show, as you are a "workspace admin".
iii. Go to "Members" page
iv. Verify that you cannot select/remove admin and default added members
v. Verify that you can "Invite" and "Remove" (admin cannot be removed) members
vi. Go to the "workspace-room" Report Details page
vii. Go to "Members" page
viii. Verify that you can "Invite" and "Remove" (you cannot remove yourself here) members

2. Logged as the "Workspace Admin":

i. Go to the "Workspace Chat" Report Details page
ii. Verify that the "Leave" button doesn't show, as you are a "workspace admin".
iii. Go to "Members" page
iv. Verify that you cannot select/remove admin and default added members
v. Verify that you can "Invite" and "Remove" (admin cannot be removed) members
vi. Go to the "workspace-room" Report Details page
vii. Go to "Members" page
viii. Verify that you can "Invite" and "Remove" (you cannot remove yourself here) members

3. Logged as the "Workspace Member":

i. Go to the "Workspace Chat" Report Details page
ii. Verify that the "Invite" button does not show.
iii. Go to "Members" page
iv. Verify that you cannot "Invite" and "Remove" members.
v. Logged as the "Workspace Member" Go to the "workspace-room" Report Details page
vi. Verify that the "Invite" button does not show.
vii. Go to "Members" page
viii. Verify that you can "Invite" and "Remove" (you cannot remove yourself here) members.

4. Logged as the "Workspace Chat Invited":

i. Go to the "Workspace Chat" Report Details page
ii. Verify that the "Invite" button does not show.
iii. Go to "Members" page
iv. Verify that you cannot "Invite" and "Remove" members.
v. Logged as the "Workspace Room Invited" Go to the "workspace-room" Report Details page
vi. Verify that the "Invite" button does show and you can invite members to the chat.

@JmillsExpensify
Copy link

$250 approved for @mananjadhav

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2
Projects
No open projects
Development

No branches or pull requests

8 participants