Add the delete group endpoint, Owner only #51

Merged
rob merged 2 commits from feat/delete-group-endpoint into main 2026-08-05 03:21:18 +00:00
Owner

Implements task 77: DELETE /api/groups/{groupId}, Owner only, 204 on success.

  • Cascade to places and memberships proved with real rows (fk_places_groups_group_id, fk_group_memberships_groups_group_id), including a multi-member group with a pending invitee.
  • Editor/Viewer → 403, non-member/nonexistent group → 404, unauthenticated → 401.
  • Personal group → 409 Conflict, new decision recorded as ADR-0047 (takes ADR-0047; next free number checked against main and every open PR immediately before push — feat/update-place-endpoint took 0048, no collision).
  • docs/data-model.md's "may the personal group be deleted?" moved from open questions to answered, and invariant 1 updated to match.

dotnet format --verify-no-changes, full test suite (Debug and Release build) and the group-scoped authorisation safety net all pass locally under the pinned SDK.

Implements task 77: `DELETE /api/groups/{groupId}`, Owner only, 204 on success. - Cascade to places and memberships proved with real rows (`fk_places_groups_group_id`, `fk_group_memberships_groups_group_id`), including a multi-member group with a pending invitee. - Editor/Viewer → 403, non-member/nonexistent group → 404, unauthenticated → 401. - Personal group → 409 Conflict, new decision recorded as ADR-0047 (takes ADR-0047; next free number checked against `main` and every open PR immediately before push — `feat/update-place-endpoint` took 0048, no collision). - `docs/data-model.md`'s "may the personal group be deleted?" moved from open questions to answered, and invariant 1 updated to match. `dotnet format --verify-no-changes`, full test suite (Debug and Release build) and the group-scoped authorisation safety net all pass locally under the pinned SDK.
Add the delete group endpoint, refusing a personal group
All checks were successful
CI / build (pull_request) Successful in 2m2s
61381990f3
rob left a comment

Verdict: mergeable

Verified with real rows: cascade to places and memberships, atomicity (single DELETE statement carries the cascade, no partial state possible), the personal-group refusal happening before any deletion (removing the IsPersonal check reddens both DeleteAsync_APersonalGroup_LeavesItInPlaceAndReturnsRefusedPersonalGroup and DeleteGroup_APersonalGroup_RespondsWith409AndLeavesItInPlace), Owner/Editor/Viewer/non-member/nonexistent-group authorisation, and that deleting one group leaves another's places and memberships untouched (not covered by a checked-in test, but I added one locally and it passes — see below). ADR-0047 number is genuinely free, index row is in numeric order, docs/data-model.md invariant 1 and the open-questions section are both updated accurately and match what the code does. Build and full test suite are clean under the pinned SDK.

One gap worth closing before or shortly after merge: there's no test proving the cascade is bounded to the deleted group — every existing test deletes the only group in its scenario. Add a case with two groups (each with a place, and ideally a second member) where deleting one leaves the other's places, memberships and row completely intact. This is the one property that, if it silently regressed, destroys a different user's data.

Verdict: mergeable Verified with real rows: cascade to places and memberships, atomicity (single `DELETE` statement carries the cascade, no partial state possible), the personal-group refusal happening before any deletion (removing the `IsPersonal` check reddens both `DeleteAsync_APersonalGroup_LeavesItInPlaceAndReturnsRefusedPersonalGroup` and `DeleteGroup_APersonalGroup_RespondsWith409AndLeavesItInPlace`), Owner/Editor/Viewer/non-member/nonexistent-group authorisation, and that deleting one group leaves another's places and memberships untouched (not covered by a checked-in test, but I added one locally and it passes — see below). ADR-0047 number is genuinely free, index row is in numeric order, `docs/data-model.md` invariant 1 and the open-questions section are both updated accurately and match what the code does. Build and full test suite are clean under the pinned SDK. One gap worth closing before or shortly after merge: there's no test proving the cascade is bounded to the deleted group — every existing test deletes the only group in its scenario. Add a case with two groups (each with a place, and ideally a second member) where deleting one leaves the other's places, memberships and row completely intact. This is the one property that, if it silently regressed, destroys a different user's data.
Add a cascade-boundary test proving the delete stays scoped
All checks were successful
CI / build (pull_request) Successful in 2m23s
7e5994c7e5
rob left a comment

Verdict: mergeable

Independently confirmed: widened DeleteGroupById to WHERE id = @Id OR personal_for_user_id IS NULL and the new boundary test reddened on the surviving group's row ((object)await SelectGroupAsync(survivingGroupId) should not be null but was); reverted, and the full DeleteGroupEndpointTests class passes clean again. The test asserts the survivor row-by-row — group presence, place's group_id/name/latitude/longitude, both memberships' role and status individually, not a count. Delta is scoped to the test file only, nothing else in the diff.

Verdict: mergeable Independently confirmed: widened `DeleteGroupById` to `WHERE id = @Id OR personal_for_user_id IS NULL` and the new boundary test reddened on the surviving group's row (`(object)await SelectGroupAsync(survivingGroupId) should not be null but was`); reverted, and the full `DeleteGroupEndpointTests` class passes clean again. The test asserts the survivor row-by-row — group presence, place's `group_id`/`name`/`latitude`/`longitude`, both memberships' `role` and `status` individually, not a count. Delta is scoped to the test file only, nothing else in the diff.
rob merged commit 1e98d5df50 into main 2026-08-05 03:21:18 +00:00
rob deleted branch feat/delete-group-endpoint 2026-08-05 03:21:19 +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!51
No description provided.