Repository navigation
feat(migration-packs): view and edit migration packs in OpenRegister - #4602
Conversation
…s, add preview endpoint
- create/update accept {"definition": {...}}; the plain body form stays
supported. update/destroy take the pack row id from the route, because
Nextcloud merges a JSON body over the URL parameters, so a body `id`
(the pack slug) replaced the route id and was cast to 0.
- MigrationPackService::update() refuses to rename a built-in pack
(BuiltinPackSlugLockedException, 422 builtin-slug-locked), so apps that
seed a pack keep finding it by slug.
- POST /api/migration-packs/preview maps up to 20 sample rows through an
unsaved definition with MappingEngine::mapRow; an invalid pack returns
422 invalid-pack with the validator errors.
- create/update/destroy/import/preview require a CSRF token and a
Nextcloud admin (AuthorizedAdminSetting plus the admin check in the
method body); NoAdminRequired/NoCSRFRequired removed from writes.
- update and destroy log the acting user and pack slug.
- Tests use a request double with Nextcloud's parameter merge order
instead of a mocked getParams(), which hid the route-id bug.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add a Migration packs list and a pack editor (WOO-588). The list shows
name, slug, source format, version, built-in and updated date, with
open, export, delete (warning that an app re-installs a built-in pack)
and JSON import. The editor has the general settings (slug read-only for
a built-in pack), an ordered field-mapping table with add, remove and
reorder, a transform dialog for every validator type with a three-way
choice for unmapped lookup values, defaults, rows to skip, an Advanced
(JSON) view and a sample-row test over /api/migration-packs/preview.
Saving sends {definition} on the route id, raises the patch version
when it was left unchanged, shows validation refusals per finding and
refuses to overwrite a pack someone else saved meanwhile.
packDraft.js keeps field mappings as an ordered list and rebuilds the
definition in the stored key order, keeping keys the editor does not
model, so the shipped stackiq TOPdesk packs and OpenRegister's template
pack save back unchanged.
The menu entry is gated on runtime.user.isAdmin, which MainMenu now
passes to CnAppNav, because a manifest permission is not evaluated
without a permissions list. Non-admins get read-only pages. Strings in
English and Dutch; user docs in docs/features/migration-packs.md.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Proposal, design, tasks and a delta on migration-mapping-packs adding requirements for the pack list and editor, a save that loses nothing it does not show, admin-only changes with a log line, the definition envelope with the route id winning, the locked slug of a built-in pack, the sample-row preview endpoint and CSRF on every write. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… a delete - create() and import() now write the same info line as update() and destroy(): acting user, pack slug and row id. An import also says whether it created a pack or replaced a stored one, because an import under an existing slug overwrites that pack, built-in ones included. A failed change writes no line. The spec and the feature page say these are info lines that Nextcloud keeps only at log level Info. - Deleting a built-in pack records its slug in app config. A pack later created, imported or renamed under that slug is built-in again, so its slug stays locked and the app that seeds it keeps finding it. - The six translated controller messages get Dutch entries in l10n/nl.json, where PHP IL10N reads them; a test checks every controller message against that catalogue. - A test drives update() through Nextcloud's own Request (route id 5, body id = slug, raw and envelope). It runs when the bootstrap loads Nextcloud and is skipped with the stub Request. - PHPUnit covers a preview whose JSON Pointer sources read a nested row. Review findings closed: SEC-1, SEC-2, SEC-3, F3, F5. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…js and nl.js The editor's strings had been added by re-serialising both bundles, which changed the indentation, spacing and key order of every line. Both files are development's again, with the 139 new keys appended in the existing format (two of them for the yes/no table wording). Review finding closed: F4. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s, JSON-view saves - The sample-row test sends a JSON Pointer column (`/a/b`) as a nested value, the way the mapping engine reads it. A flat "/identificatie" key was never found, so every pointer-sourced mapping, the seeded zgw-zaken-json pack's included, showed as missing. - A date format cleared in the transform dialog is left out of the transform, so the engine default applies. An empty targetFormat made the engine store every date as ''. - For a yes/no table the empty default is offered and summarised as storing false, which is what the engine does with it. - A save from the Advanced (JSON) view raises the patch version when the definition changed and the version did not, and the editor says so before saving, as a form save does. Review findings closed: F1, F2, F6, F7. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… src/dialogs The list declared its delete confirmation as an inline NcDialog, and the transform dialog lived under src/components. Both now sit in src/dialogs/migrationPack/, where OpenRegister keeps its other NcDialog components, and the list drives the delete dialog through open, pack and busy props with close and confirm events. The delete itself stays in the list, so its behaviour and notices are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"Migration pack" and "Migration packs" are the manifest's page titles and menu label. nl.js already carried them; nl.json, which the manifest l10n check reads, did not. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A CI spec for the migration-packs and migration-pack-detail pages: an administrator sees the list with a pack the spec created and opens it in the editor, the field-mapping table shows the pack's rows in order, and the seeded non-admin reads the pack by URL with no Save action. The pack is created through the admin API and deleted in afterAll. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s at the moved files The _notes now name what a typed page would need: nextcloud-vue 2.72.0 keeps its named index sources (flows, tasks) in a registry inside the library that an app cannot extend, and CnDetailPage only loads a register object. tasks.md lists the dialogs at their new paths and nl.json among the translated files. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 25837/25837 · 25 skipped | ||||
| Newman | ✅ 635/635 | ||||
| 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:51 UTC
Download the full PDF report from the workflow artifacts.
…t the request handling
Every error from the migration-pack endpoints now answers the shared
ADR-050 shape {message, error, details?} with static, translated text;
no response carries an exception message. Request parsing and preview
validation move to MigrationPackRequestReader, error responses and
logging to MigrationPackErrorResponder, so the controller passes phpmd
without new thresholds, baseline entries or suppressions.
- write endpoints are admin-only through Nextcloud's default middleware
(no AuthorizedAdminSetting), with the in-body check kept
- import reports created or replaced from the write itself
- preview caps the request at 1 MiB besides the row count
- a raw-body update whose id equals the route id is refused with a
pointer to the definition envelope
- a deleted built-in slug is remembered before the row goes, and
seeding a built-in pack takes its slug off the deleted list
- validator row numbers count from 1, matching the editor table
Closes review #4602 be-f1, be-f2, be-f3, be-f4, be-f5, be-f6, be-f7,
ci-f1.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… the editor accessible - the editor asks before leaving with unsaved changes (route leave, route update and beforeunload), through a new UnsavedChangesDialog - a local t() turns off escaping, so quotes and ampersands in messages show as typed - a yes/no table stores real booleans and refuses other results - the save conflict check also compares the definition and maps a 409/412 to the reload notice; a pack deleted while editing can be saved as a new pack - explainError() reads the error code and details.errors instead of parsing English exception text - the transform button's name includes its visible summary; row moves and removals keep focus and are announced; status notices and hints are exposed to assistive technology - the preview clears when the definition becomes invalid - sampleRowFrom() cannot pollute Object.prototype - a layout-only JSON edit no longer counts as a change - row labels are whole translated strings; dates use the Nextcloud locale Closes review #4602 fe-f1, fe-f2, fe-f3, fe-f4 (client side), fe-f5, fe-f6, fe-f7, fe-f8, fe-f9, fe-f10, fe-f11, fe-f12, fe-f13. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… scenario in e2e - spec.md passes openspec validate --strict; preview failures name their status and error code; every scenario has a GIVEN; the admin-gated CRUD requirement is carried as a MODIFIED block with preview, CSRF, the envelope, the built-in lock and the shared error shape - every scenario in the delta has a resolving @e2e anchor on a running Playwright test; the browser tests now cover edit and save, row moves, per-finding refusals, conflicts, delete, import, preview and the non-admin read-only pages; afterAll fails on a delete that did not succeed - proposal, design and tasks say that create, update, import and delete are logged, add a WCAG 2.2 AA section, explain the five fixtures against four seeded stackiq packs, and bring the task status in line with the branch - the feature docs gain a section on the error codes for scripts and API clients Closes review #4602 sp-f1, sp-f2, sp-f4, sp-f6, sp-f7, sp-f8, sp-f9, sp-f10, sp-f13, sp-f15; documents sp-f14. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds the 18 new frontend strings of the review fixes to en.js and nl.js,
and the 10 new backend IL10N messages to nl.json. Drops "Transform of row
{number}" from every bundle; the editor now uses the form with a colon.
The Swedish menu label "Migration packs" uses the indefinite form
"Migreringspaket", like the other Swedish menu entries.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds the 18 frontend strings of the review fixes to the JS bundles of the 35 required locales besides Dutch, so every locale stays at key-for-key parity with en.js. Placeholders are kept as-is and no value is left in English. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… check #4602) A create or rename to a slug another pack has was answered 422 `invalid-pack` with the validator's English sentence in `details.errors`, so a Dutch administrator read it untranslated. It now answers 422 `pack-slug-exists` with a localised message and no details, and the screens show their own text for the code. The status stays 422: the main spec's "Duplicate pack slug is rejected" scenario fixes it for API clients. PackSlugExistsException extends InvalidPackDefinitionException, so callers that catch the parent keep working. Design D-12, the proposal, the error list of the modified CRUD requirement and the docs error table name the new code. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ush check #4602) The editor showed a 412 on save as "Someone else saved this pack" and offered a reload that throws the edits away. On the update route Nextcloud answers 412 only for a failed request-token check, which @nextcloud/axios already retries once with a fresh token. A 412 now gets the ordinary save failure, and the edits stay. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… (pre-push check #4602) The only local run of migration-pack-pages.spec.ts failed every test with "MigrationPacksIndex did not render its heading". The page had rendered; the admin account was set to Dutch, and the tests find the pages by their English names. beforeAll now reads the language of both accounts and fails once, naming the account and the occ command that sets it to English. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…server (pre-push check #4602) The spec says a save over someone else's save is refused, but only the editor checked: it read the pack again before the PUT, and a save landing between that read and the PUT was overwritten. The update envelope now takes `expectedUpdatedAt`, the `updatedAt` the caller read. The service compares it with the stored pack, to the second, and refuses with PackChangedException, answered 409 `pack-changed`; a value that is not a date-time as the API writes `updatedAt` is 400 `invalid-request`. The editor sends it on every save and shows a 409 `pack-changed` with the reload. It keeps its own read-and-compare, which tells apart two saves within the same second. An update without the field replaces the pack as before, so API clients are unaffected. Design D-8 now says what is left: read, compare and write are three steps, so two updates arriving at the same moment can both pass. The spec gains a scenario with an e2e test, the error lists and the docs name the code, and task 1.7 records it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e-push check #4602) The preview refuses a definition and rows over 1 MiB of JSON together with 400 `invalid-request`, but the requirement listed every other 400 case and not this one, and neither design D-5 nor the docs error table named it. The requirement, D-5, task 1.3 and the docs now state the bound, and a new scenario has an e2e test anchored to it. testPreviewRejectsAnOversizedRequest already covers it in PHPUnit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…sh check #4602) The editor refused every yes/no value that was not a JSON boolean, on every row at save and in the preview. A pack written through the API or imported with {"Ja": 1, "Nee": 0} loaded, but could not be saved after editing any other row, and its select showed "Choose yes or no". The engine casts 1 and 0 to true and false, so the editor now takes them, shows them as Yes and No, and saves an untouched 1 or 0 as it was. Text such as "false", which the engine casts to true, is still refused. The editor requirement now says so, and that the server's validator does not hold an API write to it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…iled (pre-push check #4602) Tasks 1.6 and 3.7 were still open and the status paragraph said the error shape of D-12 "is being built", although the branch implements both and their PHPUnit and Jest tests pass. Both are ticked with the tests that prove them. The paragraph and task 5.1 now say that the one Playwright run failed on an admin account set to Dutch, not that the file has never run. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…(pre-push check #4602) Nine Jest cases read MigrationPackDetail.vue, MigrationPacksIndex.vue and the dialog and panel sources as text and matched attribute strings. A prettier reflow turned them red, while a broken binding (the leave dialog never rendering, the live region on the wrong element, a non-admin seeing Delete) stayed green. The @nextcloud/vue stub now renders its slots and carries its bindings as attributes, so the screens mount with their whole template. The read-only pages, the delete warning for a built-in pack, the live regions, the heading focus, Save as a new pack, the transform button's name and the shared-target hint, the leave dialog, the yes/no table, the preview's blocked text and the key/value labels are now asserted on the rendered DOM and the child components' props. Each was checked against a mutation of the binding it covers: every mutation turns a case red. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…4602) Adds "Another pack already has this slug. Choose a different slug.", the editor's text for the new `pack-slug-exists` code, to en.js and nl.js, appended in the bundles' existing layout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…check #4602) Adds "Another pack already has this slug. Choose a different slug." to the JS bundles of the 35 required locales besides Dutch, appended in each bundle's layout, so every locale stays at key-for-key parity with en.js. Each locale uses its existing word for slug and its own register. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…n the service (pre-push check #4602) MigrationPackService::update() took the expected update time as a DateTimeInterface, which put the class one dependency over PHPMD's coupling limit. It now takes the Unix time; the controller converts the parsed request value. The comparison was already to the second, so nothing changes for callers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The defaults table and the field-mapping table both named their first input "Target field of row 1", so a screen reader heard two inputs with the same name and a locator for one mapping row matched both. The defaults column is now "Field", an existing translated string, so no locale changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The lookup rows are named "Value in the source of row N" and "Stored value of row N", and "Target field of row 1" also matched rows 10 and 11 without an exact match. Run against a live instance: 38 of 38 pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ck pages Task 5.1 is done: 38 of 38 tests passed against Nextcloud 35.0.1 with this branch built. The status paragraph no longer describes the earlier run that failed on an account set to Dutch. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Verdict: Approve once the running CI is green
Strict review of 99c6e4b, covering backend, frontend, and spec/tests/l10n. All 36 findings were fixed on this branch before posting, except three minors, listed under "Still open". This is posted as a comment because GitHub does not let an author approve their own pull request.
Blockers (inline, fixed before posting)
- 🔴 phpmd failed on the controller. The request handling is split into
MigrationPackRequestReaderandMigrationPackErrorResponder(e5d8799). - 🔴
openspec validate --strictrejected the change. Both requirement bodies now use SHALL (350e6b7). - 🔴 The Playwright spec had never run. It now passes 38 of 38 on Nextcloud 35.0.1 with this branch built. The run found a duplicate accessible name, which is fixed (1c54568, 8ef92f0).
Fixed before posting
Backend (e5d8799, 6d614a3, 27bc051, 93269f9)
- Every error from
MigrationPacksControllernow uses one ADR-050 shape,{message, error, details?}, and no response carries exception text. The codes arenot-authenticated401,admin-required403,pack-not-found404,invalid-requestandtoo-many-rows400,invalid-packandbuiltin-slug-locked422, andmigration-pack-error500 (logged). - The write routes no longer carry
#[AuthorizedAdminSetting], which would have let delegated admins in. They are admin-only through the default middleware, marked@auth admin-only, and the method body checks again. - An update based on an older read is refused on the server with 409
pack-changed. - A taken slug is answered with its own code.
- A built-in slug stays locked after a delete, even when remembering it fails.
- The import pre-lookup moved inside the try.
- A preview is capped at 20 rows and 1 MiB.
- Raw-body edge cases are handled.
- The docblocks describe the lifecycle of the slug lock correctly.
Frontend (c76eec7, b3844cc, 6cb5659, 7366876)
- Leaving the editor with unsaved changes asks for confirmation, both on a route change and on
beforeunload. The dialog lives insrc/dialogs/. t()no longer HTML-escapes the placeholders.- A yes/no table stores real booleans and keeps a stored 1 or 0.
- A 412 response is no longer read as someone else's save.
- A pack deleted while it is being edited shows a translated message and offers "Save as a new pack" or "Show the JSON".
- Errors are mapped from the slug to translated text, and
details.errorsis shown as a list. - Accessibility (WCAG 2.2 AA):
- The transform button's name contains its visible summary.
- Focus stays on a moved row, and the move is announced.
- Status messages and errors are in live regions.
- Hints are linked with
aria-describedby.
- The preview clears a stale result.
sampleRowFromcannot write to the prototype.- A whitespace-only edit in the JSON view no longer counts as a change.
- Row labels are built with
t(), and dates use the Nextcloud locale. - The read-only unit test now mounts the components.
Spec, tests and docs (350e6b7, 609444b, 98da1eb, 7321876)
- Every scenario has an
@e2e migration-mapping-packs::<slug>anchor on a test that runs. - The e2e spec covers what its scenarios claim:
- editing and saving a lookup value;
- moving a row;
- refused saves;
- delete, import, preview, the non-admin cases and the menu entry.
- Cleanup fails loudly when a delete fails.
- The proposal, design and tasks agree with the code on four audit lines.
- The proposal has a WCAG 2.2 AA section.
- The preview failure scenarios name their status codes and slugs.
- A MODIFIED block covers the admin-gated CRUD requirement.
- The design explains five fixtures against four seeded packs.
- The Swedish menu label is fixed.
- The strings added in review are translated in every locale.
Still open (minor, not blocking)
- The six controller messages in the backend catalogue are translated in
nl.jsononly. The other locales fall back to English for API error messages. The frontend shows its own translated text per error slug. testUpdateReadsTheRouteIdFromARealNextcloudRequestskips outside an installed Nextcloud. The design says where it runs (the in-container step ofapi-test-coverage.yml).- The l10n bundles are compact appends, not output of
apply.js. The repo rule saysapply.jsis the only writer. Ondevelopmentthe bundles are not in the writer's layout, though, so running the writer here re-serialises every bundle: about 248k changed lines. The writer's output is kept on a local branch for a separate, reformat-only PR if wanted.
Gates and checks
Local checks at 7321876:
- PHPUnit for the migration-pack controller, service and
MigrationPacktests: green. - Jest 109 of 109 for
src/views/migrationPack. test:l10nandtest:l10n:parityOK.openspec validate --strictvalid.- phpmd and phpcs clean.
- Playwright 38 of 38.
Hydra gates (local, diff scope, on c775d20): every applicable gate passes except gate-69 page-type-discipline. Custom pages went from 36 to 38. Both new pages carry a _note: migration packs are not register objects, and @conduction/nextcloud-vue 2.72.0 has no named index source an app can add. I accept that as the reviewer.
🤖 Generated with Claude Code
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 25881/25881 · 25 skipped | ||||
| Newman | ✅ 635/635 | ||||
| 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-13 08:48 UTC
Download the full PDF report from the workflow artifacts.
…mns readable The table sized its columns from their content, so a long hint under a target field or a long transform summary squeezed the source column to a few letters. The table now has fixed column widths; hints and transform summaries wrap inside their own cell, and the row buttons sit in a block inside the cell instead of turning the cell into a flex box. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The fixture copies match stackiq's packs again, which now start at 1.0.0, and the e2e spec and the change's examples expect a save to raise 1.0.0 to 1.0.1. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 25881/25881 · 25 skipped | ||||
| Newman | ✅ 635/635 | ||||
| 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-13 16:41 UTC
Download the full PDF report from the workflow artifacts.
… to 1.0.1 The fixture copy of stackiq's module pack now starts at 1.0.0, so a save raises it to 1.0.1. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 25881/25881 · 25 skipped | ||||
| Newman | ✅ 635/635 | ||||
| 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-13 17:56 UTC
Download the full PDF report from the workflow artifacts.
…fixtures' source correctly - f1: the stackiq fixtures, the e2e spec and the fixtures README no longer claim these are the packs stackiq ships. They are copies of the packs stackiq#1288 ships (1.0.x), taken at stackiq commit e2081bb; stackiq development still ships topdesk-module 2.0.0. The tests do not read stackiq. - f2: the fixed-layout mapping table sat in no overflow wrapper, so it widened past the viewport at narrow widths. It now has a min-width inside an overflow-x:auto region (role=region, labelled by the "Field mappings" heading, keyboard-focusable). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Verdict: APPROVE (Standard) — self-review posted as COMMENT (GitHub blocks self-APPROVE)
Nothing left to address in the delta: its 2 concerns were fixed on the branch before posting. The 3 minors listed as "Still open" in round 1 stay open, each with a reason below, and they do not block. What still stands between this PR and a merge: 1 approval, and stackiq#1288 for the fixture versions to match what stackiq ships.
| Round | Found | Status |
|---|---|---|
1 (7321876) |
36 findings fixed before posting · 3 🟢 still open | 3 🟢 left open, with reasons |
2 (1db4923) — this review |
2 🟡 (fixtures claimed stackiq ships 1.0.0; the mapping table overflowed at narrow widths) | fixed before posting in 1db4923 |
| Finding | Was | Now | Where |
|---|---|---|---|
| 🟡 Fixtures claimed stackiq ships 1.0.0 | new | fixed before posting in 1db4923 |
tests/fixtures/migration-packs/README.md, e2e docblock |
| 🟡 Fixed table layout overflowed at narrow widths | new | fixed before posting in 1db4923 |
src/views/migrationPack/MigrationPackDetail.vue |
Details of this round
Scope decision: delta 7321876…170f1cd (3 commits, 10 files, 199 changed lines: cdd560d, 9fee28c, 170f1cd), plus this run's fix 1db4923.
✅ Fixed before posting (2)
- Fixture provenance (
1db4923): the fixtures README, the e2e docblock and the assertion message now say these are copies of the packs stackiq#1288 ships (1.0.x, taken at stackiqe2081bb). stackiqdevelopmentstill ships topdesk-module 2.0.0, and the tests do not read stackiq. - Table overflow (
1db4923): the mapping table sits indiv.migration-pack__table-scrollwithoverflow-x: auto,role="region",aria-labelledbypointing at the existing "Field mappings" heading, andtabindex="0". The table hasmin-width: 720px. The rest of that file's diff is only prettier re-indentation (git diff -wshows the wrapper and the CSS).
Still open from round 1, not blocking
- Controller messages translated in nl only: a translation batch for every locale, which belongs in the
apply.jspipeline. The frontend shows its own translated text per error slug. testUpdateReadsTheRouteIdFromARealNextcloudRequestskips outside Nextcloud: by design it runs in the in-container step ofapi-test-coverage.yml.- l10n bundles are compact appends: re-running the writer re-serialises about 248k lines. That belongs in a separate reformat-only PR.
Gates: CI quality / Hydra Gates ✅ on 170f1cd. CI on 1db4923 started with this push.
Checks after the fix: prettier, eslint and stylelint clean; Jest src/views/migrationPack 109/109; test:l10n and test:l10n:parity OK. Playwright was not run, and the narrow-width layout was not checked in a browser.
Before the next round: fix rules on (fix-induced 2 + missed-sibling 0 ≥ new 0).
After posting: Frontend Check (format) went red on 1db4923, because the reworded e2e spec was not prettier-formatted. Fixed in 948520d (prettier --check on all files passes).
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ❌ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 25881/25881 · 25 skipped | ||||
| Newman | ✅ 635/635 | ||||
| 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-13 19:19 UTC
Download the full PDF report from the workflow artifacts.
The fixture-provenance rewording left the spec outside the repository's prettier style, which turned the format check red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ❌ 25965/25965 · 25 skipped (no test failed, the job failed for another reason — see its log) | ||||
| Newman | ✅ 635/635 | ||||
| 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-13 19:48 UTC
Download the full PDF report from the workflow artifacts.
…gration-pack-editor # Conflicts: # l10n/nl.json
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 26198/26198 · 25 skipped | ||||
| Newman | ✅ 714/714 | ||||
| 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-15 09:00 UTC
Download the full PDF report from the workflow artifacts.
What
OpenRegister gets a management UI for its migration packs. An admin can view and edit the column mappings an import runs, in OpenRegister itself. Decision (2026-10-12, Ruben): mappings are stored and managed in OpenRegister. Integriq's mapping UI is the UX template and stays untouched.
The first user is the stackiq TOPdesk CMDB import (WOO-588, Gemeente Rotterdam). Its packs are already migration packs run by
MappingEngine. The stackiq side is ConductionNL/stackiq#1288 / #1289.Changes
UI (Integration → Migration packs)
PackDefinitionValidatorallows. A lookup's default has three options: none, empty or a value. Yes/no tables store real booleans.x-…marker.permission.API
POST /api/migration-packsandPUT /api/migration-packs/{id}accept{"definition": {...}}. On update, the route id always wins. Before, a bodyid(the pack slug) replaced the route id and the update ended in a 404.A built-in pack's slug cannot change (422
builtin-slug-locked). This also holds through a delete followed by a re-create or import.An update based on an older read is refused with 409
pack-changed.New
POST /api/migration-packs/preview: takes{definition, rows}and returns the mapped rows and errors. It takes at most 20 rows and 1 MiB.The write routes (create, update, destroy, import and preview) are admin-only and require CSRF; reads are unchanged.
#[AuthorizedAdminSetting], which would let delegated admins in. The default middleware admits admins only, the methods are marked@auth admin-only, and the method body checks again.Create, update, import and destroy each log an info line with the acting user, the slug and the row id.
Every error uses one ADR-050 shape,
{message, error, details?}, and never carries exception text:not-authenticatedadmin-requiredpack-not-foundinvalid-request,too-many-rowspack-changedinvalid-pack(withdetails.errors),builtin-slug-lockedmigration-pack-error(logged)Spec
OpenSpec change
migration-pack-editor-ui, a delta onmigration-mapping-packs, with a MODIFIED block for the admin-gated CRUD requirement.openspec validate --strictreports it as valid. Every scenario is anchored to a running Playwright test.l10n
The frontend strings are translated in every locale. The bundles are appended in their current layout, not written by
scripts/l10n/apply.js. Ondevelopmentthe bundles are not in the writer's layout, so the writer re-serialises all of them, about 248k changed lines. That reformat belongs in its own PR. The backend messages are innl.jsononly, so other locales fall back to English for API messages. The frontend shows its own translated text per error code.Checks
At 7321876:
tests/Unit/Service/MigrationPack: green.testUpdateReadsTheRouteIdFromARealNextcloudRequestskips outside an installed Nextcloud.src/views/migrationPack: 109 of 109.test:l10nandtest:l10n:parity: OK.Hydra gates (local, diff scope): every applicable gate passes except gate-69 page-type-discipline. Custom pages went from 36 to 38. Both new pages carry a
_note: packs are not register objects, and@conduction/nextcloud-vue2.72.0 has no named entity source an app can add. Accepted in review.Rig (Nextcloud 35.0.1, OpenRegister development with this branch built; stackiq with #1288/#1289; fixtures only):
tests/e2e/ci/migration-pack-pages.spec.tspasses 38 of 38.Heads-up
Task 1.6 of the OpenSpec change
flow-parity-mapping-and-webhooksplans to delete Integriq'sMappingService. That conflicts with the decision to leave Integriq untouched for now.Jira: WOO-588. Plan: https://github.com/ConductionNL/readonly-mirror-wilco-claude-plans/tree/dev-mirror/issues/jira/WOO-586/sub-tasks/WOO-588/
🤖 Generated with Claude Code