feat: add guest invitation #521
Conversation
DeepDiver1975
commented
Jun 21, 2022
•
DeepDiver1975
commented
DeepDiver1975
force-pushed
the
feat/invite-guests
branch
5 times, most recently
from
c148b82
to
b5cbf52
Compare
|
Please make the input validation consistent with the regular sharing dialog. E.g., "ab@cd" offers guest creation while you need to add ".xy" in the sharing dialog. |
|
Please make sure that the CSV import will also work with new guests (and with multiple users). |
We could now start a discussion what is the technical correct implementation.... ... anyway will adjust the new code base for the sake of consistency |
DeepDiver1975
force-pushed
the
feat/invite-guests
branch
4 times, most recently
from
4d9c6ab
to
809d8f0
Compare
We've had that one earlier and decided for usability against technical correctness :) |
DeepDiver1975
force-pushed
the
feat/invite-guests
branch
from
809d8f0
to
937c755
Compare
There was a problem hiding this comment.
I just noticed that JP added a few comments in #522 that relate to this PR. I added them here.
| var displayName = data.displayName || userId | ||
| var role = null | ||
| if (data.type === 'guest') { | ||
| displayName = userId |
There was a problem hiding this comment.
we might need a comment here. The displayname should be already initialized above prioritizing the displayname over the userid. If guests will use the userid anyway I think this needs a comment to explain it's intentional, otherwise it might be a bug because one type of users will use the displayname while others the userid.
| ]; | ||
| }, $results); | ||
|
|
||
| # guest integration |
There was a problem hiding this comment.
I'm a bit confused here and maybe we need a comment clarifying.
Aren't guests shown as part of the regular result? or is this additional result a placeholder so the user can click on it to add that new user as a guest?
|
|
||
| namespace OCA\CustomGroups\Service; | ||
|
|
||
| use OCA\Guests\Controller\UsersController; |
There was a problem hiding this comment.
I'm not sure if this works without the guests app around...
I think it's better to use either an API call or even a symfony event in order to remove the dependency with the app.
There was a problem hiding this comment.
Guests is part of the *-complete.tar bundle, so we are 'pretty sure (tm)' it is installed.
API call (with proper error handling) is of course the better solution.
Implement batch action for inviting users to groups
|
Kudos, SonarCloud Quality Gate passed! |
|
Kudos, SonarCloud Quality Gate passed! |
|
Confirmed implemented in 0.7.0-rc.1 There is no problem when the guests app is disabled. The functionality is not available then.
|
