Remove legacy reveal phase paths and rename reveal data loader
This commit is contained in:
@@ -3,7 +3,6 @@ namespace GameList.Domain;
|
|||||||
public enum Phase
|
public enum Phase
|
||||||
{
|
{
|
||||||
Suggest = 0,
|
Suggest = 0,
|
||||||
Reveal = 1,
|
|
||||||
Vote = 2,
|
Vote = 2,
|
||||||
Results = 3
|
Results = 3
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -51,7 +51,7 @@ internal static class EndpointHelpers
|
|||||||
|
|
||||||
public static Phase GetCurrentPhase(Phase phase, bool resultsOpen)
|
public static Phase GetCurrentPhase(Phase phase, bool resultsOpen)
|
||||||
{
|
{
|
||||||
var normalized = phase == Phase.Reveal ? Phase.Vote : phase;
|
var normalized = NormalizePhase(phase);
|
||||||
|
|
||||||
if (resultsOpen)
|
if (resultsOpen)
|
||||||
return Phase.Results;
|
return Phase.Results;
|
||||||
@@ -63,9 +63,10 @@ internal static class EndpointHelpers
|
|||||||
{
|
{
|
||||||
var changed = false;
|
var changed = false;
|
||||||
|
|
||||||
if (player.CurrentPhase == Phase.Reveal)
|
var normalized = NormalizePhase(player.CurrentPhase);
|
||||||
|
if (player.CurrentPhase != normalized)
|
||||||
{
|
{
|
||||||
player.CurrentPhase = Phase.Vote;
|
player.CurrentPhase = normalized;
|
||||||
changed = true;
|
changed = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -84,6 +85,17 @@ internal static class EndpointHelpers
|
|||||||
return changed;
|
return changed;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static Phase NormalizePhase(Phase phase)
|
||||||
|
{
|
||||||
|
return phase switch
|
||||||
|
{
|
||||||
|
Phase.Suggest => Phase.Suggest,
|
||||||
|
Phase.Vote => Phase.Vote,
|
||||||
|
Phase.Results => Phase.Results,
|
||||||
|
_ => Phase.Vote // legacy/invalid phase fallback
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
public static IResult PhaseMismatch(Phase required, Phase current) =>
|
public static IResult PhaseMismatch(Phase required, Phase current) =>
|
||||||
BadRequestError($"This endpoint is available in the {required} phase. Your current phase is {current}.");
|
BadRequestError($"This endpoint is available in the {required} phase. Your current phase is {current}.");
|
||||||
|
|
||||||
|
|||||||
@@ -99,7 +99,6 @@ public static class StateEndpoints
|
|||||||
private static Phase NextPhase(Phase current) => current switch
|
private static Phase NextPhase(Phase current) => current switch
|
||||||
{
|
{
|
||||||
Phase.Suggest => Phase.Vote,
|
Phase.Suggest => Phase.Vote,
|
||||||
Phase.Reveal => Phase.Vote, // legacy safety
|
|
||||||
Phase.Vote => Phase.Results,
|
Phase.Vote => Phase.Results,
|
||||||
_ => Phase.Results
|
_ => Phase.Results
|
||||||
};
|
};
|
||||||
@@ -108,7 +107,6 @@ public static class StateEndpoints
|
|||||||
{
|
{
|
||||||
Phase.Results => Phase.Vote,
|
Phase.Results => Phase.Vote,
|
||||||
Phase.Vote => Phase.Suggest,
|
Phase.Vote => Phase.Suggest,
|
||||||
Phase.Reveal => Phase.Suggest, // legacy safety
|
|
||||||
_ => Phase.Suggest
|
_ => Phase.Suggest
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ public class StateTests
|
|||||||
PasswordHash = [1],
|
PasswordHash = [1],
|
||||||
PasswordSalt = [1],
|
PasswordSalt = [1],
|
||||||
DisplayName = "Legacy",
|
DisplayName = "Legacy",
|
||||||
CurrentPhase = Phase.Reveal,
|
CurrentPhase = (Phase)1,
|
||||||
VotesFinal = true
|
VotesFinal = true
|
||||||
};
|
};
|
||||||
playerId = player.Id;
|
playerId = player.Id;
|
||||||
@@ -80,7 +80,7 @@ public class StateTests
|
|||||||
var phase = await Endpoints.EndpointHelpers.GetCurrentPhaseAsync(db, playerId);
|
var phase = await Endpoints.EndpointHelpers.GetCurrentPhaseAsync(db, playerId);
|
||||||
var player = await db.Players.FindAsync(playerId);
|
var player = await db.Players.FindAsync(playerId);
|
||||||
Assert.Equal(Phase.Vote, phase);
|
Assert.Equal(Phase.Vote, phase);
|
||||||
Assert.Equal(Phase.Reveal, player!.CurrentPhase);
|
Assert.Equal((Phase)1, player!.CurrentPhase);
|
||||||
Assert.True(player.VotesFinal);
|
Assert.True(player.VotesFinal);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
23
REVIEW.md
23
REVIEW.md
@@ -11,15 +11,11 @@ Active maintainability risks (priority order):
|
|||||||
- Cross-feature coupling still exists via shared mutable state usage across UI/data modules (`wwwroot/js/ui.js:180`, `wwwroot/js/ui.js:401`, `wwwroot/js/ui.js:622`, `wwwroot/js/data.js:82`).
|
- Cross-feature coupling still exists via shared mutable state usage across UI/data modules (`wwwroot/js/ui.js:180`, `wwwroot/js/ui.js:401`, `wwwroot/js/ui.js:622`, `wwwroot/js/data.js:82`).
|
||||||
- Impact: high regression surface and expensive refactors even after removing global `window` bridges.
|
- Impact: high regression surface and expensive refactors even after removing global `window` bridges.
|
||||||
|
|
||||||
2. Legacy phase path remains in active code (Medium)
|
2. Unauthenticated 401 response shape is still framework-driven (Medium)
|
||||||
- `Reveal` is still present in `Domain/Phase.cs:6` and compatibility branches remain in `Endpoints/StateEndpoints.cs:102` and `Endpoints/StateEndpoints.cs:111`.
|
|
||||||
- Impact: extra cognitive load and more branches to reason about during phase-flow changes.
|
|
||||||
|
|
||||||
3. Unauthenticated 401 response shape is still framework-driven (Medium)
|
|
||||||
- Endpoint and filter unauthorized responses are standardized when app logic executes (`Infrastructure/AdminOnlyFilter.cs:15`, `Infrastructure/PhaseRequirementFilter.cs:15`, `Endpoints/SuggestEndpoints.cs:18`), but anonymous challenge responses remain middleware-controlled (`GameList.Tests/StateTests.cs:214`).
|
- Endpoint and filter unauthorized responses are standardized when app logic executes (`Infrastructure/AdminOnlyFilter.cs:15`, `Infrastructure/PhaseRequirementFilter.cs:15`, `Endpoints/SuggestEndpoints.cs:18`), but anonymous challenge responses remain middleware-controlled (`GameList.Tests/StateTests.cs:214`).
|
||||||
- Impact: clients must tolerate both app-produced problem payloads and framework challenge responses.
|
- Impact: clients must tolerate both app-produced problem payloads and framework challenge responses.
|
||||||
|
|
||||||
4. Static analysis and frontend lint guardrails are still missing (Medium)
|
3. Static analysis and frontend lint guardrails are still missing (Medium)
|
||||||
- CI currently gates restore/build/test only (`.github/workflows/ci.yml:23`-`.github/workflows/ci.yml:29`).
|
- CI currently gates restore/build/test only (`.github/workflows/ci.yml:23`-`.github/workflows/ci.yml:29`).
|
||||||
- Impact: style drift and low-signal warnings can enter the codebase undetected.
|
- Impact: style drift and low-signal warnings can enter the codebase undetected.
|
||||||
|
|
||||||
@@ -33,14 +29,6 @@ Active maintainability risks (priority order):
|
|||||||
- Effort / Risk: `L / Med`.
|
- Effort / Risk: `L / Med`.
|
||||||
- Dependencies (if any): none.
|
- Dependencies (if any): none.
|
||||||
|
|
||||||
[P1] Remove legacy `Reveal` phase compatibility branches
|
|
||||||
- Problem: Severity `Medium`, Category `Complexity`. Legacy phase compatibility logic is still present in runtime paths.
|
|
||||||
- Evidence: `Domain/Phase.cs:6`, `Endpoints/StateEndpoints.cs:102`, `Endpoints/StateEndpoints.cs:111`, `wwwroot/js/data.js:30`.
|
|
||||||
- Recommendation: remove `Reveal` from enum/transition logic and update affected tests/documentation.
|
|
||||||
- Acceptance criteria (testable): no runtime references to `Phase.Reveal`; phase transitions cover only `Suggest`, `Vote`, and `Results`.
|
|
||||||
- Effort / Risk: `S / Low`.
|
|
||||||
- Dependencies (if any): none.
|
|
||||||
|
|
||||||
[P1] Unify client handling of unauthenticated 401 challenge responses
|
[P1] Unify client handling of unauthenticated 401 challenge responses
|
||||||
- Problem: Severity `Medium`, Category `API/Contracts`. Fully unauthenticated requests can still bypass application-level problem shaping.
|
- Problem: Severity `Medium`, Category `API/Contracts`. Fully unauthenticated requests can still bypass application-level problem shaping.
|
||||||
- Evidence: challenge behavior is exercised in `GameList.Tests/StateTests.cs:214` while app-level unauthorized shaping is exercised in `GameList.Tests/AuthTests.cs:164`.
|
- Evidence: challenge behavior is exercised in `GameList.Tests/StateTests.cs:214` while app-level unauthorized shaping is exercised in `GameList.Tests/AuthTests.cs:164`.
|
||||||
@@ -68,10 +56,9 @@ Active maintainability risks (priority order):
|
|||||||
## C) Suggested execution order
|
## C) Suggested execution order
|
||||||
|
|
||||||
1. Decompose `ui.js` by feature and keep orchestration thin.
|
1. Decompose `ui.js` by feature and keep orchestration thin.
|
||||||
2. Remove `Reveal` phase compatibility branches.
|
2. Normalize/declare unauthenticated 401 contract behavior.
|
||||||
3. Normalize/declare unauthenticated 401 contract behavior.
|
3. Add analyzers + JS lint gates in CI.
|
||||||
4. Add analyzers + JS lint gates in CI.
|
4. Externalize i18n/FAQ assets.
|
||||||
5. Externalize i18n/FAQ assets.
|
|
||||||
|
|
||||||
## D) Guardrails
|
## D) Guardrails
|
||||||
|
|
||||||
|
|||||||
@@ -24,7 +24,6 @@ import {
|
|||||||
import {
|
import {
|
||||||
loadState,
|
loadState,
|
||||||
loadSuggestData,
|
loadSuggestData,
|
||||||
loadRevealData,
|
|
||||||
loadVoteData,
|
loadVoteData,
|
||||||
loadResults,
|
loadResults,
|
||||||
refreshPhaseData,
|
refreshPhaseData,
|
||||||
|
|||||||
@@ -27,7 +27,7 @@ export async function loadSuggestData() {
|
|||||||
renderMySuggestions();
|
renderMySuggestions();
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function loadRevealData() {
|
export async function loadSuggestionsData() {
|
||||||
if (state.phase === "Vote" || state.phase === "Results") {
|
if (state.phase === "Vote" || state.phase === "Results") {
|
||||||
const prev = state.allSuggestions ?? [];
|
const prev = state.allSuggestions ?? [];
|
||||||
const prevById = Object.fromEntries(prev.map((s) => [s.id, s]));
|
const prevById = Object.fromEntries(prev.map((s) => [s.id, s]));
|
||||||
@@ -84,7 +84,7 @@ export async function refreshPhaseData() {
|
|||||||
const prevPhase = state.phase;
|
const prevPhase = state.phase;
|
||||||
const prevResultsOpen = state.resultsOpen;
|
const prevResultsOpen = state.resultsOpen;
|
||||||
await loadState();
|
await loadState();
|
||||||
await Promise.all([loadSuggestData(), loadRevealData(), loadResults()]);
|
await Promise.all([loadSuggestData(), loadSuggestionsData(), loadResults()]);
|
||||||
if (state.phase === "Vote") {
|
if (state.phase === "Vote") {
|
||||||
if (!state.votesRendered) await loadVoteData();
|
if (!state.votesRendered) await loadVoteData();
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
Reference in New Issue
Block a user