Skip to content

feat: create and modify organization groups #13887

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Jul 22, 2024
Merged

feat: create and modify organization groups #13887

merged 14 commits into from
Jul 22, 2024

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Jul 12, 2024 •
edited
Loading

Screenshot 2024-07-18 at 12 12 57 PM Screenshot 2024-07-18 at 12 13 08 PM Screenshot 2024-07-18 at 12 13 17 PM

Allow creating groups, managing members, deleting groups, changing settings. All the usual groups stuff, just now manageable in multi-org configurations!

@Kira-Pilot Kira-Pilot mentioned this pull request Jul 16, 2024
Closed
12 tasks
@aslilac aslilac changed the title 🫣 feat: create and modify organization groups Jul 17, 2024
@aslilac aslilac marked this pull request as ready for review July 18, 2024 18:11
@aslilac aslilac requested review from BrunoQuaresma and jaaydenh and removed request for BrunoQuaresma July 18, 2024 18:11
jaaydenh

This comment was marked as resolved.

Sign in to view

BrunoQuaresma
BrunoQuaresma reviewed Jul 19, 2024
View reviewed changes
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

The code looks good! I'm missing tests for the interactive parts but I will leave it up to you to decide when to add them.

@BrunoQuaresma
Copy link
Collaborator

  1. When accessing the groups settings, I see the error "Route not found". We probably should omit this error for non-enterprise users.
Screenshot 2024-07-19 at 10 26 27
  1. When I try to add a user that is outside of the organization, I get a non-user-friendly error message. In this case, we can improve the error message and only display valid user options in the autocomplete.
Screenshot 2024-07-19 at 10 32 52
  1. Sometimes I felt "lost" in the groups settings navigation. The user can navigate into groups, select a group, and navigate to other sub-pages like "new group" and "group settings". For nested pages like these, I think is nice to have a breadcrumb somewhere on the page, maybe on the page header on top of the title.

@aslilac
Copy link
Member Author

aslilac commented Jul 19, 2024

only display valid user options in the autocomplete

This is a similar problem to not showing users that are already added. We'd need to do a major refactor of the UserAutocomplete component to support these features. It would be really nice polish, but is out of scope for this PR.

@aslilac
Copy link
Member Author

aslilac commented Jul 19, 2024

I think is nice to have a breadcrumb somewhere on the page, maybe on the page header on top of the title.

This could be a nice bit of polish too, but will also have to wait until later.

@aslilac aslilac requested a review from BrunoQuaresma July 19, 2024 19:40
@aslilac aslilac merged commit 0a71c34 into main Jul 22, 2024
32 checks passed
@aslilac aslilac deleted the org-groups branch July 22, 2024 15:47
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jaaydenh jaaydenh jaaydenh left review comments

@BrunoQuaresma BrunoQuaresma BrunoQuaresma approved these changes

Assignees

@aslilac aslilac

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aslilac @BrunoQuaresma @jaaydenh