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

Add "Group type" field to group creation form #8993

Merged
merged 1 commit into from
Oct 9, 2024
Merged

Conversation

robertknight
Copy link
Member

@robertknight robertknight commented Oct 3, 2024

This allows setting the group type to "private", "restricted" or "open" when creating or editing a group.

This PR does not implement the part of the design where a warning is shown when changing the type of an existing group.

See #8898.

Testing:

  1. Enable the group_type feature flag
  2. Go to http://localhost:5000/groups/new
  3. You should see new UI options to set the group type
  4. Pick one of the group types other than "Private" and create the group
  5. Go to edit the group and you should see the chosen group type selected
  6. Change the group type in the edit form and click "Save changes"
  7. Refresh the page and the group type should persist

@@ -195,10 +210,17 @@ export default function CreateEditGroupForm() {
let response: CreateUpdateGroupAPIResponse;

try {
const body: CreateUpdateGroupAPIRequest = {
Copy link
Member Author

Choose a reason for hiding this comment

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

This ensures that the request body is compatible with the schema specified in api.ts (if not, TS will complain).

{config.features.group_type && (
<>
<Label id={groupTypeLabel} text="Group type" />
<RadioGroup<GroupType>
Copy link
Member Author

@robertknight robertknight Oct 3, 2024

Choose a reason for hiding this comment

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

The RadioGroup component is parametrized by the type of value the user can select. In some cases this can be inferred (eg. from the value passed to selected), but here it needed to be specified.

const radioGroup = wrapper.find('[data-testid="group-type"]');
act(() => {
radioGroup.prop('onChange')(newType);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

Using act here ensures that the form is immediately re-rendered after we change the state by toggling the radio buttons.

@robertknight robertknight requested a review from seanh October 3, 2024 10:39
@seanh seanh self-assigned this Oct 8, 2024
Base automatically changed from group-type-ui-flag to main October 8, 2024 14:27
This allows setting the group type to "private", "restricted" or "open" when
creating or editing a group.

See #8898.
@seanh
Copy link
Contributor

seanh commented Oct 8, 2024

One issue I noticed: if a confirmation tick is shown next to the Save changes button because a previous change has already been saved, then changing the selected radio box doesn't clear the tick like changing one of the text fields does. This means you end up clicking Save changes when the tick is already still present which is a little weird.

@robertknight
Copy link
Member Author

One issue I noticed: if a confirmation tick is shown next to the Save changes button because a previous change has already been saved, then changing the selected radio box doesn't clear the tick like changing one of the text fields does.

Ah, the form's save state is not reset. I'll address this as part of #8994.

@robertknight robertknight merged commit d473c4a into main Oct 9, 2024
9 checks passed
@robertknight robertknight deleted the group-type-field branch October 9, 2024 08:07
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