Extract suggestion and vote workflows into services

This commit is contained in:
2026-02-07 01:01:10 +01:00
parent d41b65d23d
commit 5d40d555d1
6 changed files with 361 additions and 299 deletions

View File

@@ -9,7 +9,7 @@ Progress update (as of February 6, 2026):
- Completed: admin auth docs aligned to account-based admin sessions (`API.md:3`).
- Completed: build/test guardrails added (`.github/workflows/ci.yml`) and root ownership/setup docs added (`README.md:1`).
- Completed: backend validators centralized for suggestions and auth (`Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthValidator.cs:11`).
- Completed: request safety hardened for redirects and forwarded headers (`Program.cs:40`, `Program.cs:104`, `Endpoints/EndpointHelpers.cs:105`, `GameList.Tests/HelperTests.cs:121`, `GameList.Tests/HelperTests.cs:219`).
- Completed: request safety hardened for redirects and forwarded headers (`Program.cs:42`, `Program.cs:106`, `Endpoints/EndpointHelpers.cs:105`, `GameList.Tests/HelperTests.cs:121`, `GameList.Tests/HelperTests.cs:219`).
Top 5 maintainability risks (priority order):
@@ -19,7 +19,7 @@ Top 5 maintainability risks (priority order):
- Impact: hard-to-debug regressions and fragile refactors in UI workflows.
2. Rule duplication still present between backend/frontend validations (High)
- Suggestion validation is centralized on the backend (`Endpoints/SuggestEndpoints.cs:45`, `Endpoints/SuggestEndpoints.cs:133`, `Endpoints/SuggestionValidator.cs:7`) but frontend still duplicates parts (`wwwroot/js/ui.js:648`, `wwwroot/js/ui.js:1019`).
- Suggestion validation is centralized on the backend (`Endpoints/SuggestionWorkflowService.cs:39`, `Endpoints/SuggestionWorkflowService.cs:109`, `Endpoints/SuggestionValidator.cs:7`) but frontend still duplicates parts (`wwwroot/js/ui.js:648`, `wwwroot/js/ui.js:1019`).
- Auth validation is centralized on the backend (`Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`, `Endpoints/AuthValidator.cs:11`) while frontend length checks remain duplicated (`wwwroot/app.js:92`, `wwwroot/app.js:121`).
- Impact: inconsistent behavior and repeated fixes across server/client.
@@ -29,8 +29,8 @@ Top 5 maintainability risks (priority order):
- Hidden module coupling through globals: `wwwroot/js/data.js:131`-`wwwroot/js/data.js:134`, plus `window` callbacks consumed in `wwwroot/js/ui.js:473`, `wwwroot/js/ui.js:696`, `wwwroot/js/ui.js:1009`.
- Impact: every UI change risks regressions outside its feature area.
4. Service-layer extraction is still pending in large endpoint files (High)
- Endpoint lambdas still own orchestration and persistence logic in `Endpoints/SuggestEndpoints.cs`, `Endpoints/AdminEndpoints.cs`, `Endpoints/VoteEndpoints.cs`, and `Endpoints/ResultsEndpoints.cs`.
4. Service-layer extraction is partially complete; admin/results workflows still inline (High)
- Suggestion and vote workflows have moved to services (`Endpoints/SuggestionWorkflowService.cs:8`, `Endpoints/VoteWorkflowService.cs:8`), but admin/results orchestration remains endpoint-heavy (`Endpoints/AdminEndpoints.cs:105`, `Endpoints/ResultsEndpoints.cs:30`).
- Impact: high cognitive load and slower, riskier feature changes.
5. Static-analysis and frontend lint guardrails remain incomplete (Medium)
@@ -101,7 +101,7 @@ Worst coupling points:
[P0][Partial] Centralize validation rules to stop backend/frontend drift
- Problem: Severity `High`, Category `Complexity/Duplication`. Validation rules are duplicated in multiple backend endpoints and frontend forms. Impact: inconsistent behavior and repeated fixes.
- Evidence: backend centralized in `Endpoints/SuggestEndpoints.cs:45`, `Endpoints/SuggestEndpoints.cs:133`, `Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`, `Endpoints/AuthValidator.cs:11`; frontend duplicates remain in `wwwroot/js/ui.js:648`, `wwwroot/js/ui.js:1019`, `wwwroot/app.js:92`.
- Evidence: backend centralized in `Endpoints/SuggestionWorkflowService.cs:39`, `Endpoints/SuggestionWorkflowService.cs:109`, `Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`, `Endpoints/AuthValidator.cs:11`; frontend duplicates remain in `wwwroot/js/ui.js:648`, `wwwroot/js/ui.js:1019`, `wwwroot/app.js:92`.
- Recommendation: Extract backend validators (e.g., `SuggestionValidator`, `AuthValidator`) and reuse in create/update paths; simplify frontend to UX-only prechecks and rely on server responses for source-of-truth.
- Acceptance criteria (testable): create/update share one backend validator path; tests cover validator once and both endpoints; frontend no longer re-implements server-only security rules.
- Effort / Risk: `M / Med`.
@@ -109,15 +109,15 @@ Worst coupling points:
[P0][Done] Harden request safety defaults (forwarded headers and redirect handling)
- Problem: Severity `High`, Category `Security`. Forwarded headers are trusted without explicit proxy/network allowlist, and image validation likely follows redirects despite "no redirects" policy.
- Evidence: `Program.cs:40`, `Program.cs:70`, `Program.cs:104`, `Endpoints/EndpointHelpers.cs:105`, `GameList.Tests/HelperTests.cs:121`, `GameList.Tests/HelperTests.cs:219`, `IIS.md:17`.
- Evidence: `Program.cs:42`, `Program.cs:72`, `Program.cs:106`, `Endpoints/EndpointHelpers.cs:105`, `GameList.Tests/HelperTests.cs:121`, `GameList.Tests/HelperTests.cs:219`, `IIS.md:17`.
- Recommendation: Configure known proxies/networks for forwarded headers; enforce `AllowAutoRedirect = false` in image validation client and add tests for redirect-chain and private-host edge cases.
- Acceptance criteria (testable): integration tests prove redirected URLs are rejected; forwarded header spoofing test fails when source is untrusted; documentation updated with trusted proxy requirements.
- Effort / Risk: `M / Med`.
- Dependencies (if any): none.
[P1] Extract service-layer workflows from endpoint lambdas
[P1][Partial] Extract service-layer workflows from endpoint lambdas
- Problem: Severity `High`, Category `Architecture`. Endpoint files contain business orchestration, persistence, and policy logic inline; large lambdas are hard to reason about and reuse.
- Evidence: `Endpoints/SuggestEndpoints.cs:43`, `Endpoints/AdminEndpoints.cs:105`, `Endpoints/VoteEndpoints.cs:35`, `Endpoints/ResultsEndpoints.cs:30`.
- Evidence: extraction completed for suggestions and votes (`Endpoints/SuggestionWorkflowService.cs:8`, `Endpoints/VoteWorkflowService.cs:8`, `Program.cs:37`, `Program.cs:38`); remaining inline orchestration is concentrated in `Endpoints/AdminEndpoints.cs:105`, `Endpoints/ResultsEndpoints.cs:30`.
- Recommendation: Introduce focused application services (`SuggestionService`, `VoteService`, `AdminWorkflowService`) and keep endpoints as transport adapters.
- Acceptance criteria (testable): endpoint handlers reduced to routing + DTO mapping + service calls; domain rule tests target service methods directly; endpoint tests remain green.
- Effort / Risk: `L / Med`.
@@ -149,7 +149,7 @@ Worst coupling points:
[P1] Make write workflows transaction-consistent and explicit
- Problem: Severity `Medium`, Category `Correctness/Architecture`. Several multi-step state changes rely on multiple DB commands without explicit transaction grouping.
- Evidence: `Endpoints/SuggestEndpoints.cs:103`-`Endpoints/SuggestEndpoints.cs:109`, `Endpoints/AdminEndpoints.cs:16`-`Endpoints/AdminEndpoints.cs:31`, `Endpoints/AdminEndpoints.cs:220`-`Endpoints/AdminEndpoints.cs:229`.
- Evidence: `Endpoints/SuggestionWorkflowService.cs:71`, `Endpoints/SuggestionWorkflowService.cs:75`, `Endpoints/AdminEndpoints.cs:16`-`Endpoints/AdminEndpoints.cs:31`, `Endpoints/AdminEndpoints.cs:220`-`Endpoints/AdminEndpoints.cs:229`.
- Recommendation: Wrap multi-entity updates in explicit transactions where consistency matters, or refactor into idempotent command handlers with compensating behavior.
- Acceptance criteria (testable): fault-injection tests prove no partial state after exceptions; transaction boundaries documented per workflow.
- Effort / Risk: `M / Med`.