feat(settings): every connection on one page, honestly - #1939
Merged
Conversation
Dossiq talks, or claims to talk, to ten systems outside it, and until now the only place any of them appeared was a 12,371-character admin page with no navigation and no answer to the one question an admin has: does this work. This is the noun that question needs. The `dossiqIntegration` schema carries a connection's key, title, status, status message, checked-at and the settings section that configures it. The status enum has four values and that is the whole vocabulary: Configured, Not configured, Not available, Error. The seed is where the honesty lives. Per M3-integrations.md the dossiq column reads `partial, stub` for eight of the ten and `spec only` for BRP and KvK, so BRP and KvK are seeded Not available with "Specified, not built yet" and carry no settings link, and NO row is seeded Configured. Only a save or a probe may write that. A page that starts by claiming eight working connections would be worse than no page. PDOK is seeded with an empty settings link too, and that one is a finding rather than a decision: the design counted eight admin sections to anchor and there are seven. PDOK's `pdok_*` keys are read by the map components and written by nothing an admin can open. Linking to a section that does not exist is precisely the class of quiet untruth this page exists to remove, so the link is absent and the seed says why. The register version moves to 1.2.0 and the config to 0.15.0 so the import does not fast-skip the new schema.
The page is an index over the config schema, admin only, in the settings
foldout. It shows the connection, its status, what the last probe or save
said, when that was, and a link to the section that configures it.
IT IS NOT THE `type: "settings"` PAGE THE DESIGN ASKED FOR, and the reason
is worth keeping rather than rediscovering. In the installed
@conduction/nextcloud-vue a settings section's widgets resolve only against
`version-info`, `register-mapping` and the `component` discriminator, while
`object-list` is registered with `registerDashboardWidget()`; CnSettingsPage
takes no page-level widgets prop either. The widget would have rendered an
empty section behind a single console warn. Task 2.2 named this outcome in
advance and licensed the index page; a custom page was never an option.
The cost is REQ-ADMIN-021, the Required apps section, which stays red.
Open settings is a COLUMN, not a row action, and that is the second thing
the tree teaches. A row action cannot carry a per-row URL: `type:
"navigate"` takes one literal target and pushes it through vue-router,
which cannot reach /settings/admin/dossiq at all. Nor can it carry a
per-row visibility: CnRowActions' `visible` predicate is a function, and
JSON cannot hold one. The built-in `link` widget substitutes {settingsUrl}
from the row and falls through to plain text when that is empty, which is
exactly what the spec asks of the BRP and KvK rows: the affordance absent,
not disabled.
AdminRoot.vue gains the seven anchors those links point at. The design's
eighth, PDOK, has no section to anchor.
The vitest spec guards the failures that are silent in a browser: an icon
outside src/icons.js renders nothing, a menu id outside settingsSection
lands in the main nav where every user sees it, and a settings link to a
missing anchor scrolls nowhere.
A page that only ever shows its seed is worse than no page, because it looks like an answer. IntegrationStatusService is the one write path: it refuses a key the seed does not ship and a status outside the four, sends the WHOLE row back because saveObject is PUT-semantic and a partial write would blank the title the page renders, and swallows its own failures. That last one matters: it runs beside a connection test whose answer is what the admin actually asked for, and a page that cannot be updated must not turn a working probe into a 500. The StUF endpoint list and the mailbox test now record what they found. A save of a section without a probe records Configured when its required keys are filled and Not configured when they are cleared, and ONLY for the connections whose own keys the payload carried, so saving the KCC form never restates the ZGW card. WHERE THAT WRITE IS CALLED FROM IS NOT WHERE THE DESIGN PUT IT. EmailTemplateController, SettingsController and StoreController each sit at PHPMD's CouplingBetweenObjects ceiling of 13. Injecting the recorder turned all three red, measured, and all three were clean before. So each records through a collaborator it already holds: EmailTemplateService for the mailbox, ConfiguredRegistryService for the Store — which is the better home anyway, since "is a registry configured" is that service's own question. SettingsController has neither and resolves the recorder from the container BY NAME; even a `catch (\Throwable)` there is a type reference that pushes the count back over, so the guard is has(). A string FQCN is exactly the kind of reference that rots into a silent no-op after a rename, so a test asserts it resolves to the real class. SettingsService was the obvious seam and could not take it: it is at all three of PHPMD's ceilings at once — 999 lines, complexity 49, coupling 12.
Every scenario the delta spec names, asserted on ids, hrefs and the objects the API holds rather than on English labels — nothing forces the language of the E2E instance, and five specs went red on a Dutch one before. The non-admin scenario needs a user who is not an admin, so ci-seed.sh creates one. Asserting the guard from the admin session would have been no test at all: the admin sees everything, so it would pass on an app with no permission checks whatsoever. The request context sends its credentials with send:'always' because dossiq answers 401 without a WWW-Authenticate header, and Playwright would otherwise never send them. The Required apps scenario is test.fixme naming its reason: REQ-ADMIN-021 needs CnLeafDependencySettings on a settings page, the library cannot host the connections there, and an index page has nowhere to put the section. The KCC scenario writes app config that outlives the run, so a last test puts the row back where the "claims nothing it has not checked" assertion needs it on a reused instance.
ADR-111 rule 1 wants three demo objects per schema, and gate-101 found the new one with zero. It failed the run; the earlier run that exited 0 was baseless and had judged nothing. The generator was NOT used to fix it. Both `--objects` and `--keep` rewrote the whole descriptor — 22,025 deletions on a --keep run — because development's mock register is ten schemas behind the real ones and the regeneration would have swept in everyone else's drift under this change's name. The three rows are inserted instead, at the file's own indent so the diff is 132 lines and nothing else moves, and `--check` validates them against the schema that will hold them. They are demo data and they read like it, but the STATES are chosen: one Not configured, one Error, one Not available, so a demo shows the three things a card can honestly say without any of them claiming a working connection.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 8, 2026 13:32
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 541/541 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-08 13:37 UTC
Download the full PDF report from the workflow artifacts.
…ons-page # Conflicts: # l10n/en.js # l10n/en.json # l10n/nl.js # l10n/nl.json
…t moved Two things the merge of development brought, both of which fail loudly and neither of which is about this change's behaviour. `manifestCaseHeader.spec.js` (case-identity, merged an hour ago) pins the manifest at 44 pages so that a subtitle change cannot quietly move anything else. Integrations makes it 45. The number is raised by exactly the page that was added rather than loosened to a range, because the pin is the assertion's whole value. And `l10n/en.js` / `nl.js` were stale against their JSON: the built browser catalogue is generated, and a generated blob committed on a branch goes stale the moment the base adds a key. CI caught it before I did, on the merge commit rather than on the commit I tested — which is the point of a `pull_request` check testing your branch merged INTO the base.
Six PHPUnit cells were red on this PR and the summary line said OK. The exit code was 1: `failOnRisky` plus `beStrictAboutCoverageMetadata`, and `IntegrationProbesRecordTest` executed `StufServices` without declaring it. WHY IT HAS TO BE REAL. `StufServices` is an immutable bundle of eight collaborators with public readonly promoted properties. `StufControllerContractTest` can mock it because its tests never read one; mine calls `endpoints()`, which reads `->register` and `->circuitBreaker`, and a mock leaves a readonly promoted property uninitialised and throws before the assertion. So it is built, and now named in `@uses` — the same dialect #1943 used on the case-type tests an hour ago. CI NAMED TWO OF THE THREE TESTS THAT BUILD IT, so the fix is the class-level declaration rather than the two tests it happened to name: an attribution that differs between runs is not a list to work through. WHAT I COULD AND COULD NOT VERIFY LOCALLY, because it is the reason this reached CI at all. This box has no pcov or xdebug and no sudo to install one, and `--no-coverage` disables the coverage-metadata check outright — so every local run I made was structurally incapable of seeing this, and printed OK with exit 0. What IS verified locally is the half that has bitten before: PHPUnit's own metadata parser resolves the annotation to '\\OCA\\Dossiq\\Service\\Stuf\\StufServices' rather than to an empty `@uses`, and that class exists. The rest is CI's to answer.
…t/integrations-page
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 541/541 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-08 14:26 UTC
Download the full PDF report from the workflow artifacts.
…ons-page # Conflicts: # l10n/en.js # l10n/en.json # l10n/nl.js # l10n/nl.json
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 541/541 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-08 14:35 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
1 participant
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.
Implements the merged OpenSpec change
pluggable-integration-registry(round 2, row A34), which extendsadmin-settings.Dossiq talks, or claims to talk, to ten systems outside it. The only place any of them appeared was
/settings/admin/dossiq: seventeen sections in 12,371 characters, no navigation, and no answer to the question an admin actually has. This adds one page under the gear that lists every connection with a status the app can back, and a link to the section that configures it.What the page says, and why that is the point
Per
M3-integrations.mdthe dossiq column readspartial, stubfor eight of the ten andspec onlyfor BRP and KvK. So the seed says exactly that:Not availablewith "Specified, not built yet", and offer no settings link.Not configuredwith "Not checked yet".Configured. Only a save or a probe may write that.The StUF endpoint health and the mailbox Test connection now write their outcome. A save of a section without a probe records Configured when its required keys are filled and Not configured when they are cleared, and only for the connections whose own keys the payload carried, so saving the KCC form never restates the ZGW card.
Three places the tree disagreed with the design, and what shipped instead
1. It is a
type: "index"page, nottype: "settings". In the installed@conduction/nextcloud-vuea settings section'swidgets[]resolves only againstBUILTIN_SETTINGS_WIDGETS(version-info,register-mapping) and thecomponentdiscriminator;object-listis registered withregisterDashboardWidget().CnSettingsPagetakes no page-levelwidgets[]prop either. The widget would have rendered an empty section behind one console warn. Task 2.2 named this outcome in advance and licensed the index page; acustompage was never an option (ADR-100 ratchet).The cost is REQ-ADMIN-021, the Required apps section, which stays RED. It needs
CnLeafDependencySettingsthrough atype: "component"widget, and an index page has nowhere to mount it. Its e2e scenario istest.fixmenaming this reason. Nothing else about the page has to change when a settings section can host a list.2. Open settings is a column, not a row action. A row action cannot carry a per-row URL —
type: "navigate"takes one literaltargetand pushes it through vue-router, which cannot reach/settings/admin/dossiqat all — nor a per-row visibility, sinceCnRowActions.visibleis a function and the action schema isadditionalProperties: false. The built-inlinkcell widget substitutes{settingsUrl}from the row and falls through to plain text when it is empty, which is exactly what REQ-ADMIN-019 asks of the BRP and KvK rows: the affordance absent, not disabled.3. Seven anchors, not the design's eight, and PDOK has no settings link. There is no PDOK section on the admin page: its
pdok_*keys are read by the map components and written by nothing an admin can open. Linking to#section-pdokwould have been the exact class of quiet untruth this page exists to remove. The design also reports two duplicated headings; everyCnSettingsSectionname inAdminRoot.vueis in fact unique, so nothing was deduplicated, and a unit test now pins the ids unique.Where the recording is called from
Not the constructors the design assumed.
EmailTemplateController,SettingsControllerandStoreControllereach sit at PHPMD'sCouplingBetweenObjectsceiling of 13; injecting the recorder turned all three red, measured, and all three were clean before. Each now records through a collaborator it already holds —EmailTemplateService::recordMailboxStatus()andConfiguredRegistryService::recordStoreStatus(), which is the better home anyway since "is a registry configured" is that service's own question.SettingsControllerhas neither and resolves the recorder from the container by name; even acatch (\Throwable)there is a type reference that pushed the count back over, so the guard ishas(), and a unit test asserts the string FQCN resolves to the real class so it cannot rot into a silent no-op.SettingsServicewas the obvious seam and could not take it: it is at all three PHPMD ceilings at once (999 lines, complexity 49, coupling 12).Verification, by exit code
eslint 0 · prettier 0 · vitest 0 (743) ·
check:manifest0 ·check-l10n0 ·check:schema-l10n0 (at baseline) ·check:l10n-js0 · phpcs 0 · phpmd 0 per directory plus the unused-params leg · psalm 0 · phpstan 0 · phpunit 0 (3236) · hydra gates with--base origin/development.composer check:strictwas not the command run: it exceeds the 300s tool timeout, and phpmd printing nothing inside it is its OOM signature rather than a pass, so the legs were run individually. Playwright was not run —tests/e2e/integrations-page.spec.tsis new and unexecuted here, and E2E runs on thedevelopmentpush, not on this PR.gate-101 (demo-data-coverage) failed the first real run: the new schema had zero demo objects. Fixed by hand rather than by the generator, which rewrote 22,025 lines even under
--keepbecause development's mock register is ten schemas behind the real ones. The three rows are validated by the generator's own--check.🤖 Generated with Claude Code