Add "Create User" button to admin dashboard #736
Conversation
Doesn't do anything... yet.
UI unit Tests11 tests 11 ✅ 0s ⏱️ Results for commit f2d011a. ♻️ This comment has been updated with latest results. |
Currently the "sign out and sign back in" behavior of the register page still works as it used to, which is not what we want when an admin is creating the user. We'll change that next.
|
@rmunn That button looks good 👍 But before you extract the register page!EDIT...looks like you already did. Unfortunately, this still applied: There has been some discussion on this idea in our agenda doc. It sounds like we don't actually want to extract/use the register page 😬. Feel free to weigh in. |
|
Extracting the register page turned out to be so easy to do, it's not like I wasted all that much time on this if we end up confirming our earlier decision. But I'm starting to feel that our earlier decision was wrong, and we do want a create-user modal on the admin dashboard. Let's discuss it on the issue so we're not discussing it in two places at once. |
Fixes merge conflicts in admin and register pages
When clicking on "Create User" in admin dashboard, we don't want the modal to change the browser title; that should be reserved for the dedicated Register page.
Pushed as a separate commit from the previous one so that the whitespace changes are isolated in their own commit, which should help a little bit with avoiding merge conflicts later.
If an admin is logged in and creating a user on the register page, we set that user's CreatedById to the admin, so that the user will count as a "guest user", just as if their account was created through the bulk add/create users tool.
Implements the TODO comment I had added earlier.
That info is now in the Create Users modal, so we don't need it here.
Now that we've removed the "Register" title from the modal, we should give it a proper title.
|
After discussion with Kevin and Tim, implemented this as a way for admins to create a single guest user (will have the CreatedById field set to the admin's ID). That fits with the design I think Kevin was thinking of in #666 (comment). Other than a little help needed with the CSS classes for the instructions (see PR description), I think this is ready. Marking as ready for review. |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Okay, finished addressing all of Tim's review comments. Implemented UI for allowing usernames. I currently do not validate usernames because we haven't yet decided on a rule; I could say "only lowercase alphanumeric plus underscore", but that might be too restrictive. We might want to allow hyphens and/or dots, for example, and we might want to allow uppercase (though I'd rather not, so that we don't have to worry about case collisions between rmunn and Rmunn and RMunn). Anyway, this is ready to review now, even though the usernames aren't validating yet, because validating them is going to be a very minor change. @hahn-kev - Want to give this a re-review? |
|
Regarding login validation...
So, I think it would be fine to just continue to use the pattern we alrady use for bulk-create. |
As long as we don't add @ to the usernameRe check, the check for
`value.contains('@')` is redundant, so let's get rid of it. We'll also
rearrange the check to have `isEmail` first as that's easier to read.
The comment mentions errors.username, but we decided not to have a separate field called username. We kept the field called email since that's its primary purpose, but the error message has been adjusted to mention usernames *or* email addresses.
This lets us get rid of the autoLogin prop as well, as if you don't want autoLogin you just don't implement an on:submitted handler.
If you're logged in as an admin, we trust you and don't expect you to be participating in DDOS attacks or anything else that would require a turnstile token.
Instaed of passing the name of the endpoint to handle into CreateUser, we will now pass in the function responsible for submitting the data to the endpoint. This will make it easier to expand the list of endpoints in the future should we decide to.
…eate-users Fixes merge conflict in locales/en.json.
|
@myieye - Commit f027c6b should address your review comments. I left that conversation open in case what I did wasn't what you had in mind. Let me know. Also, I merged develop into this branch to fix the merge conflict in en.json resulting from that last change. Once you're happy with the error message, I'm looking forward to finally getting this merged. |
Fixes #666.
Extracted the relevant parts of the register page into a component and put it in an admin-only "Create User" modal. Help text at the top of the modal links to the relevant parts of the (new-ish) Scribe documentation.
Help needed
Need to find the right CSS classes to format the text at the top of the modal so it looks good. Right now it's ugly: there's no blank space between the two paragraphs, for example.
Screenshots
UI design on large screens:
UI design on small screens:
This is basically exactly the same as how the Projects tab and its "Create Project" button are presented.
Modal design: