diff --git a/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md b/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md index f945d12..80cc2ca 100644 --- a/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md +++ b/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md @@ -61,9 +61,16 @@ **Files:** - Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs` +- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs` +- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresFixture.cs` +- Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs` - Create: `src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql` +- Modify: `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs` +- Modify: `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs` +- Modify: `tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj` +- Modify: `tests/GmRelay.Bot.Tests/packages.lock.json` -- [ ] **Step 1: Write the failing migration source-contract test** +- [ ] **Step 1: Write the failing migration and session-deletion source-contract tests** Add tests that read `V029__add_completed_game_portfolios_and_reviews.sql` and assert: @@ -115,17 +122,90 @@ public async Task MigrationV029_ShouldStoreProviderNeutralCoverKeys() } ``` -- [ ] **Step 2: Run the migration test to verify RED** +Add `PortfolioSessionDeletionSourceTests.cs`. Normalize whitespace before comparing source text and assert that both session-deletion paths explicitly unpublish linked cards before deleting the required session link: + +```csharp +[Fact] +public async Task SharedDeleteSessionHandler_ShouldUnpublishLinkedPortfolioCardBeforeDeletingSession() +{ + var source = NormalizeSql(await ReadRepositoryFileAsync( + "src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs")); + + const string unpublish = + "UPDATE portfolio_games pg SET is_public = false, updated_at = now() FROM portfolio_game_sessions pgs WHERE pgs.portfolio_game_id = pg.id AND pgs.session_id = @SessionId AND pg.is_public = true"; + + Assert.Contains(unpublish, source, StringComparison.Ordinal); + Assert.True( + source.IndexOf(unpublish, StringComparison.Ordinal) < + source.IndexOf("DELETE FROM sessions WHERE id = @Id", StringComparison.Ordinal)); +} + +[Fact] +public async Task DiscordDeleteSessionHandler_ShouldUnpublishOnlyCardsFromTheInteractionGuildBeforeDeletingSession() +{ + var source = NormalizeSql(await ReadRepositoryFileAsync( + "src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs")); + + const string unpublish = + "UPDATE portfolio_games pg SET is_public = false, updated_at = now() FROM portfolio_game_sessions pgs JOIN sessions s ON s.id = pgs.session_id JOIN game_groups g ON g.id = s.group_id WHERE pgs.portfolio_game_id = pg.id AND s.id = @SessionId AND g.platform = 'Discord' AND g.external_group_id = @GuildId AND pg.is_public = true"; + + Assert.Contains(unpublish, source, StringComparison.Ordinal); + Assert.True( + source.IndexOf(unpublish, StringComparison.Ordinal) < + source.IndexOf("DELETE FROM sessions s", StringComparison.Ordinal)); +} +``` + +- [ ] **Step 2: Add the failing PostgreSQL Testcontainers integration fixture and tests** + +Add the package reference: + +```xml + +``` + +Update the locked dependency graph: + +```powershell +dotnet restore tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --use-lock-file +``` + +Create `PortfolioMigrationPostgresFixture.cs` with a shared `PostgreSqlContainer` built from `postgres:17-alpine`. For each test, create a fresh database and apply migration files `V001` through `V029` in ordinal filename order. + +Create `PortfolioMigrationPostgresTests.cs` with these executable scenarios: + +```csharp +[Fact] +public async Task MigrationsV001ThroughV029_ShouldApplyToPostgres17() + +[Theory] +[InlineData("portfolio_game_sessions")] +[InlineData("portfolio_game_masters")] +public async Task DirectRequiredLinkDeletion_ShouldFailCommitForPublishedCard(string linkTable) + +[Fact] +public async Task ExplicitUnpublishThenSessionDelete_ShouldCommitAndPreserveFirstPublishedAt() + +[Fact] +public async Task ConcurrentPublishAndLinkDelete_ShouldNotDeadlockOrCommitInvalidPublicCard() + +[Fact] +public async Task ParentCardAndGroupCascadeDeletes_ShouldCommit() +``` + +The direct-delete theory must expect PostgreSQL `23514` at commit for each required-link table. The explicit-unpublish scenario must delete the session successfully while preserving the first `published_at`. The concurrency scenario must bound both commits with timeouts, prove there is no deadlock, and prove that an invalid public card cannot commit. The parent-card and owning-group cascade scenarios must commit successfully. + +- [ ] **Step 3: Run the Task 1 tests to verify RED** Run: ```powershell -dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests" +dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests" ``` -Expected: FAIL because `V029__add_completed_game_portfolios_and_reviews.sql` does not exist. +Expected: FAIL because `V029__add_completed_game_portfolios_and_reviews.sql` does not exist and the session-deletion handlers do not explicitly unpublish linked portfolio cards before deleting sessions. -- [ ] **Step 3: Add migration V029** +- [ ] **Step 4: Add migration V029** Create the migration with these exact tables and indexes: @@ -265,15 +345,53 @@ CREATE INDEX ix_portfolio_game_reviews_pending The deferred constraint triggers retain the link-table `ON DELETE CASCADE` behavior. At transaction commit they reject a surviving published card when either required link set is empty. Child delete triggers do not lock or update the parent card, avoiding reverse lock order. Normal session-deletion handlers explicitly unpublish linked cards before deleting sessions. Card and club cascade deletion remain harmless because no published parent survives validation. -- [ ] **Step 4: Run the migration tests to verify GREEN** +- [ ] **Step 5: Explicitly unpublish linked cards in both session-deletion handlers** -Run the Task 1 command again. Expected: PASS. +In `src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs`, run this statement inside the existing transaction after authorization and before `DELETE FROM sessions`: -- [ ] **Step 5: Commit** +```sql +UPDATE portfolio_games pg +SET is_public = false, + updated_at = now() +FROM portfolio_game_sessions pgs +WHERE pgs.portfolio_game_id = pg.id + AND pgs.session_id = @SessionId + AND pg.is_public = true +``` + +In `src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs`, start a transaction before deleting. Run this guild-scoped unpublish statement before the existing guild-scoped `DELETE FROM sessions`, then commit: + +```sql +UPDATE portfolio_games pg +SET is_public = false, + updated_at = now() +FROM portfolio_game_sessions pgs +JOIN sessions s ON s.id = pgs.session_id +JOIN game_groups g ON g.id = s.group_id +WHERE pgs.portfolio_game_id = pg.id + AND s.id = @SessionId + AND g.platform = 'Discord' + AND g.external_group_id = @GuildId + AND pg.is_public = true +``` + +Both handlers deliberately unpublish before session deletion. This keeps normal deletes successful, preserves the first-publication `published_at`, and leaves the deferred trigger as the direct-SQL and concurrency backstop. + +- [ ] **Step 6: Run the Task 1 tests to verify GREEN** + +Run: ```powershell -git add src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs -git commit -m "feat(data): add completed game portfolio schema" +dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj --filter "FullyQualifiedName~PortfolioMigrationTests|FullyQualifiedName~PortfolioMigrationPostgresTests|FullyQualifiedName~PortfolioSessionDeletionSourceTests" +``` + +Expected: PASS, including PostgreSQL 17 migration application, rejected direct required-link deletes, successful explicit unpublish plus session delete with preserved `published_at`, bounded concurrent publish/delete without deadlock or invalid public commit, and successful parent-card and owning-group cascades. + +- [ ] **Step 7: Commit** + +```powershell +git add src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql src/GmRelay.Shared/Features/Sessions/ListSessions/DeleteSessionHandler.cs src/GmRelay.DiscordBot/Features/Sessions/DiscordDeleteSessionHandler.cs tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj tests/GmRelay.Bot.Tests/packages.lock.json tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioSessionDeletionSourceTests.cs tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresFixture.cs tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs +git commit -m "fix(data): harden portfolio publication concurrency" ``` ---