From 81c04e086659340e841fd2d5ee3b25a1f30de4f8 Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Sat, 7 Feb 2026 00:36:04 +0100 Subject: [PATCH] Refactor phase reads to pure lookups and align admin docs --- API.md | 4 +-- Endpoints/AdminEndpoints.cs | 7 ++-- Endpoints/EndpointHelpers.cs | 39 +++++++++++++-------- Endpoints/ResultsEndpoints.cs | 3 +- Endpoints/StateEndpoints.cs | 19 +++++++---- Endpoints/SuggestEndpoints.cs | 9 ++--- Endpoints/VoteEndpoints.cs | 7 ++-- GameList.Tests/FiltersTests.cs | 28 ++++++++++++++- GameList.Tests/StateTests.cs | 43 +++++++++++++++++++++--- GameList.csproj | 1 + Infrastructure/PhaseOrJokerFilter.cs | 3 +- Infrastructure/PhaseRequirementFilter.cs | 3 +- 12 files changed, 124 insertions(+), 42 deletions(-) diff --git a/API.md b/API.md index ee15c16..424be51 100644 --- a/API.md +++ b/API.md @@ -1,6 +1,6 @@ # API Contract (auth-enabled) -All endpoints are JSON. Most routes require the HttpOnly `player` cookie issued after register/login. Admin access is granted via authenticated admin user or `X-Admin-Key`/`key` matching `ADMIN_PASSWORD`. +All endpoints are JSON. Most routes require the HttpOnly `player` cookie issued after register/login. Admin access is granted only via an authenticated admin user session (`IsAdmin=true` on the account). ## Auth POST /api/auth/register — accepts optional `adminKey` to set `IsAdmin=true` @@ -31,7 +31,7 @@ POST /api/votes/finalize — `{ final: bool }` toggles caller’s finalized stat ## Results (requires auth + Results phase + resultsOpen) GET /api/results — leaderboard with totals, counts, averages, caller’s vote, media/links, link metadata -## Admin (admin auth or admin key) +## Admin (requires authenticated admin user) POST /api/admin/results — `{ resultsOpen: bool }` locks/unlocks results and aligns player phases GET /api/admin/vote-status — readiness overview (who finalized) POST /api/admin/link-suggestions — `{ sourceSuggestionId, targetSuggestionId }`; merges vote groups during Vote, clears votes in the linked group, unfinalizes **all** players diff --git a/Endpoints/AdminEndpoints.cs b/Endpoints/AdminEndpoints.cs index cc41351..8a8d01d 100644 --- a/Endpoints/AdminEndpoints.cs +++ b/Endpoints/AdminEndpoints.cs @@ -57,7 +57,7 @@ public static class AdminEndpoints if (player is null) return Results.NotFound(new { error = "Player not found." }); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Vote) return Results.BadRequest(new { error = "Player must be in the Vote phase to receive a joker." }); @@ -108,7 +108,7 @@ public static class AdminEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Vote) return EndpointHelpers.PhaseMismatch(Phase.Vote, phase); @@ -172,7 +172,7 @@ public static class AdminEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Vote) return EndpointHelpers.PhaseMismatch(Phase.Vote, phase); @@ -260,3 +260,4 @@ public static class AdminEndpoints }); } } + diff --git a/Endpoints/EndpointHelpers.cs b/Endpoints/EndpointHelpers.cs index 6258c48..e4b993d 100644 --- a/Endpoints/EndpointHelpers.cs +++ b/Endpoints/EndpointHelpers.cs @@ -34,42 +34,53 @@ internal static class EndpointHelpers return existing; } - public static async Task GetPhase(AppDbContext db, Guid playerId) + public static async Task GetCurrentPhaseAsync(AppDbContext db, Guid playerId) { - var player = await db.Players.FirstOrDefaultAsync(p => p.Id == playerId); - if (player is null) + var playerPhase = await db.Players + .AsNoTracking() + .Where(p => p.Id == playerId) + .Select(p => (Phase?)p.CurrentPhase) + .FirstOrDefaultAsync(); + if (playerPhase is null) return Phase.Suggest; - var state = await db.AppState.FirstAsync(); + var resultsOpen = await db.AppState.AsNoTracking().Select(s => s.ResultsOpen).FirstAsync(); + return GetCurrentPhase(playerPhase.Value, resultsOpen); + } + public static Phase GetCurrentPhase(Phase phase, bool resultsOpen) + { + var normalized = phase == Phase.Reveal ? Phase.Vote : phase; + + if (resultsOpen) + return Phase.Results; + + return normalized == Phase.Results ? Phase.Vote : normalized; + } + + public static bool ReconcilePlayerPhase(Player player, bool resultsOpen) + { var changed = false; - // Auto-upgrade any legacy Reveal phase to Vote to avoid blank screens if (player.CurrentPhase == Phase.Reveal) { player.CurrentPhase = Phase.Vote; changed = true; } - // Keep phases aligned with results availability - if (state.ResultsOpen && player.CurrentPhase != Phase.Results) + if (resultsOpen && player.CurrentPhase != Phase.Results) { player.CurrentPhase = Phase.Results; changed = true; } - else if (!state.ResultsOpen && player.CurrentPhase == Phase.Results) + else if (!resultsOpen && player.CurrentPhase == Phase.Results) { player.CurrentPhase = Phase.Vote; player.VotesFinal = false; changed = true; } - if (changed) - { - await db.SaveChangesAsync(); - } - - return player.CurrentPhase; + return changed; } public static IResult PhaseMismatch(Phase required, Phase current) => diff --git a/Endpoints/ResultsEndpoints.cs b/Endpoints/ResultsEndpoints.cs index 3903fd5..2501bf3 100644 --- a/Endpoints/ResultsEndpoints.cs +++ b/Endpoints/ResultsEndpoints.cs @@ -23,7 +23,7 @@ public static class ResultsEndpoints var appState = await db.AppState.AsNoTracking().FirstAsync(); if (!appState.ResultsOpen) return Results.BadRequest(new { error = "Results are locked until the admin enables them." }); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Results) return EndpointHelpers.PhaseMismatch(Phase.Results, phase); @@ -96,3 +96,4 @@ public static class ResultsEndpoints ); } } + diff --git a/Endpoints/StateEndpoints.cs b/Endpoints/StateEndpoints.cs index a276986..56b452c 100644 --- a/Endpoints/StateEndpoints.cs +++ b/Endpoints/StateEndpoints.cs @@ -16,9 +16,8 @@ public static class StateEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); - var state = await db.AppState.AsNoTracking().FirstAsync(); + var phase = EndpointHelpers.GetCurrentPhase(player.CurrentPhase, state.ResultsOpen); var summary = new { CurrentPhase = phase, @@ -39,7 +38,8 @@ public static class StateEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var state = await db.AppState.AsNoTracking().FirstAsync(); + var phase = EndpointHelpers.GetCurrentPhase(player.CurrentPhase, state.ResultsOpen); return Results.Ok(new { player.Id, @@ -52,17 +52,20 @@ public static class StateEndpoints }); }); - group.MapPost("/me/phase/next", async (HttpContext ctx, AppDbContext db, IConfiguration _) => + group.MapPost("/me/phase/next", async (HttpContext ctx, AppDbContext db) => { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) return Results.Unauthorized(); - var next = NextPhase(player.CurrentPhase); var appState = await db.AppState.FirstAsync(); + var reconciled = EndpointHelpers.ReconcilePlayerPhase(player, appState.ResultsOpen); + var next = NextPhase(player.CurrentPhase); if (next == Phase.Results && !appState.ResultsOpen) { + if (reconciled) + await db.SaveChangesAsync(); return Results.BadRequest(new { error = "Results are locked until the admin enables them." }); } @@ -76,7 +79,7 @@ public static class StateEndpoints }); }); - group.MapPost("/me/phase/prev", async (HttpContext ctx, AppDbContext db, IConfiguration _) => + group.MapPost("/me/phase/prev", async (HttpContext ctx, AppDbContext db) => { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) @@ -88,10 +91,11 @@ public static class StateEndpoints return Results.BadRequest(new { error = "Only admins can move backward." }); } + var appState = await db.AppState.FirstAsync(); + EndpointHelpers.ReconcilePlayerPhase(player, appState.ResultsOpen); player.CurrentPhase = PrevPhase(player.CurrentPhase); player.VotesFinal = false; await db.SaveChangesAsync(); - var appState = await db.AppState.AsNoTracking().FirstAsync(); return Results.Ok(new { player.CurrentPhase, @@ -117,3 +121,4 @@ public static class StateEndpoints _ => Phase.Suggest }; } + diff --git a/Endpoints/SuggestEndpoints.cs b/Endpoints/SuggestEndpoints.cs index 0f99cfd..3f343e1 100644 --- a/Endpoints/SuggestEndpoints.cs +++ b/Endpoints/SuggestEndpoints.cs @@ -69,7 +69,7 @@ public static class SuggestEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); var usingJoker = phase == Phase.Vote && player.HasJoker; if (phase != Phase.Suggest && !usingJoker) return EndpointHelpers.PhaseMismatch(Phase.Suggest, phase); @@ -121,7 +121,7 @@ public static class SuggestEndpoints if (!isAdmin) { - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Suggest) return EndpointHelpers.PhaseMismatch(Phase.Suggest, phase); } @@ -181,7 +181,7 @@ public static class SuggestEndpoints if (suggestion.PlayerId != player!.Id) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase == Phase.Results) return EndpointHelpers.PhaseMismatch(Phase.Suggest, phase); @@ -244,7 +244,7 @@ public static class SuggestEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase < Phase.Vote) return EndpointHelpers.PhaseMismatch(Phase.Vote, phase); @@ -328,3 +328,4 @@ public static class SuggestEndpoints return true; } } + diff --git a/Endpoints/VoteEndpoints.cs b/Endpoints/VoteEndpoints.cs index e3bd772..38cae66 100644 --- a/Endpoints/VoteEndpoints.cs +++ b/Endpoints/VoteEndpoints.cs @@ -19,7 +19,7 @@ public static class VoteEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Vote) return EndpointHelpers.PhaseMismatch(Phase.Vote, phase); @@ -43,7 +43,7 @@ public static class VoteEndpoints if (player.VotesFinal) return Results.BadRequest(new { error = "Votes are finalized. Unfinalize before changing scores." }); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Vote) return EndpointHelpers.PhaseMismatch(Phase.Vote, phase); @@ -97,7 +97,7 @@ public static class VoteEndpoints if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != Phase.Vote) return EndpointHelpers.PhaseMismatch(Phase.Vote, phase); @@ -107,3 +107,4 @@ public static class VoteEndpoints }); } } + diff --git a/GameList.Tests/FiltersTests.cs b/GameList.Tests/FiltersTests.cs index 8251e2a..72b9b9d 100644 --- a/GameList.Tests/FiltersTests.cs +++ b/GameList.Tests/FiltersTests.cs @@ -4,6 +4,7 @@ using GameList.Domain; using GameList.Infrastructure; using GameList.Tests.Support; using Microsoft.AspNetCore.Http; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; namespace GameList.Tests; @@ -50,7 +51,30 @@ public class FiltersTests await AssertStatusAsync(result, StatusCodes.Status400BadRequest); } - private static async Task BuildContextAsync(TestWebApplicationFactory factory, bool isAdmin, Phase phase = Phase.Vote, bool hasJoker = false) + [Fact] + public async Task Phase_requirement_uses_effective_phase_without_mutating_player() + { + await using var factory = new TestWebApplicationFactory(); + var ctx = await BuildContextAsync(factory, isAdmin: false, phase: Phase.Vote, resultsOpen: true); + var filter = new PhaseRequirementFilter(Phase.Results); + var called = false; + + var result = await filter.InvokeAsync(ctx, _ => + { + called = true; + return ValueTask.FromResult(Results.Ok()); + }); + + Assert.True(called); + await AssertStatusAsync(result, StatusCodes.Status200OK); + + var playerId = Guid.Parse(ctx.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier)!); + var db = ctx.HttpContext.RequestServices.GetRequiredService(); + var storedPlayer = await db.Players.AsNoTracking().SingleAsync(p => p.Id == playerId); + Assert.Equal(Phase.Vote, storedPlayer.CurrentPhase); + } + + private static async Task BuildContextAsync(TestWebApplicationFactory factory, bool isAdmin, Phase phase = Phase.Vote, bool hasJoker = false, bool resultsOpen = false) { var scope = factory.Services.CreateScope(); var db = scope.ServiceProvider.GetRequiredService(); @@ -67,6 +91,8 @@ public class FiltersTests DisplayName = "User" }; db.Players.Add(player); + var state = await db.AppState.FirstAsync(); + state.ResultsOpen = resultsOpen; await db.SaveChangesAsync(); var ctx = new DefaultHttpContext diff --git a/GameList.Tests/StateTests.cs b/GameList.Tests/StateTests.cs index 3b637a8..01c1aaf 100644 --- a/GameList.Tests/StateTests.cs +++ b/GameList.Tests/StateTests.cs @@ -36,7 +36,7 @@ public class StateTests } [Fact] - public async Task GetPhase_upgrades_reveal_and_resets_when_results_close() + public async Task GetCurrentPhase_reads_effective_phase_without_mutating_player() { await using var factory = new TestWebApplicationFactory(); Guid playerId = Guid.Empty; @@ -63,7 +63,7 @@ public class StateTests using (var scope = factory.Services.CreateScope()) { var db = scope.ServiceProvider.GetRequiredService(); - var phase = await Endpoints.EndpointHelpers.GetPhase(db, playerId); + var phase = await Endpoints.EndpointHelpers.GetCurrentPhaseAsync(db, playerId); Assert.Equal(Phase.Results, phase); } @@ -77,10 +77,11 @@ public class StateTests using (var scope = factory.Services.CreateScope()) { var db = scope.ServiceProvider.GetRequiredService(); - var phase = await Endpoints.EndpointHelpers.GetPhase(db, playerId); + var phase = await Endpoints.EndpointHelpers.GetCurrentPhaseAsync(db, playerId); var player = await db.Players.FindAsync(playerId); Assert.Equal(Phase.Vote, phase); - Assert.False(player!.VotesFinal); + Assert.Equal(Phase.Reveal, player!.CurrentPhase); + Assert.True(player.VotesFinal); } } @@ -231,6 +232,37 @@ public class StateTests Assert.Equal("ok", resp.GetProperty("status").GetString()); } + [Fact] + public async Task State_and_me_reads_do_not_persist_phase_reconciliation() + { + await using var factory = new TestWebApplicationFactory(); + var client = factory.CreateClientWithCookies(); + await client.RegisterAsync("reader"); + + await factory.WithDbContextAsync(async db => + { + var player = await db.Players.FirstAsync(); + player.CurrentPhase = Phase.Results; + player.VotesFinal = true; + var state = await db.AppState.FirstAsync(); + state.ResultsOpen = false; + await db.SaveChangesAsync(); + }); + + var stateResponse = await client.GetFromJsonAsync("/api/state"); + var meResponse = await client.GetFromJsonAsync("/api/me"); + + Assert.Equal(nameof(Phase.Vote), stateResponse.GetProperty("currentPhase").GetString()); + Assert.Equal(nameof(Phase.Vote), meResponse.GetProperty("currentPhase").GetString()); + + await factory.WithDbContextAsync(async db => + { + var player = await db.Players.AsNoTracking().FirstAsync(); + Assert.Equal(Phase.Results, player.CurrentPhase); + Assert.True(player.VotesFinal); + }); + } + [Fact] public async Task GetPhase_aligns_to_results_when_open() { @@ -256,8 +288,9 @@ public class StateTests using var scope = factory.Services.CreateScope(); var db = scope.ServiceProvider.GetRequiredService(); var playerId = await db.Players.Select(p => p.Id).FirstAsync(); - var phase = await Endpoints.EndpointHelpers.GetPhase(db, playerId); + var phase = await Endpoints.EndpointHelpers.GetCurrentPhaseAsync(db, playerId); Assert.Equal(Phase.Results, phase); } } + diff --git a/GameList.csproj b/GameList.csproj index 8f9b63f..9ad5e5c 100644 --- a/GameList.csproj +++ b/GameList.csproj @@ -16,6 +16,7 @@ + diff --git a/Infrastructure/PhaseOrJokerFilter.cs b/Infrastructure/PhaseOrJokerFilter.cs index f5324cc..242a076 100644 --- a/Infrastructure/PhaseOrJokerFilter.cs +++ b/Infrastructure/PhaseOrJokerFilter.cs @@ -18,7 +18,7 @@ public class PhaseOrJokerFilter : IEndpointFilter if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); var allow = phase == Phase.Suggest || (phase == Phase.Vote && player.HasJoker); if (!allow) { @@ -28,3 +28,4 @@ public class PhaseOrJokerFilter : IEndpointFilter return await next(context); } } + diff --git a/Infrastructure/PhaseRequirementFilter.cs b/Infrastructure/PhaseRequirementFilter.cs index 8ecc1f5..2e805c0 100644 --- a/Infrastructure/PhaseRequirementFilter.cs +++ b/Infrastructure/PhaseRequirementFilter.cs @@ -14,7 +14,7 @@ public class PhaseRequirementFilter(Phase required, bool allowAdminOverride = fa if (player is null) return Results.Unauthorized(); - var phase = await EndpointHelpers.GetPhase(db, player.Id); + var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != required && !(allowAdminOverride && player.IsAdmin)) { return EndpointHelpers.PhaseMismatch(required, phase); @@ -23,3 +23,4 @@ public class PhaseRequirementFilter(Phase required, bool allowAdminOverride = fa return await next(context); } } +