fix: address the review findings from the beta promotion #1026 by WilcoLouwerse · Pull Request #1220 · ConductionNL/stackiq

WilcoLouwerse · 2026-10-02T14:02:27Z

Addresses the non-blocking review findings from the development → beta promotion #1026. The blockers already have their own issues (#1215, #1216, #1217) and are not part of this PR.

Fixed

Concern — A failed import stays running until the cache TTL expires: a throwing import is marked failed instead of staying running.
Concern — Ordinary set-up and import failures become bare 500s: set-up and import failures return a refusal instead of a bare 500.
Concern — No size cap, auto-detected reader, formulas evaluated: 10 MiB cap, xlsx reader only, data only, formulas never evaluated, stops at the row limit.
Concern — TOPdesk reference fields on module stay public: the external* module fields read for signed-in users only (schema 0.3.7).
Concern — Depublication fails open: versions read page by page; deleted modules take their versions out of view; failures logged critical.
Concern — Full unbounded scan on every upgrade: backfill pages through modules and runs once.
Concern — Raw /index.php/apps/... fetch without generateUrl(): register/schema from the app config, generateUrl + axios, error note.
Concern — Failed requests give no or misleading feedback: failed uploads and non-JSON responses give feedback.
Concern — Progress polling isn't stopped on unmount: polling stops on unmount.
Minor — Organisation stored as typed, not as the resolved uuid: the resolved organisation uuid is stored.
Minor — Cancel reports success for an import that already finished: cancel only applies to a running import.
Minor — A failed import is reported as completed at 100%: a failed ArchiMate import is reported as failed.
Minor — Progress requests can pile up: progress requests never overlap.

Partly fixed

Concern — Up to 500 saves run inline in the post-save request: unchanged publication no longer searches versions; moving the fan-out to a job is left open.

