Repository navigation
Description
Follow-up to the PR #83 review question that fixed the partial-index filter literal with nameof. A repo sweep found these remaining magic-string clusters, worst first:
1. "Admin" role name — three duplicate constants (cross-layer)
The same literal is independently defined in three layers:
src/Cluckwork.Api/AuthPolicies.cs—AdminRole = "Admin"src/Cluckwork.Application/Features/Users/CreateUser/CreateUserValidator.cs—AdminRole = "Admin"src/Cluckwork.Infrastructure/Persistence/DatabaseSeeder.cs—AdminRole = "Admin"
Three sources of truth for the role that gates every admin endpoint. A future rename (or the full-RBAC slice adding roles) has to find all three. Fix: one shared constant (Domain or Application — lowest layer all three reference) that the others alias.
2. JWT claim names duplicated across layers
"role": emitted insrc/Cluckwork.Infrastructure/Identity/JwtTokenService.cs:38, validated asRoleClaimType = "role"insrc/Cluckwork.Api/Program.cs:141, decoded again inweb/src/auth/claims.ts."account_id": emitted inJwtTokenService.cs:36, read insrc/Cluckwork.Api/Middleware/TenantResolutionMiddleware.cs:13.
Token issuance and token consumption agree only by convention. Fix: a ClaimNames constants class shared by Infrastructure and Api; the SPA copy stays a literal (cross-language) but gets a const in claims.ts with a comment pointing at the backend counterpart.
3. SPA status fields typed as string (~34 literal comparisons)
web/src/api/cluckwork.ts declares every status field as plain string, so the SPA's status comparisons (\"Voided\", \"Draft\", \"Submitted\", \"Locked\", \"ManagerAdjusted\", \"Active\", \"Depleted\", \"Confirmed\", …) are unchecked — a typo compiles and silently renders wrong UI (a misspelled \"Voided\" check would quietly show admin buttons on a voided entry).
Fix: string-literal union types on the API interfaces, e.g.
export type DailyEntryStatus = "Draft" | "Submitted" | "Locked" | "ManagerAdjusted" | "Voided";
export type FlockStatus = "Active" | "Depleted" | "Archived";
export type SalesOrderStatus = "Draft" | "Confirmed" | "Voided";TypeScript then rejects any misspelled comparison. Backend enum names are the wire contract (HasConversion<string>() + Status.ToString()), so the unions mirror the C# enums 1:1.
4. Minor / accepted
web/src/api/client.tsrepeats"Idempotency-Key"three times in one file — hoist a local const while touching it (server-side already hasIdempotencyMiddleware.HeaderName).- Raw
FOR UPDATESQL in 5 repository lock queries hard-codes table/column names (SalesOrderRepository,EggLotRepository×3,InventoryItemRepository,InventoryLotRepository). Unavoidable raw SQL (EF can't expressFOR UPDATE); drift breaks loudly in the Testcontainers integration suite, so acceptance is deliberate — noting for completeness. - SPA
localStoragekeys already use named constants — fine.
No behavior changes anywhere; this is rename-safety and typo-proofing. Good candidate to batch with the full-RBAC slice (item 1 & 2 touch the same files it will).
Activity
Address 3-agent review round of PR #94
mforce commentedon Jul 20, 2026
From the PR #94 review round: audit action codes ("DailyEntry.Adjust", "Payment.Void", …) are string literals in 16 handlers and duplicated as a hardcoded filter list in
web/src/routes/AuditPage.tsx. A new capture point that misses the SPA list is a silent filter gap (rows still visible under "All actions", just not filterable). When tackling this issue, centralize the codes server-side (constants class) and expose/generate the list for the SPA.F23: Audit log for critical changes — same-transaction append-only ev…
fix(web): add the 3 missing audit actions + FarmLogo entityType to th…