Add the create/rename group form component #69

Merged
rob merged 1 commit from feat/group-form into main 2026-08-05 18:23:52 +00:00
Owner

Implements task 90: GroupForm, one component for creating a group and renaming one — name only, groups have no description column (task 90's own correction).

  • Validates client-side by running CreateGroupRequest/UpdateGroupRequest's own data annotations (task 56's pattern), no restated copy.
  • Owner-only rename: the form isn't rendered to a caller whose CallerIsOwner is false, but that's a courtesy — the API's own 403 is still handled if the host's view of the role is stale.
  • 409 is told apart from a 400 by ApiProblemException.StatusCode/Errors, not by matching Detail text (task 155 is still open), and renders under a distinct .form-conflict class.
  • A personal group can still be renamed (ADR-0018/0047 forbid sharing and deleting, not renaming).

Navigation decision: no navigation on success. The groomed criterion ("navigates to the group detail page") predates the map becoming the home page (task 88, ADR-0059's "stay where you are"), and task 91 (group detail) hasn't been reshaped. The component raises OnSaved(GroupResponse) and leaves what happens next to whichever page hosts it. Reasoning is on GroupForm's own XML doc.

No ADR: both decisions above apply an existing one (ADR-0059's "stay put" direction; ADR-0058's status-code-not-prose pattern) rather than making a new one.

Implements task 90: `GroupForm`, one component for creating a group and renaming one — name only, groups have no description column (task 90's own correction). - Validates client-side by running `CreateGroupRequest`/`UpdateGroupRequest`'s own data annotations (task 56's pattern), no restated copy. - Owner-only rename: the form isn't rendered to a caller whose `CallerIsOwner` is false, but that's a courtesy — the API's own 403 is still handled if the host's view of the role is stale. - 409 is told apart from a 400 by `ApiProblemException.StatusCode`/`Errors`, not by matching `Detail` text (task 155 is still open), and renders under a distinct `.form-conflict` class. - A personal group can still be renamed (ADR-0018/0047 forbid sharing and deleting, not renaming). **Navigation decision:** no navigation on success. The groomed criterion ("navigates to the group detail page") predates the map becoming the home page (task 88, ADR-0059's "stay where you are"), and task 91 (group detail) hasn't been reshaped. The component raises `OnSaved(GroupResponse)` and leaves what happens next to whichever page hosts it. Reasoning is on `GroupForm`'s own XML doc. No ADR: both decisions above apply an existing one (ADR-0059's "stay put" direction; ADR-0058's status-code-not-prose pattern) rather than making a new one.
Add the create/rename group form component
All checks were successful
CI / build (pull_request) Successful in 2m22s
9524d12f3f
rob left a comment

Verdict: mergeable

Verified rather than read:

  • Contract-driven validation: mutated CreateGroupRequest.Name's [StringLength] from 100 to 5 with no component change; the "Coffee shops" create test then correctly failed local validation and made no API call. Reverted. Confirms no restated rule.
  • 409 vs 400: ApiProblemDetails.Errors is populated only on the API's own 400s (per ADR-0058/ADR-0025), so Errors.Count > 0 and StatusCode == Conflict can't both misfire — the branch is sound, not just conveniently untested. .form-conflict differs visually from .form-error by a real border, not just a class name, and both carry role="alert".
  • 403/stale-role path: full suite run (GroupFormTests, 8 tests) passes, including GroupForm_ApiRefusesWithForbiddenDespiteCallerIsOwner_..., which stubs a genuine 403 with CallerIsOwner: true and asserts the general-error path renders rather than crashing or silently succeeding.
  • Personal group rename: no IsPersonal special-casing anywhere in GroupForm.razor.cs; GroupForm_RenamePersonalGroupAsOwner_... exercises it directly.
  • No raw HttpClient: NoRawHttpClientInComponentsTests passes against this branch, including its own non-vacuity check (WebUiSourceInsideTheApiClient_ReachesARawHttpClientSomewhere), so the guard is genuinely exercised, not merely present.
  • Full solution: builds clean under the pinned SDK (10.0.100, confirmed against global.json); PlaceMark.WebUI.Tests (155), PlaceMark.Architecture.Tests (15) and PlaceMark.Contracts.Tests (117) all pass. main hadn't moved since the branch was cut, so no merge was needed.

One non-blocking accessibility gap: FieldErrors' <ul> carries no id, so nothing in GroupForm (or Login/Register, which share the component) can point an input at its error via aria-describedby — the error is proximate, not associated. role="alert" does get it announced on submit, so this isn't the mouse-only class of defect task 139 named, but a screen-reader user tabbing back to a field with an existing error won't be told why it's flagged. This is inherited from an already-merged pattern (Login/Register), not introduced here, so it doesn't block this PR — worth a follow-up ticket against FieldErrors itself rather than a one-off fix in this component.

What bUnit cannot show: no browser was available, so the actual screen-reader announcement of role="alert", the visual legibility of .form-conflict's border against .form-error, and any focus/keyboard behaviour are asserted from markup and CSS source only, not observed.

Verdict: mergeable Verified rather than read: - Contract-driven validation: mutated `CreateGroupRequest.Name`'s `[StringLength]` from 100 to 5 with no component change; the "Coffee shops" create test then correctly failed local validation and made no API call. Reverted. Confirms no restated rule. - 409 vs 400: `ApiProblemDetails.Errors` is populated only on the API's own 400s (per ADR-0058/ADR-0025), so `Errors.Count > 0` and `StatusCode == Conflict` can't both misfire — the branch is sound, not just conveniently untested. `.form-conflict` differs visually from `.form-error` by a real border, not just a class name, and both carry `role="alert"`. - 403/stale-role path: full suite run (`GroupFormTests`, 8 tests) passes, including `GroupForm_ApiRefusesWithForbiddenDespiteCallerIsOwner_...`, which stubs a genuine 403 with `CallerIsOwner: true` and asserts the general-error path renders rather than crashing or silently succeeding. - Personal group rename: no `IsPersonal` special-casing anywhere in `GroupForm.razor.cs`; `GroupForm_RenamePersonalGroupAsOwner_...` exercises it directly. - No raw `HttpClient`: `NoRawHttpClientInComponentsTests` passes against this branch, including its own non-vacuity check (`WebUiSourceInsideTheApiClient_ReachesARawHttpClientSomewhere`), so the guard is genuinely exercised, not merely present. - Full solution: builds clean under the pinned SDK (10.0.100, confirmed against `global.json`); `PlaceMark.WebUI.Tests` (155), `PlaceMark.Architecture.Tests` (15) and `PlaceMark.Contracts.Tests` (117) all pass. `main` hadn't moved since the branch was cut, so no merge was needed. One non-blocking accessibility gap: `FieldErrors`' `<ul>` carries no `id`, so nothing in `GroupForm` (or `Login`/`Register`, which share the component) can point an input at its error via `aria-describedby` — the error is proximate, not associated. `role="alert"` does get it announced on submit, so this isn't the mouse-only class of defect task 139 named, but a screen-reader user tabbing back to a field with an existing error won't be told why it's flagged. This is inherited from an already-merged pattern (`Login`/`Register`), not introduced here, so it doesn't block this PR — worth a follow-up ticket against `FieldErrors` itself rather than a one-off fix in this component. What bUnit cannot show: no browser was available, so the actual screen-reader announcement of `role="alert"`, the visual legibility of `.form-conflict`'s border against `.form-error`, and any focus/keyboard behaviour are asserted from markup and CSS source only, not observed.
rob merged commit 127e787084 into main 2026-08-05 18:23:52 +00:00
rob deleted branch feat/group-form 2026-08-05 18:23:52 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/PlaceMark!69
No description provided.