fix(schemas): the keys these seeds write that no schema declares - #1786
Merged
Conversation
OpenRegister gives a declared property a magic-table column and an undeclared one nothing at all. MagicMapper::prepareObjectDataForTable() is a whitelist by omission: it walks the schema's declared properties and copies those out of the payload, so a key the schema does not declare is never read, and there is no JSON blob column to fall back on. The save answers 200 and the value is gone. MEASURED FIRST, on the live dossiq-proof rig. Its dossiq source declares case.properties and mode: array; the case schema in its database has neither, and carries the old child-record extends-form. Both sides read version 1.12.0. 7882afd added the properties array and f7c9f87 switched the declaration to array mode, and neither bumped the version the importer gates on, so on that instance the create form still writes answers the old way against a schema that no longer matches the file. The version goes to 1.13.0, which is the one thing that makes the update unconditional rather than dependent on the content-diff fallback. THE THREE VTH CHECKLIST TEMPLATES never installed on any instance. The seeds carried a flat items[] and the schema requires sections[], so every save threw "The required property (sections) is missing", the step logged a warning, carried on, and reported success with zero rows. They are reshaped into sections, `type` becomes `responseType`, and photoRequired becomes the enum it was declared as rather than a boolean. caseTypeSlug is what a shipped file can carry, so the step now resolves it to the caseType uuid instead of writing a key nothing stores. genericRole IS DECLARED, NOT DROPPED. It is editable from the Roles tab, specified in case-types and admin-settings, and shipped by three seed files. It was discarded on every one of those writes. mandateGuard goes the other way: nothing reads it, and it is removed, following the dossiq#1782 precedent for `catalog`. FOUND BY THE SWEEP, not by the report. Sweeping every shipped seed payload against its schema turned up three more families: extensionDuration where the schema says extensionPeriod, `type`/`enum` on VTH property definitions where the schema says propertyType/enumValues, and three VTH templates shipping statusTypes, documentTypes and roleTypes as bare strings, which seedSubObjects() skips outright, so those templates seeded nothing at all while reporting success. The sweep ships as SeedPayloadSchemaConformanceTest. It reads the same merged configuration the installer imports, checks nested objects as well as top-level keys, and allows only the two keys OpenRegister genuinely consumes: `id`, which addresses the object, and `slug`, which lands on @self. `uuid` is deliberately not allowed, because nothing reads it.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 3, 2026 19:40
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 540/540 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-03 19:46 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
pushed a commit
that referenced
this pull request
Sep 3, 2026
…assing All three were green in my terminal and red in CI, each for a different reason, and all three were readable locally the whole time. I read the printed summary instead of the exit code. 1. eslint. Identical output in both places, "823 problems (0 errors)", and exit 2. ESLint keeps eslint-suppressions.json, and a suppression for a file that still gets linted but no longer has the problem is a hard error. Removing the voorstelReminder handler took the console call its src/customComponents.js suppression covered. Pruned: 11 entries, of which ten were already dead on development for files deleted earlier. Only the customComponents one was load-bearing, because ESLint never lints a file that does not exist and so never calls its suppression unused. That is why development stayed green carrying the same ten. 2. prettier. Dropping 'proposal' from EXPECTED_SEARCHABLE_SLUGS made the array short enough to fit one line, which prettier --check wants and I did not run. 3. PHPUnit, all six matrix cells, with ZERO failures and ZERO errors: Tests 2922, Assertions 17500, Risky 1. phpunit.xml sets failOnRisky="true" and beStrictAboutCoverageMetadata="true", so one risky test reddens the cell. SeedPayloadSchemaConformanceTest executes RegisterFragmentMerger in setUp() without listing it, and strict coverage metadata only fires when a coverage driver is loaded. There is none on this box, so the local run cannot see it at all. Added the @uses annotation. That third one is not mine: development is red on the same risky test in the same six cells, from #1786 onward. Fixed here because this PR is red on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
OpenRegister gives a declared property a magic-table column and an undeclared one nothing at all.
MagicMapper::prepareObjectDataForTable()is a whitelist by omission: it walks the schema's declared properties and copies those out of the payload, so a key the schema does not declare is never read, and there is no JSON blob column to fall back on. The save answers 200 and the value is gone.Measured before changing anything
On the live
dossiq-proofrig, in one command per side:caseschema versionpropertiesarrayarray7882afd added
case.propertiesand f7c9f87 switched the declaration to array mode. Neither bumped the version the importer gates on, so that instance still runs the old declaration against a file that no longer matches it.casegoes to 1.13.0, which is the one thing that makes the update unconditional rather than dependent onschemaContentDiffers()as a fallback.The three VTH checklist templates never installed
The seeds carried a flat
items[]; the schema requiressections[]. Every save threw "The required property (sections) is missing", the step logged a warning, carried on, and reported success with zero rows. They are reshaped intosections,typebecomesresponseType, andphotoRequiredbecomes the enum it was declared as instead of a boolean.caseTypeSlugis the only stable identifier a shipped file can carry, so the step resolves it to thecaseTypeuuid rather than writing a key nothing stores.Declared vs stopped writing
roleType.genericRolecase-typesandadmin-settings, shipped by three seed files, discarded on every writecaseType.mandateGuardcatalogcaseType.catalogslugSaveObject::setSelfMetadata()reads it and stores it on@self, which is whereexistingSlugs()reads it backuuid$uuidparameter or fromidThree more families the sweep found
Not in the original report, found by pointing the sweep at every shipped seed:
extensionDurationwhere the schema saysextensionPeriod, in two seed files.type/enumon VTH property definitions where the schema sayspropertyType/enumValues, 17 sites.integeris not in thepropertyTypeenum either, so those becomenumber.statusTypes,documentTypesandroleTypesas bare strings.seedSubObjects()skips any record that is not an array, so those templates seeded nothing at all while reporting success.The sweep
SeedPayloadSchemaConformanceTestreads the same merged configuration the installer imports, through the realRegisterFragmentMergerrather than the monolith alone. It checks nested objects as well as top-level keys, and allows only the two keys OpenRegister genuinely consumes.Its first version passed a mutation that should have failed it: it only compared top-level keys, so a
typeplanted back insidesections[].items[]went unseen. That hole is closed, and the mutation now produces two findings and a red run.Verified locally
phpunit(138 Settings tests),phpcs,psalm,phpstan,phpmd,check:schema-l10nat baseline,npm run l10n:build, and hydra-gates v1.11.1: all 75 applicable gates passed, and all 75 ran.🤖 Generated with Claude Code