fix(import): reconcile magic tables for every imported schema — closes #2082#2086
Merged
Merged
Conversation
A configuration import updated the schema definition but never reconciled the physical magic table, so a property added to an EXISTING schema in an EXISTING register never got a column. ensureTableForRegisterSchema() was only reached from two places: - autoCreateRegisterIfApplication() — new, auto-created registers only - importSeedData() — schemas that ship seed objects only An app shipping a register.d fragment that adds a property to an existing schema hit neither, so the column simply never appeared — not even after a FORCED re-import. The column only materialised if something later happened to write an object of that type, because the save path does call ensureTableForRegisterSchema(). That is worse than a silent no-op when the schema's own RBAC read rule matches on the new property: the generated SQL references a column that does not exist, so EVERY read — including the anonymous public path — returns a raw SQL error. Observed live on softwarecatalog 2026-07-24: 'beoordeeling' gained 'status' (public read rule matched on it → HTTP 500 on every anonymous read) and 'gebruik' gained the TIME fields; 11 columns across two schemas were missing, so three shipped features could not persist their data. Fix: after import, reconcile every imported schema against every register that holds it. Failures are logged, never fatal — an otherwise-successful import must not be lost because one table could not be reconciled. Live-verified: a single import created all 11 previously-missing columns (gebruik time_classification/time_rationale/time_review_date; module bbn_level/dpia_*/eol_product_slug/verwerkingsregister_ref) and the anonymous read that had been throwing returned 200.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ❌ | ✅ 174/174 | |||
| npm | ✅ | ✅ 555/555 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-07-24 18:20 UTC
Download the full PDF report from the workflow artifacts.
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
1 participant
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.
Closes #2082.
Root cause
ensureTableForRegisterSchema()was reachable from only two import paths:autoCreateRegisterIfApplication()— newly auto-created registers onlyimportSeedData()— schemas that ship seed objects onlyAn app shipping a
register.dfragment that adds a property to an existing schema in an existing register hits neither, so the physical column is never created — not even by a forced re-import. It only appears if something later happens to write an object of that type, because the save path does callensureTableForRegisterSchema().The sync logic itself was fine — it simply was never invoked from import.
Why it's worse than a silent no-op
When the schema's own RBAC read rule matches on the new property, the generated SQL references a non-existent column, so every read throws:
Fail-closed still holds (nothing leaks), but the public read path is a hard error.
Observed live (softwarecatalog, 2026-07-24)
beoordeelinggainedstatus(public read rule matched on it → anonymous 500) andgebruikgained the TIME fields. 11 columns across two schemas were missing, so three already-merged features could not persist their data at all.Fix
After import, reconcile every imported schema against every register that holds it (a schema can belong to several, each with its own table). Failures are logged and never fatal — an otherwise-successful import must not be lost because one table could not be reconciled.
Verification
gebruik.time_classification/time_rationale/time_review_date,module.bbn_level/dpia_status/dpia_date/dpia_volgende_beoordeling/dpia_document_ref/verwerkingsregister_ref/eol_product_slug— and the anonymous read that had been throwing returned 200. Baseline captured before/after; environment restored afterwards.Note:
tests/Unit/AppHost/BootstrapFactoryChainTest.phpbreaks full-suite discovery onphpunit-unit.xml— confirmed pre-existing (reproduced with my changes stashed), so the new test was run directly.Built in a worktree off
developmentto avoid disturbing other in-flight openregister work.🤖 Generated with Claude Code