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

Fix: allow admins to reinvite revoked users #2496

Merged
merged 1 commit into from
Nov 13, 2023
Merged

Conversation

philipperolet
Copy link
Contributor

@philipperolet philipperolet commented Nov 12, 2023

Also prevents inviting a user already in the workspace
Related task

Notes

  • At the time, no email is sent to inform the user that they've been reintegrated to the workspace (shout out if you think we need one)
  • Animation of the modal popup: not done, should be done properly using Transition appear=true => sparkle change, for a following PR
  • Edge cases that can be safely ignored because very edgy with no impact
    • if an admin is on the page, and a user joins via other means while the admin has not yet send the invitation, the user will receive an email => can be safely ignored
    • if 2 admins are on the page, and one revokes a user, the other will have to reload if needing to invite them again

image

Also prevents inviting a user already in the workspace
Related [task](dust-tt/tasks#226)
Copy link
Contributor

@spolu spolu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I presume this will be weird when the email invite differs from the email used by the user:

  • Invite user by email A
  • User joins with email B
  • User gets revoked
  • Admin wants to re-instate, only way to do it is email B right?

What happens if I resend an invitation to email A in that case?

@philipperolet
Copy link
Contributor Author

You're correct in this case it will be weird. If admin resends an invite to email A instead of B, they will receive an invite link.

  • If they join with email A this time => they will be able to join but won't have their conversation history
  • if they join with email B => it will fail saying the user has been revoked

@philipperolet
Copy link
Contributor Author

I will merge & deploy so all the other cases are handled properly which is the priority. @gabhubert if you have remarks on behaviour and copy, happy to add them in a follow-up PR

@philipperolet
Copy link
Contributor Author

Regarding the case you mentioned @spolu , I would say it's fine to wait for it to happen and fix then since my intuition is it won't be a frequent occurence--but would have no problem handling it earlier should you beleive otherwise

@philipperolet philipperolet merged commit 4a09b1b into main Nov 13, 2023
1 check passed
@philipperolet philipperolet deleted the fix-revoked-invites branch November 13, 2023 10:21
@gabhubert
Copy link
Collaborator

Thanks @pr!

For copy: (not I'm using $member but I know we currently use "user" in front, we'll have to move it all later)

"$email is a revoked $member of the workspace. Reinstating as $member of the workspace them will also immediately reinstate their conversation history on Dust."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants