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

Add GetAcceptedUsers to Invite api #107

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

mirekys
Copy link
Member

@mirekys mirekys commented Feb 22, 2021

This PR adds a new rpc endpoint to fetch all users that accepted a certain InviteToken with a certain token name/label.

This requires that additional name field is also added by this PR to the InviteToken resource.
This field holds a user-definable token name, so that the user can later query for all users that accepted the token with this name.

Related reva#1484

@mirekys mirekys requested a review from labkode as a code owner February 22, 2021 16:09
@labkode labkode merged commit ea7ed45 into cs3org:master Mar 3, 2021
@ishank011
Copy link
Contributor

@mirekys this doesn't look like a good solution to me. Users shouldn't have to name tokens when generating them. This adds an unnecessary step that affects UX. Instead, I would go for searching over all the users who've accepted a user's invite. There an issue can be that the local user doesn't know about the display names/emails of remote users with which they accepted the invite, but since they're sending the invite, I think we can assume they know their display names anyway. That is how this will work in the UI anyway, the original user will type the remote user's name.

After discussing this with @labkode, I propose removing name from the token and adding a method FindAcceptedUsers, similar to the user API, which will search over the accepted users' attributes (display name, email, etc.). We can add one optional field to the invite token description in which the original user can add some notes to be sent to the remote users in the mail.

@milkydan
Copy link

milkydan commented Mar 5, 2021

@ishank011 we have been thinking about such a use case. In case you have some Slack channel you wish to create the group from the users contained in that channel. So you'll generate an invitation (QR code) and you will insert it into the Slack channel. Then you don't have any idea who accepted the invitation (you don't know the email and so on), so you cannot find the particular user. That is the reason why we needed an optional field in the invitation so we can easily identify particular users eg. from the Slack channel over accepted users.

@ishank011
Copy link
Contributor

@milkydan this isn't something I'd worry about.

  • If we compare this to the sharing with the local users, you do need to know some attribute about the user you want to share with. So that is a prerequisite that we can assume on the users' side.
  • Since the field is optional, if the user doesn't enter a name, there's no way to search for who accepted it.
  • Also it won't scale. If a user generates 10 tokens with different names, expecting him to recall these names to find users is not ideal.

We haven't enabled group sharing yet, that is something that is on the agenda for this year and needs to be discussed. This can be one way which we can consider but for the time being, for individual user sharing, I think searching over user attributes would be the better method.

@milkydan
Copy link

milkydan commented Mar 5, 2021

@ishank011 ok we discussed it and agreed with your proposal. Thx 👍

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.

4 participants