zulip / zulip

Zulip server and web application. Open-source team chat that helps teams stay productive and focused.
https://zulip.com
Apache License 2.0
21.14k stars 7.65k forks source link

Offer a way to edit streams/role details for invitations #22099

Open jwillmer opened 2 years ago

jwillmer commented 2 years ago

Request: Be able to modify pending invites.

Reason: Sometime we forget to add a channel to an invite and it would make it very convenient if we could modify the pending invite instead of waiting for the user to join or cancel/recreate the invite.

timabbott commented 2 years ago

This is clearly a thing we'll want to support. It's probably most relevant for reusable invitation links, just because those are something one would want to maintain over time.

It's likely a moderately complex project, since we don't currently send the data on what streams are associated with an invite to clients, but potentially fairly straightforward, especially if we can reuse the component for selecting which streams to select from the existing invitation UI.

zulipbot commented 2 years ago

Hello @zulip/server-onboarding, @zulip/server-settings members, this issue was labeled with the "area: settings (admin/org)", "area: invitations" labels, so you may want to check it out!

bigBrain1901 commented 1 year ago

@zulipbot claim

zulipbot commented 1 year ago

Welcome to Zulip, @bigBrain1901! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

bigBrain1901 commented 1 year ago

Hi! I would love to work on this issue. If I understand the issue correctly, the following is what I propose. Kindly confirm.

  1. Add an "Edit" action apart from the "Revoke" action in the SETTINGS / INVITATIONS modal.
    image
  2. On click of this button, the same Invite users to Zulip modal opens up - in edit mode with pre-filled invite fields instead of default values.
  3. On the new Edit an Invite to users modal, the "Save Changes" action button will allow the updation of the invite.
alya commented 1 year ago

That general approach sounds reasonable to me in terms of the user experience.

bigBrain1901 commented 1 year ago

... we don't currently send the data on what streams are associated with an invite to clients ...

One doubt, what is the reason for keeping back the data on associated streams from clients?
If it is alright to add the data on associated streams as an array of stream_ids to the invite object, that could be done.

Otherwise, we could consider a new endpoint to fetch this data on a per-invite basis like -
GET /json/invites/{invite_id}/streams

This could be favourable if there are -

  1. Security or disclosure issues with having the stream_ids in the invite object.
  2. Maybe, it is an overhead on the GET /json/invites endpoint?
timabbott commented 1 year ago

I think there's no reason we couldn't have do_get_invites_controlled_by_user also return a list of Stream IDs associated with the invitation. We only query that endpoint when showing this panel. Since I expect only administrators will be able to edit invites, the client should be able to display the streams with their name and other details in most cases; and I'm sure we can figure out an "Unknown stream (ID 17)" display format for any where the current user can't see metadata on it.

I don't see any reason this change would be expensive if done with a proper bulk query.

It might also be reasonable to add a GET /json/invites/{invite_id} that returns metadata on a single invite, but I'd expect that to be the whole object, not just the stream IDs list.

bigBrain1901 commented 1 year ago

The Case Story

  1. Users must be able to edit their multiuse_invites.
  2. Users must be able to edit the following fields a. invite_as b. streams
  3. Users must see this "Edit" option in organization_settings/invitations

A screencast of the expected behaviour

screen-recorder-wed-mar-08-2023-23-06-20.webm

Request

Please let me know if this is the desired behaviour, and any changes.

alya commented 1 year ago

Thanks! The "Edit" button should be a pencil, as in all the other menus. We may want to change the other buttons there to icons as well.

As a note for next time, please include screenshots of key screens any time you're requesting feedback, as they are easier to review than a video. (Also posting a video can be helpful as well in case there's a question about the interactions.)

bigBrain1901 commented 1 year ago

Perfect, so I have a commit that adds this feature. But I haven't written tests for it yet.

Shall I create a new Draft PR for an initial review of the UI/UX, and then proceed to write tests if that review passes?
Do let me know.

alya commented 1 year ago

@bigBrain1901 You can always create a draft PR if that's helpful for your workflow, but your work will generally be reviewed when it's complete (including tests).

bigBrain1901 commented 1 year ago

The PR is now ready for review, including tests.

(I messed up a rebase in the previous PR, hence this new one :angel: )

zulipbot commented 1 year ago

@bigBrain1901 You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!