Repository navigation
Conversation
…tion has a nested $ref SchemaValidation looked up the failing field's schema from error.schemaPath. When a referenced definition itself contains a $ref, AJV compiles it separately and reports a schemaPath relative to that definition, so the lookup against the root schema found nothing and the user got the standard text instead of the custom error message. Use error.parentSchema, which AJV sets with verbose: true and which is the schema that failed in every case.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the validation fix and the nested $ref case addressed by the pull request. |
| Description check | |
The description is complete. It explains the issue, implementation, scope, related issue, manual and automated verification, test coverage, and repository checklist status. |
| Linked Issues check | |
Issue #4389 requires the field's custom errorMessage for validation failures in definitions with nested $ref, including rootElementPath validation. SchemaValidation.tsx now reads `error.parent… |
| Out of Scope Changes check | |
The changes are limited to the validation lookup and focused unit tests. The tests directly cover issue #4389. The separate case of errorMessage beside a $ref remains unchanged, as stated in the P… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. |
Full details: Linked Issues check
-
Explanation
Issue
#4389requires the field's customerrorMessagefor validation failures in definitions with nested$ref, includingrootElementPathvalidation.SchemaValidation.tsxnow readserror.parentSchema.errorMessageinstead of resolvingerror.schemaPathagainst the root schema. The table test covers root properties, definitions without and with nested$ref, and a nested$refwith a sub-schema.
Full details: Out of Scope Changes check
-
Explanation
The changes are limited to the validation lookup and focused unit tests. The tests directly cover issue
#4389. The separate case oferrorMessagebeside a$refremains unchanged, as stated in the PR scope.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
could someone re-run the failed cypress job? pretty sure it's the TT02 enviroment being slow and not this change all the steps in "Deeply linked hidden with component lookups" passed. it failed in the global afterEach => waitUntilSaved timed out on the last save to TT02 same commit (747ba71) passed the full cypress run earlier today in run 39701849548 also needs labels for check-label. kind/bug I think, backport or backport-ignore is your call |
olemartinorg
added
squad/utforming
Description
A custom
errorMessagein the data model was ignored when the field sat in a definition that itself contains a$ref, and the user got «Feil format eller verdi» instead.SchemaValidationlooked up the field's schema fromerror.schemaPath, but AJV compiles such a definition separately and reports a path relative to it, so the lookup against the root schema found nothing. The issue has the steps and screenshots.This PR uses
error.parentSchemainstead. AJV sets it because the validator is created withverbose: true, and it is the schema that failed, whether the definition was inlined or not. That also covers therootElementPathcase, so thegetSchemaPartOldGeneratorbranch is no longer needed here.getSchemaPartandgetSchemaPartOldGeneratorinsrc/utils/schemaUtils.tsare now only used by their own tests. I left them to keep this PR to the fix, and can remove them here or in a separate PR if you prefer.An
errorMessageplaced beside a$ref({ "$ref": "#/$defs/X", "errorMessage": "…" }) is not read before or after this change, since the error comes from the referenced definition. That is a separate case, and I have not changed it.Same steps as in the issue, with app-frontend 4 from the CDN and with this branch, for
Innsender.kontaktInfo.epost:4)v9 has the same lookup in
src/App/frontendin altinn-studio. I understand you sync changes from this repo over there, so I have not made a separate PR, but I am happy to make one if you prefer.Related Issue(s)
errorMessagein the data model is ignored when the definition contains a$ref#4389Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSchemaValidation.test.tsxhas a new table test that checks that the field'serrorMessageis used as the text key when the field is in:$ref;$ref;$ref, validated against a sub schema (rootElementPath).The last two fail on
main, and the first two pass there too.yarn tscand the fullyarn testpass locally on Node 22 (169 test suites and 3653 tests passed, 1 suite and 61 tests skipped), and ESLint and Prettier pass on the changed files. The changed lines are covered.Manual test in localtest with a copy of one of our apps (Altinn.App 8.12.11), where
Virksomhet.epostandInnsender.kontaktInfo.epostboth have"errorMessage": "Epost.feilmelding", and onlyKontaktInfohas a nested$ref. The local builds were loaded through localtest'sfrontendVersioncookie:main(cd18b44) built locally: the custom message underVirksomhet.epost, «Feil format eller verdi» underInnsender.kontaktInfo.epost.Summary by CodeRabbit
contact.phoneNumberfield, including when validation uses a referenced subschema.