chore(app): add missing FluentValidation query validators (#243)#365
Conversation
Adds validators for 31 read-model queries flagged by the prod-readiness audit as lacking a validator. Each guards UserId (non-empty Guid) and, where present, the entity id, date range (from<=to, <=366 days), Period, Language, and public-profile Slug — mirroring the existing query-validator pattern (GetHabitSchedule/GetAllHabitLogs/GetDailySummary). Purely additive and behavior-preserving: valid requests are unchanged; malformed input is now rejected at the trust boundary via the existing ValidationBehavior pipeline instead of failing deeper in the handler. Representative validator tests cover every added file (UserId/entity-id/ slug guards) plus the range/period/language logic on GetRetrospective and GetGoalReview. Refs thomasluizon/orbit-ui-mobile#243 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Review Complete
Scope: PR #365 in thomasluizon/orbit-api — chore(app): add missing FluentValidation query validators (#243)
Recommendation: APPROVE
No prior reviews or resolved threads existed on this PR, so the full diff (31 new validator files + 3 new test files, 100% additive) was reviewed.
| Severity | Count |
|---|---|
| Critical (incl. breaks old mobile clients) | 0 |
| High | 0 |
| Medium | 0 |
| Low / Info | 1 (positive observation only) |
Findings
None Critical/High/Medium. One Info note: the 31 new validators are byte-for-byte consistent in style with the pre-existing GetHabitScheduleQueryValidator pattern, so no deviation to fix.
Verification performed
- Independent correctness cross-check (general-purpose agent read all 31 validator/query-record pairs directly): every
RuleForproperty name matches the actual record property, no typos;GetPlansQuery(Guid UserId, string? CountryCode, string? IpAddress)confirmed against its test's 3-arg constructor calls;AppConstants.MaxRangeDays = 366/MaxLanguageLength = 10/SupportedLanguages = ["en","pt-BR"]confirmed atsrc/Orbit.Application/Common/AppConstants.cs:15,40,84;RetrospectivePeriodRange.IsKnownPeriodconfirmed atsrc/Orbit.Application/Habits/Queries/RetrospectivePeriodRange.cs:19; DI auto-registration confirmed viaAddValidatorsFromAssemblyContaining<CreateHabitCommandValidator>()atsrc/Orbit.Api/Extensions/ServiceCollectionExtensions.cs:235; no optional/nullable Guid filter was wrongly forced to.NotEmpty(). - security-reviewer subagent: PASS —
GetPublicProfileQueryValidatorcorrectly validates onlySlug(record has noUserId, matches its[AllowAnonymous]controller); date-range bound onGetRetrospectiveQueryprevents unbounded queries; no secrets/PII, no raw SQL/reflection/unsafe deserialization in any new file. - contract-aligner: N/A — no DTO, Controller route, or
packages/sharedtype changed; gate correctly not met (confirmed directly, not assumed). - Comment policy:
grep -nE '^\+.*//'over the full diff returned zero matches — no narration comments (ORBIT0001-clean). - Test coverage: 31 validators, 40 new tests, 1:1 coverage including edge cases (366-day boundary, unknown period, unsupported/empty language).
- Own spot-checks reproduced the subagents' key claims directly (AppConstants values,
IsKnownPeriod, DI registration line) rather than trusting them blind.
Since no Critical/High finding survived, Phase 6's adversarial skeptic pass and /second-opinion step were not triggered (nothing to challenge).
Deferred (not verifiable in this CI job)
- Build/Tests (dotnet) — handled by separate required CI checks (Build / Unit Tests / SonarCloud) per this job's scope.
- Cross-repo parity / i18n / FEATURES.md — sibling
orbit-ui-mobilerepo isn't checked out in this job; this diff is backend validation-only and touches no DTO/route, so parity gate is correctly not triggered, but consumer-side verification itself is not verifiable here.



What
Adds FluentValidation validators for 31 read-model queries across the Application layer that the frozen
/prod-readinessaudit flagged as lacking one. Each new validator lives under its feature'sValidators/folder and follows the existing query-validator pattern (GetHabitScheduleQueryValidator,GetAllHabitLogsQueryValidator,GetDailySummaryQueryValidator):UserIdguard (non-emptyGuid) on every user-scoped query.GoalId/HabitId) on the by-id/detail/metrics/logs queries.from <= to,<= MaxRangeDays) + Period (known-period) + Language (supported-language) onGetRetrospectiveQuery; Language onGetGoalReviewQuery.GetPublicProfileQuery.Why it's behavior-preserving
Purely additive — no existing Query, handler, or Command validator was touched (disjoint slice). Validators auto-register via
AddValidatorsFromAssemblyand run through the existingValidationBehaviorpipeline. Valid requests (the only ones the web/mobile clients ever send —UserIdcomes from the JWT) are unchanged; malformed input is now rejected at the trust boundary with a clean 400 instead of failing deeper in the handler.No API contract / DTO / endpoint change, so no
packages/sharedor consumer edit applies (the parity nudge is a false positive for an internal-validation-only change).Tests
QueryValidatorGuardTests— reject-invalid / accept-valid for every added file (UserId, entity-id, Plans, Slug guards), keeping new-code coverage healthy across all 31 validators.GetRetrospectiveQueryValidatorTests— period,from>to,>366-day range, unsupported/empty language edge + failure cases.GetGoalReviewQueryValidatorTests— UserId, unsupported/empty language.dotnet buildclean (0 errors); fullOrbit.Application.Testssuite green (2728 passed, +40 new).Refs thomasluizon/orbit-ui-mobile#243