feat(cmdb-import): run the mapping packs stored in OpenRegister, seeded from the shipped files by WilcoLouwerse · Pull Request #1288 · ConductionNL/stackiq

WilcoLouwerse · 2026-10-12T10:24:32Z

What

The CMDB import runs the mapping packs stored in OpenRegister, where an admin views and edits them (ConductionNL/openregister#4602, the migration-pack editor). The packs under lib/Settings/cmdb-import/ stay the seed and the fallback. Rework of this PR's first version (read-only overview of the shipped files) after the decision of 2026-10-12: mappings are managed in OpenRegister.

How

Resolution (CmdbImportProfile, CmdbImportPackStore): module, manufacturer, usage and business owner come from OpenRegister's migration-pack store by slug; the municipality pack maps the import options and always comes from its file. A stored pack is validated by PackDefinitionValidator and by the import's own rules (CmdbImportPackRules): required targets, and a per-pack allowlist of plain fields — relations, publication dates, @self, ids and admin-only fields are refused. A broken pack stops the import with 503 MAPPING_UNAVAILABLE naming the pack, never a silent fallback.
Seeding (SeedCmdbImportPacks, install + post-migration; version bumped so it runs on upgrade): the 4 packs are created as built-in with an x-stackiq marker (shipped version + hash). A later release updates a stored pack that was not edited; an edited one is left alone and reported as "newer version shipped". A deleted pack is seeded again on the next import.
Store availability: OpenRegister's pack store counts only when it has the hardened write routes (openregister#4602). Without it the import uses the shipped files and says so in the report; once a seed happened, a missing store fails closed.
Date columns: the profile's plus every column a pack maps with a date transform.
GET /api/settings/cmdb-import/mapping adds per pack source, packId, storedVersion, shippedVersion, edited, newerShipped, editable, editUrl (IURLGenerator), and top-level openRegisterPacks.
OpenSpec change cmdb-import-mapping-view rewritten for this scope (one mapping per instance); docs/features/cmdb-import.md explains viewing and editing in OpenRegister.

Checks

PHPUnit (Cmdb services, the repair step, CmdbImportController, the mapping endpoint) 258 passed; hydra gates (local, diff scope) all applicable green incl. gate 110 (version bump).

Rig (NC 35; OpenRegister development + openregister#4602; fixtures only): the 4 packs are seeded (on an instance that upgraded stackiq before OpenRegister had the hardened store, the seed runs at the next repair or import); the endpoint reports source: openregister with edit links; an edited pack is what the import runs (2/2 rows); a pack mapping provider stops the import with 503 naming the pack; restoring the pack makes it "not edited" again.

Merge order and conflicts

Needs feat(migration-packs): view and edit migration packs in OpenRegister openregister#4602 in the OpenRegister the instance runs to use the stored packs (without it: shipped files + a report warning). Do not promote the stackiq beta before that OpenRegister release.
Conflicts with feat(cmdb-import): read the archive sheet and let module and usage be archived #1285–feat(cmdb-import): offer the missing-records choice and an Archived quick filter #1287 (WOO-587) in CmdbImportProfile.php, CmdbExportImportService.php, their tests, docs/features/cmdb-import.md, the main spec list and l10n: whichever merges second keeps both sides (the rig build merged them that way).

Jira: WOO-588 — plan: https://github.com/ConductionNL/readonly-mirror-wilco-claude-plans/blob/dev-mirror/issues/jira/WOO-586/sub-tasks/WOO-588/WOO-588-plan-09-10.txt

🤖 Generated with Claude Code

Adds the OpenSpec change cmdb-import-mapping-view (ADDED REQ-CMDB-020, MODIFIED REQ-CMDB-014) and the admin-only GET /api/settings/cmdb-import/mapping. It loads the import profile and its five packs through CmdbImportProfile, the loader the import uses, and answers the flat {profile, packs}; a broken pack answers 503 MAPPING_UNAVAILABLE with the loader's reason. First of two slices of #1280. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-12T10:39:00Z

Quality Report — ConductionNL/stackiq @ `fb382bb`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
format		✅
check-schema-l10n		✅
check-l10n-js		✅
composer			✅	✅ 133/133
npm			✅	✅ 807/807
app:check-code	⏭️
info.xml	✅
REUSE	❌
lockfile sync		✅
PHPUnit					✅ 1230/1230 · 20 skipped
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 locally with npx playwright test, or from the Actions tab on a branch with no open pull request into `development`.
Hydra gates					✅

Quality workflow — 2026-10-12 10:38 UTC

Download the full PDF report from the workflow artifacts.

…line The mapping endpoint returned the loader's exception message as details.reason unbounded, and its docblock and design D2 said that message named files and validator rules only. The validator's messages also quote the configuration value they refused, and nothing limited their length. The reason is now the message's first line, cut to 500 characters, and both texts say what it carries: the file, the rule and the refused value, never a workbook cell or a person. Mutation-checked: testTheReasonIsOneBoundedLine fails when the endpoint returns the raw message and passes with the bound. phpcs, phpmd and phpstan report 0; the CMDB and SettingsController suites pass (306 tests). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The cmdb-import-mapping-view delta now says details.reason is one line of at most 500 characters and may quote the refused configuration value, as the endpoint and design D2 do since the reason is bounded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

WilcoLouwerse · 2026-10-12T12:49:30Z

+			array_column($data['packs'], 'file')
+		);
+
+		// What is shown is what runs: every pack lists as many mappings as the loader hands the engine.


🟢 Minor — 'Same loader' and 403 are asserted by no test

Contract-branch table: (1) 'the same loader the import uses': the test builds new CmdbImportProfile(...) itself, passes it in, and compares the answer with $profile->pack() of the same object, so it would stay green if the controller's fallback built a different directory or class; nothing ties it to the instance CmdbExportImportService gets (both are container-autowired today: no CmdbImportProfile registration in Application.php, so the claim holds, unpinned). (2) 'non-admin 403': only static reflection (no attribute) is tested; the 403 itself is the rig check in the PR body and the e2e in #1289. (3) 503 branches 'profile unreadable' and 'profile names no pack for target' have no test at either level (only a missing pack file and an invalid pack are covered).

Suggested fix: one test that resolves the controller's default (cmdbImportProfile: null) overview and the overview of new CmdbImportProfile(container: ...) and asserts equal; one profile-level test for a profile without packs.usage.

Done when:

the null-profile controller test goes red when the fallback in getCmdbImportMapping() is pointed at another directory

WilcoLouwerse · 2026-10-12T12:49:30Z

+				[
+					'success' => false,
+					'error' => $e->getErrorCode(),
+					'message' => $this->translate(text: 'The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.'),


🟢 Minor — New 503/500 messages are not extractable and not translated in this slice

The two user-facing messages go through a private translate(string $text) that calls $this->l10n->t($text) with a variable. Nextcloud's translation tooling extracts literal ->t('...') calls, and tests/l10n/check-l10n.js does not scan PHP at all, so nothing ensures these strings reach l10n/nl.json. The l10n files are in #1289; with this slice alone git grep -n -e 'cannot be shown' -- l10n is empty, so a Dutch admin gets English until the second PR lands (REQ-CMDB-020/ADR-005 require Dutch and English for the error).

Suggested fix: call $this->l10n->t('...') with the literal string (CmdbImportController does), and keep the null-translator fallback in the constructor default instead of a wrapper; or note in the PR body that the slice ships English-only until #1289.

WilcoLouwerse · 2026-10-12T12:49:30Z

+  - GIVEN the endpoint's answer WHEN the block is expanded THEN one table per pack renders with `data-testid="cmdb-import-mapping-<target>"`, the usage table holding "Applicatie Status" → `status` with the lookup pairs
+  - GIVEN a 503 WHEN the block loads THEN it shows the error and the code and no table
+  - GIVEN the answer WHEN the section renders its help text THEN the sheet names are the endpoint's; before and on failure they are `PROFILE_DEFAULTS.sheets`
+- [x] Implement


🟢 Minor — tasks.md marks Tasks 3-5 implemented; their files are not in this slice

Task 3 (CmdbImportMapping.vue, vitest), Task 4 (l10n, docs section) and Task 5 (e2e) carry [x] Implement, and the spec's @e2e tests/e2e/spec-coverage/cmdb-import-mapping.spec.ts points at a file that does not exist at this head (checked: CmdbImportMapping.vue, cmdbImportMapping.spec.js, cmdb-import-mapping.spec.ts are all absent). They arrive with #1289, so this is correct for the stack but wrong for #1288 alone, and if #1289 is split or reordered the checkboxes mislead. Not blocking; flag so the stack order is deliberate.

Suggested fix: none required; tick Tasks 3-5 in #1289.

Split artefact: this slice was cut from a larger PR; the item resolves once the stack above it merges. Fix only if this slice may merge on its own for a while.

github-actions · 2026-10-12T12:52:02Z

Quality Report — ConductionNL/stackiq @ `2913bd6`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
format		✅
check-schema-l10n		✅
check-l10n-js		✅
composer			✅	✅ 133/133
npm			✅	✅ 807/807
app:check-code	⏭️
info.xml	✅
REUSE	❌
lockfile sync		✅
PHPUnit					✅ 1231/1231 · 20 skipped
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 locally with npx playwright test, or from the Actions tab on a branch with no open pull request into `development`.
Hydra gates					✅

Quality workflow — 2026-10-12 12:52 UTC

Download the full PDF report from the workflow artifacts.

WilcoLouwerse

Verdict: APPROVE (Thorough) — self-review posted as COMMENT (GitHub blocks self-APPROVE)

No blockers: the endpoint is admin-only with CSRF, answers flat, logs unexpected errors with a static 500, and loads the profile through the same autowired CmdbImportProfile as the import. The unbounded failure reason was fixed before posting; what remains is that the view does not yet show the import's write rules.

Update 2026-10-12: The remaining concern was fixed on the branch after posting.

🟡 Concerns (1)

Overview omits the write rules, so 'what the next import runs' is overclaimed — lib/Service/Cmdb/CmdbImportProfile.php:629. The docblock (and SettingsController.php:3899, design.md:11, proposal.md:10, spec.md:11) promise that the answer is exactly what the next import runs. ✅ Addressed in ecc9c8b0, df28777e, 6ecb274a.

🟢 Minor (3)

'Same loader' and 403 are asserted by no test — tests/Unit/Controller/SettingsControllerCmdbImportMappingTest.php:225. Contract-branch table: (1) 'the same loader the import uses': the test builds new CmdbImportProfile(...) itself, passes it in, and compares the answer with $profile->pack() of the same object, so it would stay green if the controller's fallback built a different directory or class; nothing ties it to the instance CmdbExportImportService gets (both are container-autowired today: no CmdbImportProfile registration …
New 503/500 messages are not extractable and not translated in this slice — lib/Controller/SettingsController.php:3935. The two user-facing messages go through a private translate(string $text) that calls $this->l10n->t($text) with a variable.
tasks.md marks Tasks 3-5 implemented; their files are not in this slice — openspec/changes/cmdb-import-mapping-view/tasks.md:34. Task 3 (CmdbImportMapping.vue, vitest), Task 4 (l10n, docs section) and Task 5 (e2e) carry [x] Implement, and the spec's @e2e tests/e2e/spec-coverage/cmdb-import-mapping.spec.ts points at a file that does not exist at this head (checked: CmdbImportMapping.vue, cmdbImportMapping.spec.js, cmdb-import-mapping.spec.ts are all absent).

✅ Fixed before posting (1)

details.reason returns $e->getMessage() to the client — fixed in 0108ef0.

✅ Verified clean

No auth attribute (Nextcloud default: admin + CSRF) with @auth admin-only <reason>, asserted by testTheRouteIsForNextcloudAdminsOnly.
503 path: no absolute path or stack trace reaches the body; the 500 path returns a static message and logs the exception.
required is always a boolean; the spec's done → in-progress flip follows writing-specs' re-opening rule.

Scope decision: slice 1 of 2 of the closed #1280, stacked on development; reviewed against its own base only.
Gates: CI quality / Hydra Gates and the required PHP/Vue quality checks green on {h}; quality / PHPUnit was still running at submission.
ADRs verified: ADR-005, ADR-012, ADR-031, ADR-050 — all resolve on hydra main.
Jira context: WOO-588 — CMDB-import: mapping inzichtelijk en beheerbaar voor de beheerder (status: Refinement)

…view The overview listed the pack field mappings only, so the usage fields interneAnnotation and timeClassification looked like ordinary mappings while the import writes them on create or when the stored value is empty, and the module type Application and the never-written publication dates showed nowhere. mappingOverview() now answers per pack `defaults`, `createOnlyDefaults`, `createOnlyFields` and `neverWritten`, built from the accessors the import uses, plus the profile's `externalKeyPrefix`. REQ-CMDB-020, tasks.md and design.md name the new keys; the controller and profile tests pin them. Review #1288 f2 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

createOnlyFields is documented as mapped fields, but for the module pack the accessor returned ['type'], a key of the create-only defaults that merge() never reads on update (it only walks mapped fields). The overview now answers that value under createOnlyDefaults only and subtracts the default keys from createOnlyFields; the import's accessors are unchanged. REQ-CMDB-020 and design.md say so, and the controller and profile tests pin it. Review pre-push p6 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…OnlyFields The overview subtracted every key of createOnlyDefaults from createOnlyFields. merge() guards a mapped create-only field on update, so a mapped field that also had a create default would have shown as written on every update. Only defaults that are not mapped fields leave the list now (the shipped module type); REQ-CMDB-020, design and the overlap assertions say the same. Mutation-checked: testAMappedCreateOnlyDefaultStaysACreateOnlyField fails on the previous subtraction and passes now. Review pre-push q1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The mapping tables listed every mapped field as written on each import, but the import writes the usage's interneAnnotation and timeClassification only on create or when the stored value is empty, sets the module type on create, and never writes the module's publication dates on an existing record. The block now shows, per row, when the import writes the field, and under each table the pack defaults, the create-only values and the never-written fields that the endpoint answers since the previous slice. New strings in en and nl; REQ-CMDB-020, the user docs, the vitest and the e2e API check name them. Review #1288 f2 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-12T16:17:23Z

Quality Report — ConductionNL/stackiq @ `e58a1d5`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
format		✅
check-schema-l10n		✅
check-l10n-js		✅
composer			✅	✅ 133/133
npm			✅	✅ 807/807
app:check-code	⏭️
info.xml	✅
REUSE	❌
lockfile sync		✅
PHPUnit					✅ 1232/1232 · 20 skipped
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 locally with npx playwright test, or from the Actions tab on a branch with no open pull request into `development`.
Hydra gates					✅

Quality workflow — 2026-10-12 16:17 UTC

Download the full PDF report from the workflow artifacts.

The module, manufacturer, usage and business-owner packs now live in OpenRegister's migration-pack store, where an administrator edits them, and the import runs what is stored there. A repair step seeds missing packs as built-in packs and replaces a pack nobody edited when a release ships a newer one; an x-stackiq marker (shipped version and the hash of the shipped pack) tells an edit from a release, and an edited pack is never overwritten. The shipped files stay the seed, and the fallback while nothing was stored. Every pack must follow the import's rules: the required name and APPID mappings, and only fields of its target schema, never object metadata, match keys or the relations the import sets. An invalid or rule-breaking pack, a renamed pack, or a store that is gone after a seed answers 503 MAPPING_UNAVAILABLE naming the pack and its id. A column a pack maps with a date transform is converted from an Excel date, and the note cleanup uses the pack's own separator. The municipality pack stays a shipped file. GET /api/settings/cmdb-import/mapping answers per pack its source, OpenRegister id, stored and shipped versions, edited and newerShipped, editable and an editUrl to OpenRegister's pack editor, plus openRegisterPacks. The OpenSpec change and the documentation describe editing in OpenRegister. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…rdened pack stores The import's rules took every property of the target schema as writable, so an edited pack could map relations (organization.participants writes back into other organisations), publication dates and admin-only fields, all saved with RBAC and multitenancy off. Each target now has an explicit allowlist of vetted plain fields: what the shipped packs map, the module's licentietype and the usage's phase start dates. Relations ($ref, inversedBy, writeBack, uuid, related objects, on the property or its items), publication dates and admin fields are refused as mappings and as defaults; the organisation status and registeredBy defaults stay pinned. OpenRegister's pack store now counts as available only when BuiltinPackSlugLockedException exists, i.e. on the OpenRegister whose pack writes require CSRF and lock a built-in slug. Released versions keep the shipped files with a warning, and fail closed once ids were recorded. A seed that loses a create race to another request uses the row that request stored instead of answering 503. The note cleanup keeps the concat separator when a later plain mapping of the same field exists, because the engine skips a mapping whose column is empty. The mapping endpoint's 503 for a stored pack carries details.editUrl to its editor. The spec delta keeps only scenarios an e2e test proves: the new cmdb-import-packs.spec.ts drives the store, the mapping and the import through the API. Store-removed, install and upgrade behaviour stays in the requirement text and PHPUnit; the UI scenarios move to the UI change. Review findings S1, S2, F2 (endpoint part), F5, F6, F7 (endpoint part) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…es it The loader's docblock still said a pack may map every field of its target schema; it now names the rule CmdbImportPackRules applies. Review finding S1 (sweep) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…pgrade SeedCmdbImportPacks is registered as a post-migration repair step, but Nextcloud only runs repair steps when <version> in appinfo/info.xml moves. With the version unchanged, `occ upgrade` answers "No upgrade required." on an existing install and the packs are never seeded. 0.2.5-unstable.20261008120000 -> 0.2.6-unstable.20261012120000, in line with the 0.2.6 unstable releases already cut from development. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

github-actions · 2026-10-12T19:39:30Z

Quality Report — ConductionNL/stackiq @ `b45ea87`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
format		❌
check-schema-l10n		✅
check-l10n-js		✅
composer			✅	✅ 133/133
npm			✅	✅ 807/807
app:check-code	⏭️
info.xml	✅
REUSE	❌
lockfile sync		✅
PHPUnit					❌ 1307/1308 · 20 skipped
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 locally with npx playwright test, or from the Actions tab on a branch with no open pull request into `development`.
Hydra gates					✅

Quality workflow — 2026-10-12 19:39 UTC

Download the full PDF report from the workflow artifacts.

WilcoLouwerse and others added 2 commits October 12, 2026 14:43

WilcoLouwerse commented Oct 12, 2026

View reviewed changes

WilcoLouwerse and others added 3 commits October 12, 2026 17:04

WilcoLouwerse and others added 4 commits October 12, 2026 18:52

WilcoLouwerse mentioned this pull request Oct 12, 2026

feat(migration-packs): view and edit migration packs in OpenRegister ConductionNL/openregister#4602

Open

WilcoLouwerse changed the title ~~feat(cmdb-import): expose the import mapping read-only to the admin~~ feat(cmdb-import): run the mapping packs stored in OpenRegister, seeded from the shipped files Oct 12, 2026

Navigation Menu

Repository navigation

feat(cmdb-import): run the mapping packs stored in OpenRegister, seeded from the shipped files - #1288

feat(cmdb-import): run the mapping packs stored in OpenRegister, seeded from the shipped files#1288
WilcoLouwerse wants to merge 10 commits into
developmentfrom
feat/woo588-cmdb-import-mapping-view-1-endpoint

WilcoLouwerse commented Oct 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

Uh oh!

WilcoLouwerse Oct 12, 2026

Uh oh!

WilcoLouwerse Oct 12, 2026

Uh oh!

WilcoLouwerse Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

WilcoLouwerse left a comment •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

WilcoLouwerse commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Checks

Merge order and conflicts

Uh oh!

github-actions Bot commented Oct 12, 2026

Quality Report — ConductionNL/stackiq @ fb382bb

Uh oh!

Uh oh!

WilcoLouwerse Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

WilcoLouwerse Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

WilcoLouwerse Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 12, 2026

Quality Report — ConductionNL/stackiq @ 2913bd6

Uh oh!

WilcoLouwerse left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Verdict: APPROVE (Thorough) — self-review posted as COMMENT (GitHub blocks self-APPROVE)

🟡 Concerns (1)

🟢 Minor (3)

✅ Fixed before posting (1)

✅ Verified clean

Uh oh!

github-actions Bot commented Oct 12, 2026

Quality Report — ConductionNL/stackiq @ e58a1d5

Uh oh!

github-actions Bot commented Oct 12, 2026

Quality Report — ConductionNL/stackiq @ b45ea87

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

WilcoLouwerse commented Oct 12, 2026 •

edited

Loading

Quality Report — ConductionNL/stackiq @ `fb382bb`

Quality Report — ConductionNL/stackiq @ `2913bd6`

WilcoLouwerse left a comment •

edited

Loading

Quality Report — ConductionNL/stackiq @ `e58a1d5`

Quality Report — ConductionNL/stackiq @ `b45ea87`