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

ui/users: create user dialog #1536

Merged
merged 26 commits into from
May 20, 2021
Merged

ui/users: create user dialog #1536

merged 26 commits into from
May 20, 2021

Conversation

dctalbot
Copy link
Contributor

@dctalbot dctalbot commented May 13, 2021

  • Identified the issue which this PR solves.
  • Read the CONTRIBUTING document.
  • Code builds clean without any errors or warnings.
  • Added appropriate tests for any new functionality.
  • All new and existing tests passed.
  • Added comments in the code, where necessary.
  • Ran make check to catch common errors. Fixed any that came up.

Description:
Currently, goalert admins can only create users for basic auth via the CLI. This PR brings that functionality into the UI.

Part of #1247

Screenshots:
Screen Shot 2021-05-18 at 8 30 17 AM

Screen Shot 2021-05-17 at 5 20 31 PM

Describe any introduced user-facing changes:
Admins will have a create user fab on the users list page

Describe any introduced API changes:

  createUser(input: CreateUserInput!): User
input CreateUserInput {
  username: String!
  password: String!
  name: String
  email: String
  role: UserRole
}

Additional Info:

  • This functionality may move into a central user management page in the future

@mastercactapus
Copy link
Member

A couple of initial thoughts:

  • For role, maybe just an Admin checkmark in the dialog to replicate the --admin flag in CLI rather than making it a multi-step process
  • Still allow creating a user if Basic auth is disabled, but add a notice to the dialog that they will be unable to login until it's turned on

@dctalbot dctalbot requested a review from Forfold May 17, 2021 22:21
@dctalbot dctalbot requested a review from mastercactapus May 18, 2021 21:20
Copy link
Contributor

@Forfold Forfold left a comment

Choose a reason for hiding this comment

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

Code looks good, but functionally I'm seeing field errors for username show up under the display name field instead

@dctalbot
Copy link
Contributor Author

dctalbot commented May 19, 2021

Code looks good, but functionally I'm seeing field errors for username show up under the display name field instead

Steps to reproduce? I'm not seeing that @Forfold

@Forfold
Copy link
Contributor

Forfold commented May 19, 2021

For sure:

  1. Set username to something "test " (note the space at the end)
  2. Fill in rest of required fields
  3. Attempt submit

Screen Shot 2021-05-19 at 1 25 03 PM

@dctalbot dctalbot requested a review from Forfold May 19, 2021 21:52
dctalbot and others added 2 commits May 20, 2021 11:24
Copy link
Contributor

@Forfold Forfold left a comment

Choose a reason for hiding this comment

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

lgtm!

@dctalbot dctalbot requested a review from mastercactapus May 20, 2021 15:35
@dctalbot dctalbot merged commit 2f06977 into master May 20, 2021
@dctalbot dctalbot deleted the create-user branch May 20, 2021 16:09
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