fix(demo-data): the example data no longer creates apps that cannot be opened - #853
Merged
Merged
Conversation
The setup walkthrough's example data imported three objects for every schema in the buildiq register. That register holds the schemas buildiq keeps about itself, so the import created three apps that appear in the Apps list and on the dashboard and cannot be opened: their applicationVersion rows carry application: 00000000-0000-4000-8000-000000000000, the placeholder a format: uuid property gets, and a manifest with no pages, so the detail page renders empty and /apps/buildiq/builder/<slug>/ never resolves. Three more stood in the template store beside the four real built-ins and would clone into an equally empty app, and three export-job rows read as finished exports nobody ran. Seen on the demo instance on 2026-09-18, app c5a2e155-432b-4ea8-a965-56661b991463, slug ccdc. Every one of those objects satisfies its schema, which is why the generator and its --check were both green. Conformance is about an object's shape; nothing in a schema says whether its rows are content somebody authors or bookkeeping the app writes. Eight schemas now say so with x-openregister-demo-data, the marker ConductionNL/.github#781 adds: built-app the register of apps buildiq has built applicationVersion written by publish and promote built-app-route an index the Application lifecycle maintains application-template the store's starter packs export-job written by the export pipeline rule-execution-log written by the rule engine, a GDPR art. 22 record agent-run written by AgentRunLogger formDraft one person's half-filled form, owner-only hello-message keeps its three objects: it is the app's content schema and the reason the dataset exists. So do the schemas an administrator authors by hand (automation, buildAgent, component-block, pageLayout, registrationForm and the rule-set family): their rows are content, not bookkeeping, and none of them generates today anyway. Regenerated with the patched generator. The dataset goes from 18 objects to 3, and --check exits 0 with five SKIP lines that each say why.
…ookkeeping The dataset is generated, so it can regress without anybody editing it: a generator run against a schema that has lost its declaration writes the control-plane objects straight back. They then import in silence, because they satisfy their schemas, OpenRegister has no opinion about what they mean, and the wizard reports a cheerful count over three apps that cannot be opened. DemoDataService now reads the x-openregister-demo-data declarations from the app's own descriptors, the same file the generator reads, and refuses the import before anything is written when the dataset addresses one of them. It throws rather than filtering: half a dataset is not what the operator asked for, and SetupController already surfaces the message. Three arms cover it, plus a guard on the shipped file: - DemoDataServiceTest: the refusal, that the importer is never called, and the control that an ordinary content schema still imports. - DemoDatasetExcludesControlPlaneTest: the shipped dataset carries no object for a declared schema, the five control-plane schemas still declare it with a reason, and hello-message still carries its three objects. Verified by mutation. Against the dataset shipped before this change the guard test reddens naming all five schemas and three objects each; with the refusal removed from install(), two of the three service arms redden and the control stays green.
… clean The first version put path discovery, the mock filter and the per-schema decision in one method: cyclomatic complexity 15 against a threshold of 10, an NPath of 1188 against 200, and five phpcs errors from the `?:` on glob and an unnamed argument. All NEW findings on lines this change wrote, so they are this PR's to fix. No behaviour change. phpcs, phpmd and phpmd-unusedparams all exit 0 on the file, and the sixteen demo-data tests still pass.
Contributor
Quality Report — ConductionNL/buildiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ❌ | ||||
| phpstan | ❌ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| format | ❌ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ❌ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 752/752 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ | ||||
| Newman | ❌ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ❌ |
Quality workflow — 2026-09-18 10:22 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
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.
Found while recording a demo. The setup walkthrough's "Load example data" step imported three objects for every schema in the buildiq register, and that register holds the schemas buildiq keeps about itself. The import therefore created:
built-approws ("Voorbeeld Name 1..3") that appear in the Apps list and on the dashboard and cannot be opened. TheirapplicationVersionrows carryapplication: "00000000-0000-4000-8000-000000000000", the placeholder aformat: uuidproperty gets, and a manifest with zero pages, so the detail page renders empty and/apps/buildiq/builder/<slug>/never loads.application-templaterows standing in the Store beside the four real built-in templates. Cloning one produces an equally empty app.export-jobrows reading as finished exports nobody ran.Measured on the demo instance on 2026-09-18 before they were deleted by hand: app
c5a2e155-432b-4ea8-a965-56661b991463, slugccdc, status archived, opened to an empty detail page, and its version hadmanifest.pages: [].Why nothing caught it
Every one of those objects satisfies its schema, so the generator was right and
--checkwas green. Conformance is about an object's shape. Nothing in a schema says whether its rows are content somebody authors or bookkeeping the app writes, so no check could tell the difference and ADR-111 rule 1 asked for three of each.What changed
Eight schemas now declare
x-openregister-demo-datawith the reason, and the generator honours it. The marker is added in ConductionNL/.github#781.built-appapplicationVersionbuilt-app-routeapplication-templateexport-jobrule-execution-logagent-runAgentRunLoggerformDraftThe first five are the ones the generator actually wrote objects for. The last three generate nothing today, because the register lists its schemas by slug while their definitions are keyed by name, so the resolver never pairs them. They are marked anyway: that resolution is a plausible thing to fix, and the day it is fixed is the day this bug comes back.
Kept on purpose.
hello-messagekeeps its three objects: it is the app's content schema and the reason the dataset exists. So do the schemas an administrator authors by hand,automation,buildAgent,component-block,pageLayout,registrationFormand therule-setfamily. Their rows are content, not bookkeeping. They do reference an Application that the demo data no longer creates, but a dangling reference in generated data is a different defect, and the fix for it is reference-aware generation rather than exclusion.The dataset goes from 18 objects to 3.
The import now defends itself
The dataset is generated, so it can regress without anybody editing it. A generator run against a schema that has lost its declaration writes the control-plane objects straight back, and they import in silence: they satisfy their schemas, OpenRegister has no opinion about what they mean, and the wizard reports a cheerful count over three apps that cannot be opened.
DemoDataService::install()now reads thex-openregister-demo-datadeclarations from the app's own descriptors, the same file the generator reads, and refuses before anything is written when the dataset addresses one of them. It throws rather than filtering, because half a dataset is not what the operator asked for, andSetupControlleralready surfaces the message.How this was verified
Mutation first, because a guard that cannot fail reports the same green as one that passed.
DemoDatasetExcludesControlPlaneTestagainst the dataset shipped before this change: red, naming all five schemas and three objects each.install(): two of the three service arms red, and the control arm (an ordinary content schema still imports) green, which is what separates "the guard works" from "the guard refuses everything".--checkon the old dataset with the new marker: exit 1, five FAIL lines. On the new dataset: exit 0, five SKIP lines that each say why.Local, on this branch,
composer check:strictonce andnpm run lintonce:npm run linttest:allphpcsAppInfo/Application.php,Integration/PageLayoutLeafProvider.php,Integration/RegistrationFormLeafProvider.php,Listener/BuildiqLeafRegistrationListener.php,Service/LayoutDeltaService.php,Service/PageLayoutValidator.php,Service/RegistrationFormValidator.phppsalmPageLayoutLeafProvider,RegistrationFormLeafProvider,BuildiqLeafRegistrationListenerphpstanPageLayoutLeafProviderandRegistrationFormLeafProviderphpmdgatescomposer check:strictDemoDataServicethat do not exist.~/.pdependis shared by every checkout on this machine, and it served the pre-0dbf2fd5analysis of the file: complexity 15 and NPath 1188, the exact numbers from before the split. Running the same command with a privateHOME, so pdepend builds a cold cache, returns zero findings on the file and 20 inherited ones acrossLayoutDeltaService,PageLayoutLeafProvider,RegistrationFormLeafProvider,PageLayoutValidatorandRegistrationFormValidator.phpcs,phpmdandphpmd-unusedparamseach exit 0 against the file directly.gate-60 icon-vocabulary fails on
src/icons.js:ContentSaveEditOutlineandFormSelectare used by a manifest and not registered. Inherited. Both names sit inlib/Settings/register.d/50-registration-forms.jsonondevelopmentalready, andsrc/icons.jsis not in this diff. It belongs to the CI repair lane that owns that file.gate-101 demo-data-coverage fails locally, and it will fail on this PR's CI too, until ConductionNL/.github#781 merges. The gate runs the generator from
conduction/hydra-gates, and the version onmaindoes not know the marker yet, so it asks buildiq for the threebuilt-approws this PR removes. Verified by running the patched generator by hand with the gate's own arguments (--check --only-changedover this diff's files): exit 0, five SKIP lines. Merge #781 first.developmentis red on 10 jobs from other work, so the comparison above is against that branch rather than against green.What an existing instance still needs
Nothing in this PR touches data that is already there. An instance where somebody ran the demo import keeps its fifteen rows until a person deletes them, and this change only stops the next import creating more. They are:
They were deleted by hand on the demo instance on 2026-09-18.
A repair step should not clean them up, and this PR does not add one. ADR-111 rule 3 keeps demo data out of Repair in one direction, and the same reasoning holds in the other: a Repair step runs on upgrade, unasked, on instances nobody is watching. The rows it would have to delete are
built-appobjects, which ARE buildiq's app registry, and the only thing separating a demo app from a real one is a slug pattern somebody could have typed. A heuristic delete that is wrong once has removed an app and its version history. Nothing inopenspec/changes/openbuild-first-time-setup/asks for a removal path, so there is no spec to implement against either.The honest repair is the one an operator performs deliberately, after looking at the list. If that turns out to be common, the thing to build is an "uninstall example data" action on the walkthrough step that removes exactly the slugs the shipped dataset declares, as a decision the operator makes rather than one an upgrade makes for them. That needs a spec first.
🤖 Generated with Claude Code