Repository navigation
fix: make validation messages explain how to fix each field - #698
Conversation
|
Copy check, Claim: the new shared templates are still defective person-facing copy, because pt-BR text carries the untranslated English property name ( Verdict: AGREE, high confidence. So the copy is not approved yet. The specific pairs ( |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes: Reviewed the complete validation-copy rewrite and its regression tests, including the client error-handling paths in paired UI PR #1573.
- Bilingual instructions: Replaces rule-style messages with actionable English and pt-BR instructions for verification codes, sub-habit fields, tag colors, linked-item limits, and shared FluentValidation templates.
- Preserved contracts: Keeps validation rules, limits, error codes, formatting arguments, named placeholders, and response fields unchanged, including the two counted goal/tag errors.
- Regression coverage: Adds exact verification-code copy assertions and exercises real login, API key confirmation, and account deletion validators through the HTTP exception handler in both locales.
Verification: 1,166 focused application tests and 38 middleware tests passed. The full solution build and full test suite were not independently rerun; staging verification and the separate copy-approval step described in the PR remain outside this review.
gpt-6.1-sol ｜ 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Copy approval for review batch 1 (
Verdict: the copy is approved. The client key |
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes: Reviewed the changes since 382820d1, including field-specific localization, expanded sub-habit validation, and the inclusive profile-name limit.
- Added field-specific instructions: Attached bilingual catalog selectors through internal validation state, preserving original error codes, named placeholders, limits, and HTTP response fields across the inventoried validator flows.
- Expanded sub-habit copy selection: Included invalid descriptions and emoji in the existing ownership-scoped habit lookup while retaining the fast path for valid text.
- Strengthened regression coverage: Added exact bilingual validator-to-HTTP assertions and accepted-boundary checks, including all three verification-code rules, suggestion and bulk flows, and checklist limits.
- Corrected the profile limit: Made the English instruction explicitly allow 50 characters, matching the validator and Portuguese copy.
Verification: the complete solution build passed with zero errors, and all 9,346 tests passed with LANG unset. Also checked the shared validation schemas and backend-message mapping in paired UI PR #1573; the internal copy selectors do not change the consumer contract. Post-deployment staging verification remains outside this review.
gpt-6.1-sol ｜ 𝕏
1 participant
Closes thomasluizon/orbit-tickets#1204
The shared validation templates exposed C# property names beside form fields, including English names inside Portuguese messages. Each inventoried field now uses localized instructional copy while retaining its original FluentValidation error code, limits and HTTP response shape. Tag color errors ask the person to choose from the swatches; login and both step-up flows return the existing six-digit instruction for all three code rules.
Common/ErrorCopy.ValidationFields.csholds the field catalog, andCommon/ValidationCopyExtensions.csattaches its key withWithMessageplus internal validation state. Localization selects that key rather than overwriting the rule with its generic code template. The field key stays internal;errorDetails.codeis still the original validation code.Common/ErrorCopy.Validation.cskeeps developer-only generic templates with unquoted names and contains the dedicated validation-code instructions.The rules attach the copy in Auth, ApiKeys, Habits, Tags, Goals, Support, Referrals, Profile and ChecklistTemplates. Shared habit rules also cover onboarding, bulk creation and suggestions. Sub-habit editing extends the existing lookup for invalid titles to invalid descriptions and emoji, so those fields receive child-specific copy. Valid text avoids that lookup. Domain guards and request DTOs are unchanged.
Inventory
The table shows emitted codes, their fields and the final messages. Maximum-length rows show the actual rule limit and preserve the entered-count placeholder. The two
{0}limits are supplied by the existing call sites: 20 linked habits and 5 tags.Source evidence: UI PR 1573 and the API validators, plus read-only inspection of the consumer's
packages/shared/src/i18n/en.json,pt-BR.json, login hooks, API key management, name editing and checklist template callers. There are no consumer edits. The inventory includes the original forms plus referral code, profile name, API key name, checklist text and chat message. Suggestion and bulk endpoints validate the same person-entered habit and tag text.Client keys for follow-up
Only keys that still differ appear here. The existing instructions for code format, email format, goal progress and support subject/message match the API word for word in each applicable locale. pt-BR habit, sub-habit, tag and goal title instructions and goal unit also match; their English client keys still state rules.
tooLongExternal interface evidence
The FluentValidation version installed here is 12.1.1. Its own
lib/net8.0/FluentValidation.xmldefinesDefaultValidatorOptions.WithMessage,WithState, andValidationFailure.CustomState/FormattedMessagePlaceholderValues.WithMessageapplies only to the preceding rule; the field attachment deliberately does not callWithErrorCode.An actual validator result printed by
ValidationLocalizationTestsduring this round had the following complete shape, showing the FluentValidation fields consumed by localization. This is real producer output, not a hand-authored fixture:[{"PropertyName":"Items[0]","ErrorMessage":"Write the checklist item","AttemptedValue":"","CustomState":{"Values":[],"CopyKey":null},"Severity":0,"ErrorCode":"VALIDATION_TEMPLATE_ITEM_REQUIRED","FormattedMessagePlaceholderValues":{"CollectionIndex":0,"PropertyName":"Items","PropertyValue":"","PropertyPath":"Items[0]"}}]Re-derive validator results with
env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~EveryValidatorFamily_ReturnsPortugueseCopyAndAlignedCodes --logger "console;verbosity=detailed"; the existing test prints the real errors. The new field tests drive real validators throughValidationExceptionHandlerand check the produced HTTP messages and codes in both request languages. Their inputs use the repository's own commands rather than an invented external response.Test evidence
Before modifying tests or implementation:
env -u LANG dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~ErrorCopyTests|FullyQualifiedName~ValidatorTests': exit 0, 1,725 passed with the defect present.env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ValidationLocalizationTests: exit 0, 35 passed with the defect present.Stronger regression before the implementation fix:
env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~PersonFacingValidators_ReturnLocalizedInstructionsWithUnchangedCodes: exit 1, 164 failed and 14 passed out of 178. Failures showed the real old rule strings, includingThe length of 'Message' must be 5000 characters or fewer. You entered 5001 characters., instead of the expected localized field instructions.After the fix, the same regressions pass as part of the focused localization suite: exit 0, 245 passed. This includes 202 field cases, 8 checklist length cases and the existing 35 localization cases. The application catalog and validator suite passes: exit 0, 1,812 tests. Code-format assertions remain covered. Accepted values and exact allowed lengths are checked, not just rejection paths. The change was compiled and committed before broader verification, with hooks intact.
The table below is the run on commit
6eeaa91a.env -u LANG dotnet build Orbit.slnxenv -u LANG dotnet testenv LC_ALL=en_US.UTF-8 dotnet build Orbit.slnxenv LC_ALL=en_US.UTF-8 dotnet testBuilds retain pre-existing package-version and obsolete-API warnings. Both commit-hook runs passed formatting, root allowlist, dash, timeless and suppression gates. The working tree is clean. No bare narration comments were found in the changed C# files. No generated architecture files were staged.
Review fix
fd4dc203(the English profile name limit, see Copy approval) was rerun by the orchestrator:env -u LANG dotnet build Orbit.slnxexit 0,env -u LANG LC_ALL=en_US.UTF-8 dotnet build Orbit.slnxexit 0,env -u LANG LC_ALL=en_US.UTF-8 dotnet test Orbit.slnx --no-buildexit 0. TheLANGunset full run failed one test outside this change,EventsControllerTests.Stream_SendsPublishedChangesOnce, on its 2 second hang guard while the machine ran another worker;env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --no-build --filter FullyQualifiedName~EventsControllerTeststhen passed 12 of 12 in five consecutive runs. That flaky guard is filed formainseparately.Staging evidence
Both live five-digit requests returned HTTP 400 with the old messages. The branch has not been deployed, so these observations confirm the defect and response shape; they do not prove the new copy is live. Reproduce with the commands below, then compare the messages after deploying the reviewed commit.
Complete observed response shapes, with request ids redacted:
en:
{ "type": "ValidationFailure", "status": 400, "requestId": "<redacted>", "errors": { "Code": [ "'Code' must be 6 characters in length. You entered 5 characters.", "Code must be a 6-digit number" ] }, "errorDetails": { "Code": [ { "code": "ExactLengthValidator", "message": "'Code' must be 6 characters in length. You entered 5 characters." }, { "code": "VALIDATION_VERIFICATION_CODE_FORMAT", "message": "Code must be a 6-digit number" } ] } }pt-BR:
{ "type": "ValidationFailure", "status": 400, "requestId": "<redacted>", "errors": { "Code": [ "'Code' deve ter exatamente 6 caracteres. Você digitou 5 caracteres.", "O código deve ter 6 dígitos" ] }, "errorDetails": { "Code": [ { "code": "ExactLengthValidator", "message": "'Code' deve ter exatamente 6 caracteres. Você digitou 5 caracteres." }, { "code": "VALIDATION_VERIFICATION_CODE_FORMAT", "message": "O código deve ter 6 dígitos" } ] } }Copy approval
/second-opinion(Claude Opus 5.5) judged the claimed copy defect against every new en and pt-BR message in this change. The first pass on6eeaa91areturned AGREE (medium): the English profile name message "Keep it under 50 characters." said below 50 while the rule and the pt-BR text allow 50. Review fixfd4dc203changed it to "Use 50 characters or fewer.". The second pass on the complete fixed set returned DISAGREE (medium), so the copy is approved: each en and pt-BR pair matches in meaning, keeps the agreed terms (meta, tag, modelo de checklist, chave de API, código de indicação), uses correct placeholders and states each limit correctly. The verdict is also posted in this conversation.Assumptions
errorDetails.code, which the ticket forbids.Manual steps
redesign/main, the orchestrator runs Release API withenvironment=stagingandbranch=redesign/main, then sends a five-digit code toPOST /api/auth/verify-codein both languages and confirms HTTP 400 with "Enter all 6 digits" and "Digite os 6 dígitos" while the codes stayExactLengthValidatorandVALIDATION_VERIFICATION_CODE_FORMAT.environment=productionfrommain. No new configuration, secret, migration or backfill is required.thomasluizon/orbit-tickets#1205aligns the client keys listed above, including the profile nametooLongkey.