fix(integrations): stop naming apps that were renamed out from under us - #2061
Merged
Merged
Conversation
Three of dossiq's cross-app bindings still named an app by an id or a
namespace nothing answers to any more. None of them errored: every one of
these lookups is duck-typed, so a stale name returns false or 404 and the
integration quietly does nothing.
PdokService gated on isInstalled('openconnector') and read
pdok.feature_flag out of the openconnector app-config namespace. Integriq
is that app renamed, and IAppConfig keys on the app id, so the flag read
permanently off and the guard reported "not installed" about an app that
was. Both now resolve through FleetAppId.
The same method then called linkToRoute('openconnector.pdok.parcel').
That route does not exist under either name: integriq publishes four PDOK
routes and parcel is not one of them. The stale guard is the only thing
that kept the crash off, so correcting the id alone would have turned a
quiet empty list into an uncaught RouteNotFoundException. searchParcel
now says the endpoint is missing instead of calling one that is not there.
src/services/pdokService.js hit /apps/openconnector/api/pdok from the
browser and 404d on every current instance, which the shim reads as
"integriq absent" and turns into an empty result, so the address field
just stopped suggesting. It resolves the app segment from OC.appswebroots
now, the browser-side equivalent of FleetAppId. It also asked for
/lookup?id=, and integriq mounts that as /lookup/{id}, so the id moves
into the path.
BeschikkingGenerationService asked the container for
OCA\Docudesk\Service\DocumentService and caught the miss, so since the
filinq rename every beschikking silently became a text stub. The name is
resolved now, and the call moves to generateDocument(), which is what
filinq publishes; generateFromTemplate() never existed there, so fixing
only the namespace would have left the same stub behind a green diff.
The tests could not have caught any of this. PdokServiceTest mocked
isInstalled to answer true for 'openconnector' and mocked linkToRoute to
return a URL. Every BeschikkingGenerationService case made the container
throw, so the suite never reached the generation call. Both now assert
against the names and contracts the other apps really ship, and both
were mutation-checked.
Also corrects admin- and user-facing copy that still names openconnector,
docudesk or decidesk, and two settings links pointing at docs.procest.nl,
which is not where these docs are published.
rubenvdlinde
requested review from
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 9, 2026 09:11
…used Three CI cells, two causes, and both were invisible to a local run. PHPUnit went red in all six matrix cells with `OK, but there were issues!` and exit 1: five risky tests, each reporting that it executed OCA\Dossiq\Support\FleetAppId, which neither test class declared. That is `beStrictAboutCoverageMetadata` plus `failOnRisky`, and it only fires when a coverage driver is loaded. CI loads one and a local `composer test:all` does not, so the check was silently absent locally and the local exit 0 said nothing about it. The remedy and its trap are already documented in WOORedactionServiceTest, which hit exactly this in #1863; both test classes now carry the same class-level declaration and the same warning against ever writing the tag name with an at-sign in the prose. The l10n pair is the fourteen rewritten strings. A source string IS its translation key, so renaming the app inside one orphans the old key and leaves the new one with no entry. Rather than extract the new keys as English-to-English and abandon eleven human translations, the keys are renamed in place across all 38 catalogues and each locale's own translation carries over with the product name swapped. The diff is 233 added and 233 removed lines per side, which is what a pure rename looks like: no reserialisation, no reordering. The browser catalogues under l10n/*.js are rebuilt from them; `check:l10n-js` was green on the last push only because the JSON had not moved yet, so that was a fourth cell waiting to turn red. Prettier wanted two wrapped lines in pdokService.js. Run from the repo root, since the config is declared in package.json rather than a .prettierrc and a run outside the root silently loses it and fails every file instead of one. Also drops the em-dashes from the lines this branch adds, per the voice rules, and capitalises the app names consistently inside a sentence: "the Integriq connection ... from Decidiq" rather than one of each.
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 | ✅ | ✅ 549/549 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-09-09 09:33 UTC
Download the full PDF report from the workflow artifacts.
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 | ✅ | ✅ 549/549 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-09 10:08 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every DSO beschikking has been a text placeholder, not a PDF, since the docudesk rename
This is the finding to act on, so it goes first.
BeschikkingGenerationServiceasked the DI container forOCA\Docudesk\Service\DocumentService. Filinq is docudesk renamed, and the rename moved the PHP namespace along with the app id, so no instance has registered that class for months. The lookup threw, the throw was caught, a debug line nobody reads was written, and the method fell through tocreateStubBijlage(), which attaches a text placeholder and returns'success' => true. The controller returns 201. Nothing anywhere reports a problem.So every permit decision generated through the DSO flow since the rename has been a stub rather than the rendered PDF, and it has looked like a success to the caseworker, to the API and to the logs.
Fixing the namespace alone would not have fixed it.
generateFromTemplate(), the method this service called, has never existed on filinq'sDocumentServiceeither. It would have thrown into the same catch and produced the same stub, behind a diff that read as a repair. The call moves to the publishedgenerateDocument(templateId, dataRefs, options), andattachBijlageToZaak()now reads theoutput.fileIdandoutput.namethat filinq actually returns instead of top-level keys that were never in that array. Generating into Files needs an owner, so the service takesIUserSession; without a session it falls back to the stub and says so.What this PR fixes
The fleet renamed its apps (
openconnectortointegriq,docudesktofilinq,decidesktodecidiq,hrmqtohumaniq), the directory names did not move, and several of dossiq's bindings still named an app by an id or a namespace nothing answers to.None of them errored. Every cross-app reference here is a duck-typed runtime lookup: a stale id makes
isInstalled()answer false, a stale namespace makesclass_exists()answer false andcontainer->get()throw, and each one sits behind a guard that degrades gracefully. The integrations did not break loudly, they went dark.The other live defects
PdokServicegated onisInstalled('openconnector')and readpdok.feature_flagout of theopenconnectorapp-config namespace.IAppConfigkeys on the app id, and integriq's ownMigrateAppConfigKeysrepair step copies that key tointegriq, so the flag read permanently off andgetServiceStatus()reported "not installed" about an app that was. Both resolve through the existingFleetAppIdhelper now.The same method then called
linkToRoute('openconnector.pdok.parcel'). That route does not exist under either name. Integriq publishes exactly four PDOK routes:The stale guard is the only thing keeping the crash off: on a current instance the old id resolves false, the method returns early, and nobody reaches the route. Correcting the id alone would have turned a quiet empty list into an uncaught
RouteNotFoundException, while the diff read as a fix.searchParcel()reports the gap now instead of calling an endpoint that is not there. Integriq does shipSources\Pdok\PdokWfsSourceAdapter, which can querykadastralekaart:perceel, but it is not exposed over HTTP; closing the gap properly means integriq publishing a parcel route.src/services/pdokService.jshit/apps/openconnector/api/pdokfrom the browser and 404d on every current instance. The shim reads a 404 as "the app is absent" and hands the caller an empty result, so address autocomplete simply stopped suggesting and nothing reported an error. It resolves the app segment fromOC.appswebrootsnow, the browser-side equivalent ofFleetAppId. It also asked for/lookup?id=, where integriq mounts/lookup/{id}, so the id moves into the path.Why the tests did not catch any of it
PdokServiceTestmockedisInstalledto answer true for'openconnector'and mockedlinkToRouteto return a URL, so the suite was green against a name no instance answers to and a route that does not exist. EveryBeschikkingGenerationServiceTestcase madecontainer->get()throw, so no test ever reached the generation call.Both now assert against the names and contracts the other apps really ship, and each new assertion was mutation-checked: reverting the canonical name, reverting the method name, pinning the app-config read to a literal id, trimming the JS candidate list to one name, and moving the lookup id back to a query parameter each redden the intended assertion, and all restore green.
Also in this PR
Admin- and user-facing copy that still named
openconnector,DocudeskorDecidesk(settings labels, DigiD and eHerkenning activation hints, the BRP, KvK, ZGW and ZTC adapternotevalues, two dialogs), and two settings links pointing atdocs.procest.nl, which is not where these docs are published;docusaurus.config.jsdeclaresprocest.conduction.nland 41 other links insrc/agree.Renaming a user-facing string renames its translation key, so all 38 l10n catalogues are updated. The keys are renamed in place, and each locale's own human translation carries over with the product name swapped, rather than extracting the new keys as English-to-English and abandoning eleven real translations. The diff is 233 added and 233 removed lines on each side, which is what a pure rename looks like: no reserialisation, no reordering.
l10n/*.jsis rebuilt from the JSON.Deliberately not changed
procestas dossiq's OWN legacy id: migration repair steps, group names,X-Procest-Signature, theSOURCE_APPliteral decidiq matches on. Frozen by design.OCA\Decidesk/OCA\Decidiqevent names inWorkflowListenerRegistrar,ContractDecisionDelegationServiceandCommitteeDelegationService. Already correct, both spellings listed newest first.mandaat_decidesk_connectionand itsdecidesk-defaultvalue, and thedecidesk-decisionsleaf id. Stored configuration, not lookups.Verification
lint,phpcs,phpmd,psalm,phpstanandtest:all(3478 tests) each exit 0, read as$?rather than off the summary line. Full vitest 1082 passing.test:l10n,check:l10n-js,check:schema-l10n,check:manifest,eslintandprettier --checkall exit 0.run-hydra-gates.sh: all 76 applicable gates green.One check cannot run locally and CI is the instrument for it. PHPUnit's
beStrictAboutCoverageMetadataplusfailOnRiskyonly fires when a coverage driver is loaded. This machine has neither xdebug nor pcov and cannot install one, so the first push went red in all six matrix cells withOK, but there were issues!and exit 1 while the local run was honestly green. The five risky tests each namedOCA\Dossiq\Support\FleetAppIdas executed but undeclared; both test classes now declare it, following the precedent and the warning already written intoWOORedactionServiceTestfrom when this happened in #1863. Please read the PHPUnit cells on this push rather than my local result for that one.