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

Allow org admins to create guest users #1373

Merged
merged 9 commits into from
Jan 17, 2025

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Jan 13, 2025

Fixes #1268.

This adds the same "Create User" button, with the same dialog, to the org page that is on the admin dashboard, if the currently logged-in user has the right to manage the org (i.e., is an org admin).

image

It goes to the same "Create User" dialog as the one on the admin page (not screenshotted because this PR makes no changes to it), and has the same effect (if no email provided, creates guest user with "CreatedById" set to the org admin).

One possible improvement, which I have not yet made, would be that if an org admin creates a guest user, that user is automatically added to the currently-viewed org. (With, perhaps, a checkbox to turn that off, in the rare case where an org admin might be wanting to add a guest user NOT associated with any org — but the default should be on).

@myieye - Your thoughts on UI would be welcome. For example, should we change the name of this button when it's on the org page? I feel like having "Add Member", "Bulk Add Members", and "Create User" buttons all one after the other end up being a little much and has the potential for user confusion. But as I'm writing this, it's kind of late in the day and I'm having trouble thinking of what the alternative might be. I'll probably have better ideas tomorrow morning, though.

@rmunn rmunn requested review from hahn-kev and myieye January 13, 2025 21:03
@rmunn rmunn self-assigned this Jan 13, 2025
Copy link

github-actions bot commented Jan 13, 2025

UI unit Tests

12 tests  ±0   12 ✅ ±0   0s ⏱️ ±0s
 4 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit bd2425b. ± Comparison against base commit a8d082f.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 13, 2025

C# Unit Tests

104 tests  +1   104 ✅ +1   5s ⏱️ ±0s
 16 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit bd2425b. ± Comparison against base commit a8d082f.

This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.
Testing.LexCore.Services.FwLiteReleaseServiceTests ‑ CanGetLatestRelease
Testing.LexCore.Services.FwLiteReleaseServiceTests ‑ CanGetLatestRelease(platform: Linux)
Testing.LexCore.Services.FwLiteReleaseServiceTests ‑ CanGetLatestRelease(platform: Windows)

♻️ This comment has been updated with latest results.

@myieye
Copy link
Contributor

myieye commented Jan 14, 2025

@rmunn I just pushed this. What do you think?

image

I'd be inclined to always (i.e. it's not optional) add new users to the org, because:

  • It simplest for us
  • We want users to be in orgs

If the admin desperately doesn't want them to be in the org, they can remove them after.

@rmunn
Copy link
Contributor Author

rmunn commented Jan 14, 2025

@myieye - Looks good. I fixed a lint error and a compiler error, and increased the contrast (the text-success color wasn't standing out well on a light background, see below) but otherwise I like that idea.

Before the change, light BG:

before

After the change, light BG:

after

@rmunn
Copy link
Contributor Author

rmunn commented Jan 14, 2025

Ran out of time tonight to add the guest user to the org automatically; will add that code tomorrow morning. Then this will be ready for re-review.

rmunn added 2 commits January 15, 2025 10:21
It was a little confusing, so I consolidated it with
CanCreteGuestUserInProject by making the project ID optional. Site
admins can create guest users without a project ID, but org admins will
be required to specify a project ID when creating guest users.
@rmunn
Copy link
Contributor Author

rmunn commented Jan 15, 2025

@myieye - I've pushed commits 4b02d26 and a1de5f1 which assign new users to the org if they're added from the org page, and fix the problems you pointed out with the CanCreateGuestUserInAnyProject permission. (It was a mistake caused by coding late when I should have wrapped up for the evening). The permissions structure should be more sensible and comprehensible now.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

looks good, I suggest making projectId non nullable now. I'm not sure that orgId should be nullable, but I'm fine with how it works now.

backend/LexBoxApi/GraphQL/ProjectMutations.cs Outdated Show resolved Hide resolved
backend/LexBoxApi/GraphQL/UserMutations.cs Show resolved Hide resolved
@rmunn
Copy link
Contributor Author

rmunn commented Jan 16, 2025

@hahn-kev - One review comment addressed. The other one I don't think is actually necessary, but if you say "Let's make the change anyway" then I'll go ahead and do it tomorrow.

@rmunn rmunn requested a review from hahn-kev January 16, 2025 19:34
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

looks good to me! Nice work

backend/LexBoxApi/Services/PermissionService.cs Outdated Show resolved Hide resolved
@rmunn rmunn merged commit 30c8d5a into develop Jan 17, 2025
18 checks passed
@rmunn rmunn deleted the feat/org-admins-can-create-guest-users branch January 17, 2025 15:21
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.

allow org admins to create guest users
3 participants