Fix Barrios/Register 500 + revoke Google access on team soft-delete (#498, #494)#227
Fix Barrios/Register 500 + revoke Google access on team soft-delete (#498, #494)#227peterdrier wants to merge 3 commits intomainfrom
Conversation
Promote QA batch: service ownership, dashboard cluster, rota partials
…ies-collective#498) SyncBarrioLeadsMembershipForUserAsync now short-circuits when the user already has an active team_members row on the Barrio Leads system team, avoiding an IX_team_members_active_unique violation during a second camp registration. Also adds a DbUpdateException catch to CampController.Register as a belt-and-suspenders so any future race surfaces as a friendly ModelState error rather than an unhandled 500. Covered by two new SystemTeamSyncJob tests on the idempotent path. Closes nobodies-collective#498 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…#494) TeamService.DeleteTeamAsync now closes out all active team_members rows (LeftAt = now) and delegates Google resource deactivation to ITeamResourceService.DeactivateResourcesForTeamAsync, a new method owned by TeamResourceService (the sole writer for google_resources). It flips IsActive on every active resource for the team and writes a GoogleResourceDeactivated audit log entry for each. Actual revocation of Drive permissions and Group membership happens on the next GoogleWorkspaceSyncService tick via the existing remove-user paths. Adds a belt-and-suspenders r.Team.IsActive filter to the bulk Google sync queries (SyncResourcesByType, Group settings drift, UpdateDriveFolderPaths, EnforceInheritedAccessRestrictions, and the per-team add-user path) so a future bug in the delete path cannot re-leak access to inactive teams. Covered by new unit tests: - TeamServiceTests.DeleteTeamAsync_* verify membership closure and that the owning service is called (no direct DB on google_resources). - TeamResourceServiceDeactivateTests verify IsActive flips and audit log entries are written for each previously-active resource. Closes nobodies-collective#494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The preview deployment for humans-qa is ready. 🟢 Open Preview | Open Build Logs | Open Application Logs Last updated at: 2026-04-15 02:24:13 CET |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 388ffdd5cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .ThenInclude(t => t.Members.Where(tm => tm.LeftAt == null)) | ||
| .ThenInclude(tm => tm.User) | ||
| .Where(r => r.ResourceType == resourceType && r.IsActive) | ||
| .Where(r => r.ResourceType == resourceType && r.IsActive && r.Team.IsActive) |
There was a problem hiding this comment.
SyncResourcesByTypeAsync now filters resources with r.Team.IsActive, but DeleteTeamAsync marks the team inactive before the next sync tick (TeamService.DeleteTeamAsync). That means resources for deleted teams are excluded from the reconciliation pass, so the remove paths (RemoveUserFromGroupAsync / RemoveUserFromDriveAsync reached via execute sync actions) never run for those memberships. In practice, users who already had Google Group/Drive access can retain it after a soft-delete, which defeats the intended access revocation behavior.
Useful? React with 👍 / 👎.
1 participant
Summary
Batch 3 of sprint 20260415 — two Camps/Google-sync fixes.
nobodies-collective#498 — /Barrios/Register 500 on duplicate Barrio Leads membership
Registering a second camp as an existing Barrio Leads member crashed with an IX_team_members_active_unique violation because SyncBarrioLeadsMembershipForUserAsync unconditionally inserted a TeamMember row.
Closes nobodies-collective#498
nobodies-collective#494 — Revoke Google resource access when a team is soft-deleted
Soft-deleting a team only flipped Team.IsActive. TeamMembers.LeftAt stayed null and GoogleResources kept IsActive = true, so the hourly sync job continued provisioning Drive and Group access for a deleted team indefinitely.
Closes nobodies-collective#494
Test plan
Note: Humans.Integration.Tests require a running Docker daemon and could not be executed in this worktree. Unit coverage is in Humans.Application.Tests.