Also from the review summary: openapi.json now documents the four /api/itsm/* routes (status, config, setup, import).

Not changed: needs a decision

Concern — Progress and cancel stop working without a memcache: choice of fallback store without a memcache.
Concern — Any supplier can notify the owners of any product: who may notify whose product owners is an authorisation decision.
Concern — Mock register carries older, more public schemas (not verified end-to-end): regenerate the mock register or strip its schemas: a fleet decision.
Concern — Navigation done with window.location.assign instead of a link: the library renders headerActions only as NcActionButton (needs a nextcloud-vue change).

Verification

composer lint, plus phpcs, phpmd, phpstan and psalm on the changed files, are clean.
PHPUnit (no server): 1092 tests. The 20 errors are the same as on the base (missing Doctrine classes in the environment).
Jest: 101/101. ESLint and Prettier are clean on the changed files. test:l10n passes.
redocly lint on openapi.json reports no structural errors.

Refs: WOO-589

🤖 Generated with Claude Code

… not running or completed A CMDB import that throws outside a row (a progress write, an \Error) now marks its operation failed before the error propagates, and the controller answers IMPORT_FAILED for any Throwable instead of a bare 500. An ArchiMate import that fails is stored with status `failed` at the percentage it reached rather than `completed` at 100%. Both use a new ProgressTracker::failOperation(). A cancel of a CMDB import that is no longer running answers OPERATION_NOT_FOUND and leaves no cancel flag in the cache. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…nd stores the organisation's uuid A missing flow template or a preflight that throws no longer escapes as a bare 500: setUp() logs it and answers the `{created: false, message}` refusal the settings page renders, and integriq's connection report hears about it. The organisation the admin picks is looked up by id, uuid or slug; the set-up now writes the found object's uuid into the flows and the stored config, since the flows compare it with a usage's consumer uuid. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ops at the row limit An upload over 10 MiB is refused before it is read. An .xlsx file is always read with PhpSpreadsheet's Xlsx reader in data-only mode, and a formula gives the value Excel cached instead of being calculated. CSV and XLSX rows are read one by one and reading stops one row past MAX_ROWS. A file flow the engine refuses to run answers `{started: false, message}` instead of a bare 500. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…-in users only The CMDB import adds externalId, externalNumber, externalKey, externalCreatedAt and externalModifiedAt to `module`, and externalKey embeds the importing municipality's uuid. They now carry the same `authenticated` read rule as the other service desk references, so an anonymous reader of a published module no longer sees them. The module schema goes to 0.3.7 so installed registers pick the rules up. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…versions out of public view The mirror that copies an application's publication onto its versions failed open in several ways. It now: - reads a module's versions page by page instead of stopping at 500; - saves the mirrored fields without validation, so a version holding older data the schema no longer accepts still follows its module; - clears the versions of a deleted module (ObjectDeletedEvent); - logs at critical level when a write that would take a version out of public view fails, since that version stays readable anonymously. A module update that leaves publicationDate and registeredBy as they were no longer searches its versions, which keeps ordinary module saves from paying for the fan-out. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…dules and runs once The backfill read every module in one unbounded findAll() on each `occ upgrade`. It now reads modules in pages of 200 and, after a pass in which no version or search failed, sets the app-config flag `module_version_publication_backfilled` so later upgrades skip it. A pass with failures is not recorded and runs again on the next upgrade. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… showing nothing The exchange settings section loaded its organisations with a raw `/index.php/apps/...` fetch and hard-coded register and schema slugs, which breaks under a sub-path, and turned a failed call into an empty list. It now reads the configured register and schema from `/api/voorzieningen/config` and calls OpenRegister through `@nextcloud/axios` and `generateUrl()`, as the CMDB import does, and shows a note when the organisations cannot be loaded. The CMDB page catches a failed file upload or a non-JSON answer (a proxy error page) and says the file could not be sent, and shows a separate "could not be loaded" note when the exchange status call fails, instead of claiming no service desk is connected. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…erlaps The ArchiMate import section now stops its progress polling when it is unmounted mid-import, as the CMDB import section already does. The poller skips a tick while the previous progress request is still waiting, so a slow server no longer collects concurrent requests. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Adds GET /api/itsm/status, GET /api/itsm/config, POST /api/itsm/setup and POST /api/itsm/import, with who may call them, their request bodies, the success shapes and the `{created|started: false, message}` refusals the controller answers with. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ules The paging loops of the version mirror and its backfill move their per-page work into helpers and count each page before the loop condition. The file import checks the upload's size without the error control operator. ProgressTracker's public-method count, now eleven with failOperation(), carries a justified suppression like the other classes in this app. No behaviour changes. Refs: WOO-589 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

github-actions · 2026-10-02T14:08:57Z

Quality Report — ConductionNL/stackiq @ `9538a25`

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			✅	✅ 130/130
npm			✅	✅ 807/807
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 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-02 14:08 UTC

Download the full PDF report from the workflow artifacts.

github-actions · 2026-10-02T14:38:34Z

Quality Report — ConductionNL/stackiq @ `6afb1e1`

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			✅	✅ 130/130
npm			✅	✅ 807/807
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 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-02 14:38 UTC

Download the full PDF report from the workflow artifacts.

WilcoLouwerse and others added 10 commits October 2, 2026 15:10

Merge branch 'development' into fix/woo589-review-findings

fc586ee

WilcoLouwerse merged commit 4da250f into development Oct 2, 2026
49 checks passed

WilcoLouwerse deleted the fix/woo589-review-findings branch October 2, 2026 14:50

Navigation Menu

fix: address the review findings from the beta promotion #1026 - #1220

fix: address the review findings from the beta promotion #1026#1220
WilcoLouwerse merged 11 commits into
developmentfrom
fix/woo589-review-findings

WilcoLouwerse commented Oct 2, 2026

Uh oh!

github-actions Bot commented Oct 2, 2026

Uh oh!

github-actions Bot commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

WilcoLouwerse commented Oct 2, 2026

Fixed

Partly fixed

Not changed: needs a decision

Verification

Uh oh!

github-actions Bot commented Oct 2, 2026

Quality Report — ConductionNL/stackiq @ 9538a25

Uh oh!

github-actions Bot commented Oct 2, 2026

Quality Report — ConductionNL/stackiq @ 6afb1e1

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Quality Report — ConductionNL/stackiq @ `9538a25`

Quality Report — ConductionNL/stackiq @ `6afb1e1`