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

Improve usability of organisation select on batch_invitations/new #2430

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

chrislo
Copy link
Contributor

@chrislo chrislo commented Oct 11, 2023

Trello: https://trello.com/c/gJkINQst

This makes it easier to select the organisation for a batch of users by excluding closed organisations and sorting the remaining ones alphabetically. We did something similar in #2426 and #2417 - there's enough subtle differences in those two select boxes (indicating a current organisation and adding a "None" option respectively) that I haven't attempted to extract the duplication. I think the new scopes are self-explanatory enough that I'm happy to keep them inline in this view.

@chrislo chrislo requested a review from floehopper October 11, 2023 14:06
@floehopper floehopper self-assigned this Oct 11, 2023
Copy link
Contributor

@floehopper floehopper left a comment

Choose a reason for hiding this comment

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

This looks good to me 👍

It would be nice to add a test (e.g. in BatchInvitationsControllerTest) to check we're not rendering options for closed organisations. However, I'm going to mark this as approved and you can decide whether it's worthwhile.

This makes it easier to select the organisation for a batch of users
by excluding closed organisations and sorting the remaining ones
alphabetically. We did something similar in #2426 and #2417 - there's
enough subtle differences in those two select boxes (indicating a
current organisation and adding a "None" option respectively) that I
haven't attempted to extract the duplication. I think the new scopes
are self-explanatory enough that I'm happy to keep them inline in this
view.
@chrislo chrislo force-pushed the improve-organisation-select-on-new-batch-upload-page branch from e47cec0 to 330d660 Compare October 11, 2023 14:20
@chrislo
Copy link
Contributor Author

chrislo commented Oct 11, 2023

Thanks @floehopper - I added a controller test as you suggested, good idea!

@chrislo chrislo enabled auto-merge October 11, 2023 14:24
@chrislo chrislo merged commit c60b886 into main Oct 11, 2023
6 checks passed
@chrislo chrislo deleted the improve-organisation-select-on-new-batch-upload-page branch October 11, 2023 14:31
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.

2 participants