feat(setup): ask which example set to load, instead of planting one on install - #1024
Merged
Conversation
…n install
Installing this app planted 334 objects nobody asked for.
SettingsService::loadConfiguration() merges decidesk_register.json with all 26
register.d fragments, and every one of them carried its own
x-openregister.seedData.objects. The InitializeSettings repair step runs that
merge, so a fresh install seeded a Gemeenteraad Amsterdam, a VvE Zeewaarts, a
pub quiz, five ACME B.V. bodies and eight placeholder TOOI mappings into the
operator's register. A municipality got the VvE data. A VvE got the council
data. Nobody chose any of it.
The wizard meanwhile offered a DIFFERENT dataset, whose own step text said
"Skip this on a production install". By the time an operator read that
sentence, 334 objects were already in their register.
So the fragments now declare schemas only, and the objects live in
lib/Settings/profiles/, one file per example set. A bare install plants
nothing; the wizard asks which organisation this is for, and loads that.
## The split had to be a closure, not a partition
The seeds cross-reference each other by slug. The reference graph has one
connected component of 170 objects tangling municipal, corporate and
association bodies together, so no partition exists. Each set is instead
anchored on its governance bodies and closed over outbound references, so every
reference resolves inside the set carrying it. Sets may overlap, and 15 objects
do. Verified: 334 of 334 classified, zero dangling references, zero orphans.
## Why the descriptor declares no register
An example set carries @self.configuration/register/schema on every object and
declares NO components.registers. That is load-bearing.
ImportHandler::importRegister() calls setApplication($appId) unconditionally
when it updates an existing register, so a descriptor that declared decidiq
would re-point the register at the profile's config id and hydrate over its
authorization block: the baseline that stops any authenticated user rewriting
another body's decisions.
Verified on a live instance: importing this shape left application=decidiq, the
version and the authorization hash byte-identical, imported 45 objects, and
added nothing on a second run.
The files sit in a SUBDIRECTORY because RegisterDescriptorService scans
lib/Settings/*.json non-recursively and indexes by declared register slug; four
profiles in lib/Settings would collide with each other and with the app's own
register.
## Two wizard steps, because an action carries no body
CnSetupWizard::runAction() posts to /api/setup/action/{action} with no body, so
an action cannot carry the answer. A choice step records which set via the new
POST /api/setup/config, and the run-action step reads it back. ADR-111 keeps
the schema-generated mock on offer, so it is one option in the same choice
rather than a second question about the same thing.
## Ten pre-existing defects this surfaced, all fixed
Validating every seed against its schema found:
- Three seeds keyed on `regulation`, which is not a decidiq schema slug: the
app's is `regeling`. Because importSeedData() resolves a slug cross-app with
multitenancy off, `regulation` resolves to LEARNIQ's schema.
- Regeling.status was required and named by x-openregister-lifecycle, but never
declared as a property, so OpenRegister created no magic-table column.
Measured: oc_openregister_table_21_262 carried every other property and no
status column, which means the declared in-preparation -> adopted ->
in-effect -> lapsed map could never advance a regulation.
- Six invalid enum values on the pub-quiz decision-stage seeds.
- Two governance bodies missing the required `domain`.
- The works-council seeds described a city council: all three WOR consultation
requests carried governanceBody: gemeenteraad-amsterdam, a null-UUID
director, and a raadsvergadering as their overlegvergadering. Replaced with a
real 45-object set built around an ondernemingsraad at ACME B.V.
## Verification
1235 PHP unit tests and 378 vitest tests pass. PHPCS 0 errors, PHPMD, PHPStan
and Psalm clean. Manifest validates against schema 2.26.0. The manifest-drift
guard was proved to FAIL when the manifest and the shipped sets disagree.
Two hydra gates (22, 53) still fail, and did before this change: hydra-gates
v1.10.0 vendors manifest schema 2.25.0, which predates the `flow` page type
that FlowDetail already used. That is fleet debt in ConductionNL/.github.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 30, 2026 19:54
…xample set Loading the municipality set on a freshly reset instance reported "Imported 199 example object(s)" and 198 landed. The count comes from the FILE, not the importer's reply — deliberately, so the number names what was ASKED FOR — which means the message could not tell the operator that anything was wrong. The importer's log could: [ImportHandler] Skipping seed object for 'termijnagenda-item' - import failed: SQLSTATE[22001]: String data, right truncated: value too long for type character varying(36) OpenRegister types a relation column as varchar(36), the width of a UUID. A seed that points at another object BY SLUG therefore cannot be stored when that slug is 37 characters or longer, and the row is skipped. `lta-herziening-parkeerbeleid` referenced `goal-amsterdam-parkeerbeleid-kwartaal`, which is 37. Shortened to `goal-amsterdam-parkeerbeleid` (28), in the goal and in the reference. MEASURED, NOT GUESSED, AND THE RULE IS NARROWER THAN IT LOOKS. Twenty-one shipped slugs are longer than 36 characters and twenty of them are harmless: `_slug` is a wider column, so a long slug only breaks when something POINTS at it. Exactly one was referenced, and it is the one that failed. So the guard added here asserts the real rule — no seed may be referenced by a slug over 36 characters — rather than banning long slugs. Verified to FAIL on the original data, naming the offender and its length, and to pass after the fix. After it: all four sets import completely on a fresh instance. municipality 199/199 association 56/56 corporate 91/91 works-council 45/45
The Dutch strings for the new setup steps went into l10n/nl.json, but the frontend reads the GENERATED l10n/nl.js, which is a separate committed artefact. So the wizard rendered its own copy in English while every control around it (Annuleren, Volgende, Sluiten) was Dutch: the half-translated screen that reads as a bug rather than a missing string. Regenerated with npm run l10n:build. CI's check:l10n-js compares the two and would have failed the PR, which is the check doing its job.
… stop it lying about missing schemas
Two defects the local e2e run surfaced, both of which would have made CI
unreadable.
## 1. The seed settled a step that no longer exists
`ci-seed.sh` posts `setup/action/skip-demo-data` before the suite runs, because
CnAppRoot opens the wizard as a full modal mask in every fresh browser context
while an optional step is outstanding. seed-profiles renamed that action to
`skip-example-set`.
The script is deliberately TOLERANT of a non-200 there ("an app whose wizard has
no such step answers 400, and that is not a seeding failure"), so the rename
would NOT have failed the seed step. It would have failed every later spec
instead, on `<ol class="cn-wizard-dialog__progress">` intercepting the click —
the message that accuses the selectors. Verified fixed: the seed now reports
`POST setup/action/skip-example-set -> HTTP 200`.
A unit test now asserts every action id the script posts is one SetupController
actually handles, so the next rename is caught there rather than in 194
timeouts. It fails on the old name, naming it.
## 2. "Schemas missing after import" was a paging artefact
The verification asked for `?_limit=1000` and reported every required slug it
could not find as a missing schema:
::error::Decidiq schemas missing after import:
['meeting', 'action-item', 'minutes', 'vote', 'transcript', ...]
All ten were in the database. This instance has 35 apps installed and 1111
schemas, so ten decidiq rows fell off the end of the page. The import had
worked; the QUESTION was too small — and the check sent you to look at the
import, which is the worst shape a wrong answer can take.
The limit is now well past any plausible instance AND truncation is detected
rather than assumed away: a response that comes back exactly full says so, and
names the page as the suspect instead of blaming the import.
`skip-example-set` returned 200, reported "No example data was loaded", and left
the setup wizard open over every page.
Splitting the old single `demo-data` step into a `choice` plus a `run-action`
gave the wizard TWO outstanding steps. The skip action wrote only
`demo_data_decided`, which closes the second. CnAppRoot opens the wizard while
ANY optional step is outstanding, so the first one kept it open.
Measured 2026-08-30 against a live instance: after ci-seed.sh posted the action,
the status was still
{"example-set": {"done": false}, "load-example-set": {"done": true}}
and the e2e suite failed on `<ol class="cn-wizard-dialog__progress">`
intercepting clicks Playwright had already resolved — the "visible, enabled and
stable, then timeout" signature that means COVERED, not slow. Six specs were
red by test 13 for that reason alone.
Skipping IS choosing none, so the action now records that choice too. Verified
live: both steps report done from the one call.
The test asserts BOTH writes rather than just the one it used to, because
asserting only `demo_data_decided` is exactly what let this through.
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ❌ | ||||
| check-l10n-js | ❌ | ||||
| check-schema-l10n | ❌ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 557/557 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-30 23:30 UTC
Download the full PDF report from the workflow artifacts.
Resolved eight conflicts. Both sides had moved on every one, so nothing
was taken wholesale in either direction.
Taken from development:
l10n/nl.json, l10n/nl.js - a strict superset, verified by key set: it
already carries all four of this branch's wizard keys plus thirteen
more from the governance and approval seams.
tests/e2e/ci-seed.sh - #1043 supersedes this branch's fix. Both
saw that the wizard mask was failing the suite; development's goes
further by posting example_profile=none to setup/config, because the
skip action alone leaves the example-set step open, and by asserting
the resulting state instead of the status code.
example-set-setup-step.spec.ts - formatting only, development's is the
linted form.
Kept from this branch:
SetupController.php - skip-example-set must also write
PROFILE_KEY=NONE_PROFILE so BOTH steps close. development still has
the unfixed version. This complements the ci-seed change rather than
competing with it: the seed takes the robust path, and the action is
correct for anyone who calls it.
municipality.json - development carries
goal-amsterdam-parkeerbeleid-kwartaal, which is 37 characters. An
OpenRegister relation column is varchar(36), so the referencing seed
fails to insert and the object is dropped while the importer still
reports the count from the file. Taking development's side here would
have reintroduced the bug this branch fixed.
SetupControllerTest.php, SeedProfileServiceTest.php - both sides add
the same tests in different places. Taking ours for both hunks yields
all five unique tests; mixing the sides would have declared
testAnUnknownSetIsRejectedRatherThanStored and
testLoadingWithoutAChoiceRefusesRatherThanGuessing twice and fatalled.
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 537/537 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 06:15 UTC
Download the full PDF report from the workflow artifacts.
…spelling
The merge took development ci-seed.sh, which settles the setup step with
POST api/setup/config and example_profile=none, and kept this branch test,
which asserted the script posts api/setup/action/<id>. Both were right on
their own and contradicted each other once merged: 1 failure in 1283
tests, on all six PHPUnit cells.
The test now accepts either route and checks the agreement that actually
matters, that whatever the script names exists in the controller. For the
config route that means the profile key.
Two traps while writing it, both caught by running the assertions against
the real files rather than trusting the suite to be right:
- sweeping every "key": in the script picks up error, success, message
and provider from unrelated responses it parses, and asserts the
controller reads those. It does not, so the test failed for a reason
that was not a defect.
- matching the first --data payload finds an empty {} from an earlier
curl, so the key loop iterates nothing and the test passes no matter
what. The payload is now bound to its own url.
Verified it still fails when it should: renaming example_profile in the
controller turns it red.
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 537/537 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 08:47 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 31, 2026
…top ci-seed lying about missing schemas (#1073) * fix(ci): repair the schema-l10n ratchet development is failing, and stop ci-seed lying about missing schemas Two fixes that were written alongside seed-profiles (#1024) but did not make it into the squash that landed. The first is a live breakage on `development`. ## 1. `development` is failing check:schema-l10n right now seed-profiles declared `Regeling.status` — a property that was REQUIRED and named by the schema's own `x-openregister-lifecycle` map, but never declared, so OpenRegister created no magic-table column and the lifecycle could never advance. The description it shipped with was 782 characters of engineering rationale: which magic table was measured, why the column was missing, what the transition map could not do. `fieldsFromSchema()` renders a property description as **the helper text under a form field**, so that text was pointed at somebody filling in a regulation. It is also not a translatable sentence, and it pushed the uncovered-schema-string ratchet from 1632 to 1633. So the description is now one short line for the reader of the form, and the rationale moved to a `_statusNote` sibling that no UI renders and the checker does not read. The short line is in en.json and nl.json, with l10n rebuilt. Ratchet: 1631 uncovered against a baseline of 1632. ## 2. "Schemas missing after import" was a paging artefact `ci-seed.sh` verified the register import by fetching `?_limit=1000` and reporting every required slug it could not find: ::error::Decidiq schemas missing after import: ['meeting', 'action-item', 'minutes', 'vote', 'transcript', ...] All ten were in the database. A dev instance with 35 apps installed holds 1111 schemas, so ten decidiq rows fell off the end of the page. The import had worked; the question was too small — and the error sent you to investigate the import, which is the worst shape a wrong answer can take. The limit now outruns any plausible instance, and truncation is DETECTED rather than assumed away: a response that comes back exactly full says so, and names the page as the suspect instead of the import. ## Verification 1275 unit tests, 378 vitest, PHPCS 0 errors, PHPStan clean, manifest valid, and all three l10n checks green. Prettier applied to the e2e spec. * fix(rbac): the governance scope backfill asked for a schema slug that does not exist `GovernanceRoleScopeProjector::reconcileAll()` called `setSchema('governancebody')`. The register declares `governance-body`, hyphenated, like every other slug this app uses. So `findAll()` threw. Its only caller is `ProjectGovernanceRoleScopes`, a repair step, which catches `\Throwable` and downgrades it to a warning — correctly, because a repair must never fail an upgrade. The result is that every upgrade this app has ever run printed Governance RBAC scope backfill skipped: Schema slug "governancebody" is not carried by register "decidiq" (id 28), which carries 91 schema(s). 0 schema(s) elsewhere on this instance carry this slug. and REQ-RBAC-001 never projected a single body. Nothing failed. Nothing was done. The warning even names the real cause, and reads as an environment problem. Measured on a live instance, before and after: `reconcileAll()` threw; it now returns **22 bodies reconciled**. ## The guard matters more than the character `setSchema()` takes a slug the register owns, resolved at runtime, so code and register can drift with nothing to notice — and the one caller that would have noticed is required to swallow it. So the new test compares every `setSchema('…')` literal in `lib/` against every slug the register and its fragments declare. It fails on the original typo, naming both the slug and the file: 'governancebody (asked for in GovernanceRoleScopeProjector.php)' Eleven other call sites were already correct; this was the only one that was not, and it was invisible. 1276 unit tests pass. PHPCS 0 errors, PHPStan clean. --------- Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
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.
Installing this app planted 334 objects nobody asked for.
SettingsService::loadConfiguration()mergesdecidesk_register.jsonwith all 26register.dfragments, and every one of them carried its ownx-openregister.seedData.objects. TheInitializeSettingsrepair step runs that merge, so a fresh install seeded a Gemeenteraad Amsterdam, a VvE Zeewaarts, a pub quiz, five ACME B.V. bodies and eight placeholder TOOI mappings into the operator's register. A municipality got the VvE data. A VvE got the council data.The wizard meanwhile offered a different dataset, whose own step text said "Skip this on a production install". By the time an operator read that sentence, 334 objects were already in their register.
So the fragments now declare schemas only, and the objects live in
lib/Settings/profiles/, one file per example set. A bare install plants nothing; the wizard asks which organisation this is for, and loads that.The split had to be a closure, not a partition
The seeds cross-reference each other by slug. The reference graph has one connected component of 170 objects tangling municipal, corporate and association bodies together, so no partition exists. Each set is instead anchored on its governance bodies and closed over outbound references, so every reference resolves inside the set carrying it. Sets may overlap, and 15 objects do. Verified mechanically: 334 of 334 classified, zero dangling references, zero orphans.
Why the descriptor declares no register
An example set carries
@self.configuration/register/schemaon every object and declares nocomponents.registers. That is load-bearing, not stylistic.ImportHandler::importRegister()callssetApplication($appId)unconditionally when it updates an existing register, so a descriptor that declareddecidiqwould re-point the register at the profile's config id and hydrate over itsauthorizationblock: the baseline that stops any authenticated user rewriting another body's decisions.Verified on a live instance: importing this shape left
application=decidiq, the version and the authorization hash byte-identical, imported 45 objects, and added nothing on a second run.The files sit in a subdirectory because
RegisterDescriptorServicescanslib/Settings/*.jsonnon-recursively and indexes by declared register slug; four profiles inlib/Settingswould collide with each other and with the app's own register.Two wizard steps, because an action carries no body
CnSetupWizard::runAction()posts to/api/setup/action/{action}with no body, so an action cannot carry the answer. Achoicestep records which set via the newPOST /api/setup/config, and therun-actionstep reads it back. ADR-111 keeps the schema-generated mock on offer, so it is one option in the same choice rather than a second question about the same thing.Ten pre-existing defects this surfaced, all fixed
Validating every seed against its schema found:
regulation, which is not a decidiq schema slug. The app's isregeling. BecauseimportSeedData()resolves a slug cross-app with multitenancy off,regulationresolves to learniq's schema.Regeling.statuswas required and lifecycle-driven but never declared as a property, so OpenRegister created no magic-table column. Measured:oc_openregister_table_21_262carried every other property and nostatuscolumn, which means the declaredin-preparation → adopted → in-effect → lapsedmap could never advance a regulation.decision-stageseeds.domain.governanceBody: gemeenteraad-amsterdam, a null-UUIDdirector, and a raadsvergadering as their overlegvergadering. Replaced with a real 45-object set built around an ondernemingsraad at ACME B.V.Verification
1235 PHP unit tests and 378 vitest tests pass. PHPCS 0 errors; PHPMD, PHPStan and Psalm clean. Manifest validates against schema 2.26.0. The manifest-drift guard was proved to fail when the manifest and the shipped sets disagree, rather than passing vacuously.
Known, and not caused by this change
Hydra gates 22 and 53 fail on this branch and on
developmentalike: hydra-gates v1.10.0 vendors manifest schema 2.25.0, which predates theflowpage typeFlowDetailalready uses. Page 27 is byte-identical at HEAD. Fleet debt inConductionNL/.github, being fixed separately.🤖 Generated with Claude Code