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 in src/Cluckwork.Infrastructure/Identity/JwtTokenService.cs:38, validated as RoleClaimType = "role" in src/Cluckwork.Api/Program.cs:141, decoded again in web/src/auth/claims.ts."account_id": emitted in JwtTokenService.cs:36, read in src/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.ts repeats "Idempotency-Key" three times in one file — hoist a local const while touching it (server-side already has IdempotencyMiddleware.HeaderName).- Raw
FOR UPDATE SQL in 5 repository lock queries hard-codes table/column names (SalesOrderRepository, EggLotRepository ×3, InventoryItemRepository, InventoryLotRepository). Unavoidable raw SQL (EF can't express FOR UPDATE); drift breaks loudly in the Testcontainers integration suite, so acceptance is deliberate — noting for completeness.
- SPA
localStorage keys 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).
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
ClaimNamesconstants class shared by Infrastructure and Api; the SPA copy stays a literal (cross-language) but gets a const inclaims.tswith a comment pointing at the backend counterpart.3. SPA status fields typed as
string(~34 literal comparisons)web/src/api/cluckwork.tsdeclares everystatusfield as plainstring, 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.
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).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.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).