feat: require an organization name on edit and make memberships editable - #217
Merged
Conversation
The organization edit form submitted the name without checking it was non-empty and its save control was not disabled for an empty value, so clearing the field and saving wrote an empty name straight through and left the organization unidentifiable in every list. The create form already guarded this. The edit form now matches it, reports the problem inline, and disables save. Member roles come from a constrained selector built from the roles the instance actually defines, replacing free-form comma-separated text. A typo was previously accepted silently, and a mistyped role simply grants nothing. An existing membership can be edited in place through the update hook that already existed but had no interface. Changing someone's role previously meant removing and re-adding them, which discarded their membership history and invited mistakes. The dialog reuses the shared Dialog and RoleChips rather than adding a new modal or a second role control. The organization list has a search field. It filters what the feed returned rather than querying the server, and says so in a comment, because GET /admin/organizations takes no search parameter. Organization deletion and server-side pagination are not included. Neither has a verifiable endpoint from this repo, and a control that appears to work while doing nothing is worse than its absence. #159 stays open for those two.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #143. Partially addresses #159 (see below).
Empty organization name (#143)
handleSubmitsenteditName.trim()with no check, and the save control was not disabled for an empty value, so clearing the field and saving wrote an empty name and left the organization unidentifiable in every list. The create form already guarded this.The edit form now matches it: submission is refused, the field is marked invalid with an inline message, and save is disabled.
Membership management (#159)
Constrained role selector. Member roles were free-form comma-separated text with no validation, so a typo was accepted silently and a mistyped role simply grants nothing. Roles now come from
useRoles(), the same source the user dialogs use, so only roles the instance defines can be submitted.Editing an existing membership.
useUpdateOrganizationMemberalready existed but nothing in the interface called it, so changing someone's role meant removing and re-adding them, which discarded their membership history. There is now an Edit action on each member row opening a dialog for roles and scopes. It reuses the sharedDialogandRoleChipsrather than adding a second modal or a second role control.Search. The organization list has a search field over name and slug.
What is not in this PR, and why
The issue also asks for organization deletion and pagination on the list. Neither is here:
useOrganizationsexposes no delete mutation, and I could not confirm from this repo that aDELETE /admin/organizations/:idroute exists on the API or is forwarded by the server adapter.AGENTS.mdis explicit that destructive admin mutations must be checked against the upstream API and the adapter together before the dashboard calls them, and a delete that 404s after a confirmation dialog is worse than no delete.GET /admin/organizationsis called with no parameters and I have no evidence it acceptslimit/offset. The search here therefore filters the loaded page and there is a comment saying so, rather than a pager that appears to work and silently does nothing.I have left #159 open. Confirm the delete route and the list parameters (or point me at the API repo) and I will finish both in a follow-up.
Checks
npm run lint,npm run typecheck,npm run format:check, andnpm testall pass (397 tests). New cases: only instance-defined roles offered, the empty-name guard, and the list filter. The existing add-member test moved from typing"admin, member"into a text field to selecting from the constrained control, which is the fix landing.