Skip to content

fix(integrations): the Woo redaction and beschikking template seams now reach filinq - #2059

Merged
rubenvdlinde merged 4 commits into
developmentfrom
fix/connect-leaf-adapters
Sep 9, 2026
Merged

fix(integrations): the Woo redaction and beschikking template seams now reach filinq#2059
rubenvdlinde merged 4 commits into
developmentfrom
fix/connect-leaf-adapters

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What this fixes

Two of dossiq's filinq integrations were built on both sides and never connected. Both reported success while doing nothing, which is what made them urgent: from outside they were indistinguishable from working channels.

1. Woo redaction made no call at all

WOORedactionService::queueViaDocuDesk() looped the documents, wrote status: 'queued' on each one, logged that each had been queued, and returned. The comment where the call belonged said the API call was "deferred to DocuDeskService" and that we would "let Docudesk poll". Filinq ships no ingestion that polls for such intents.

So the branch taken when filinq is installed redacted nothing, and the manual fallback, taken only when filinq is absent, was the app's only working redaction path. The app was also failing its own spec: woo-case-type's "Redaction via Docudesk integration" requirement already said the document MUST be sent to the anonymisation pipeline.

Each document now goes to filinq's AnonymizationService through a new FilinqRedactionClient: extract entities, then anonymise, as the acting user, on the file id this app resolved.

Three outcomes replace the one fictional status:

  • filinq redacted it, with the entity count and the anonymised file id
  • filinq could not be given the document (no file id, no id/fileName pair) so it falls to manual carrying the reason
  • filinq ran and detected nothing, which also falls to manual. Filinq still writes an output file in that case, but its content is the input's. A document a human assessed as deels openbaar has something to remove, so zero detections means the detector answered nothing rather than that the document is clean. This is not hypothetical: it is what the dev instance does today, because no detection backend is configured there.

Nothing is called queued any more, because nothing queues.

2. The beschikking template seam had nowhere real to point

The substitution plumbing already worked and already told the admin to name a real adapter in beschikking_template_adapter. Dossiq shipped only MockTemplateEngineAdapter, which hashed its own arguments into a fileId and a checksum and reported a constant four pages.

FilinqTemplateEngineAdapter renders through filinq's DocumentService and returns metadata about the file filinq actually wrote: its Nextcloud file id, a SHA-256 over the produced bytes, and the page count read out of those bytes. Both fallback messages and the Integrations page now name that class instead of asking for "a real adapter class" that did not exist.

Absence is an error inside the adapter, not a fallback. The choice between filinq and the mock stays one level up in SubstitutableAdapterRegistrar, where it is logged and shown on the Integrations page.

This is not #2061's repoint. That PR fixes BeschikkingGenerationService (the DSO bijlage, a direct container-get with a text-stub fallback). This is BeschikkingService::compose() going through the substitutable seam. Checked at #2061's head: zero file overlap, and MockTemplateEngineAdapter is still the only TemplateEngineAdapterInterface implementation there.

The rename trap

Both cross-app lookups go through FleetAppId, which resolves the app id and the PHP namespace across the docudesk-to-filinq rename. Either stale half returns null without erroring, which is exactly the silent no-op this change exists to remove.

What was observed on the leaf's side

Run against the live dev instance, not only in unit tests.

  • All three filinq services resolve in process from the dossiq namespace: OCA\Filinq\Service\DocumentService, TemplateService, AnonymizationService.
  • Template render: filinq wrote a 28,284-byte PDF to /admin/files/DocuDesk/filinq/beschikking.pdf, file id 1633. The bytes hash to 831884e6…, which is exactly the checksumSha256 the composition reports, and the page count reads 1. The mock, for the same call, answered fileId: doc-53df963a0b0d, a checksum over its own arguments, and 4 pages.
  • The bound seam: with beschikking_template_adapter set, ConfiguredAdapter binds FilinqTemplateEngineAdapter and the render produces a real file; unset, it binds the mock. The config key was restored to absent afterwards.
  • Woo redaction: filinq accepted the hand-off and wrote /admin/files/woo-redaction-probe2_anonymized.pdf, 29,136 bytes. It detected zero entities, so the result correctly reports awaiting_manual_redaction with reason: filinq_detected_no_entities rather than claiming a redaction. The document with no resolvable file reports file_unresolved. Nothing reports queued.
  • Every constructor dependency of both new classes resolves from the real dossiq app container.

All probe artifacts were deleted from the instance afterwards.

The tests assert the call, not the return

The test this replaces asserted status === 'queued' and passed against a method that made no call. It would have passed for as long as the defect existed.

Each new guard was watched failing before being kept:

Mutation Reddens
restore the redaction no-op the recorded call list, empty instead of two entries
hash the arguments mock-style in the renderer fileId is the mock's hash, not filinq's stored id
point the resolver at the stale docudesk id filinq_unavailable, on the call path
skip filinq's entity extraction the ordered call list loses its first entry
drop the class name from the Integrations row the row no longer names a class an admin can paste
lowercase the unregistered-job marker the timed-job registration invariant

Each file was restored from a byte copy afterwards and diffed against the backup.

The doubles declare filinq's real signatures, checked parameter by parameter against filinq's source. That is still only a claim about a double, so it is not what this rests on: the live runs above exercised the actual leaf.

The coverage ratchet was right

The first CI run failed one PHPUnit leg on the coverage ratchet, at 83.65% against a 93.38% base. It was not noise and it was not a deletion penalty: resolveVersion(), a whole public method that reaches filinq's TemplateService, had no test behind it at all. The version a beschikking records could have come back wrong without anything noticing, and the mock it replaces answered v1 for every template on every date, which is what made an untested version function easy to miss.

Worth saying plainly, because the reflex when a ratchet reddens is to argue with the ratchet. This one found a real gap and the fix was to write the tests it asked for.

Berichtenbox is not touched, and here is why

Integriq ships only BerichtenboxClientMock. The live BerichtenboxClientHttp that its own docblock names does not exist in the repo, and its absence waits on Logius BBK 1.7 OAuth credentials and a PKIoverheid Services-server certificate, which is a procurement item rather than a coding one. dossiq-delivers-nothing already records this as delivery-inventory item 5, staged to phase 4: "Nothing to move today — there is no transport."

Integriq does ship an ADR-041 command contract, DeliveryRequestedEvent, and dossiq already consumes it in PublicationService — so this is not blocked on a missing route. But it carries no read-status counterpart for BerichtenboxAdapterInterface::getReadStatus(), and behind it is the same mock. Wiring dossiq to it would move the mock one app to the left, flip the Integrations row from Simulated to Configured, and still deliver nothing to Mijn Overheid. That is the regression this whole line of work exists to prevent, so the row stays Simulated and honest.

Three dead crons found on the way out, one documented and two reported

A TimedJob runs only when appinfo/info.xml names it. BerichtenboxReadStatusJob is not named there, so it has never run, and nothing said so — the same ambiguity as everything else here, since an unregistered job and a job that runs daily and finds nothing leave identical evidence.

It stays unscheduled, because polling BerichtenboxClientMock every 24 hours would write back a read status the mock invents an hour after send. A cron that appears to confirm citizens are reading their post, while no post has left the instance, is worse than no cron. The class now says that, and says register-it-with-the-transport.

Looking for that marker found two more. Of the five jobs absent from info.xml, DeadlineNotificationDispatchJob and StufRetryJob are queued in code and are fine. AppointmentReminderJob and ShareMaintenanceJob are timed jobs that are neither registered nor referenced anywhere in lib/ — so no appointment reminder has ever been sent, and share maintenance has never run. Both are recorded in TimedJobRegistrationTest's UNEXPLAINED map rather than fixed here: registering a job is a behaviour change this branch cannot verify, and inventing a reason for someone else's decision would be the same lie in a new place. These two need an owner.

The new test demands only that an unregistered timed job say why, never that it be registered, so adding a <job> entry satisfies it with no test edit.

Verification

  • phpunit -c phpunit-unit.xml: 3499 tests, 19548 assertions, exit 0 (read from the exit code, not the summary line)
  • PHPCS, PHPMD (both rulesets), Psalm, PHPStan on every changed file: exit 0
  • hydra gates: 79 of 80 applicable ran. gate-27 no-phantom-cross-app-rpc PASS, gate-16 spec-coverage PASS, gate-1/2/3 PASS. The three failures are not from this diff: two are Ajv missing in the local clone, one is the pre-existing 946-request Newman collection.

…ow reach filinq

Two of dossiq's filinq integrations were built on both sides and never
connected, and both reported success while doing nothing.

WOO REDACTION MADE NO CALL AT ALL. `WOORedactionService::queueViaDocuDesk()`
looped the documents, wrote `status: 'queued'` on each, logged that each had
been queued, and returned. The comment where the call belonged said the API
call was "deferred to DocuDeskService" and that we would "let Docudesk poll",
and filinq ships no ingestion that polls for such intents. So an instance WITH
filinq installed took that branch, reported every document queued, and redacted
none of them; the manual branch, taken only when filinq is ABSENT, was the
app's only working redaction path. The app's own spec already required the
call: woo-case-type REQ "Redaction via Docudesk integration" says the document
MUST be sent to the anonymisation pipeline.

Each document now goes to filinq's `AnonymizationService` through the new
`FilinqRedactionClient`: extract entities, then anonymise, as the acting user,
on the file id this app resolved. Nothing is called `queued` any more, because
nothing queues. A document filinq cannot be given falls to manual redaction
carrying the reason. So does a run where filinq detected NO entities: it still
writes an output file, but that file's content is the input's, and a document a
human assessed as deels openbaar has something to remove, so zero detections
means the detector answered nothing rather than that the document is clean.
That last case is not hypothetical: it is what the dev instance does today.

THE BESCHIKKING TEMPLATE SEAM HAD NOWHERE REAL TO POINT. The substitution
plumbing already existed and already told the admin to name a real adapter in
`beschikking_template_adapter`, but dossiq shipped only
`MockTemplateEngineAdapter`, which hashed its own arguments into a fileId and a
checksum and reported a constant four pages. `FilinqTemplateEngineAdapter`
renders through filinq's `DocumentService` and returns metadata about the file
filinq actually wrote: its Nextcloud file id, a SHA-256 over the produced
bytes, and the page count read out of those bytes. Both fallback messages and
the Integrations page now name that class instead of asking for "a real adapter
class" that did not exist.

Both cross-app lookups go through `FleetAppId`, which resolves the id AND the
namespace across the docudesk-to-filinq rename; either stale half returns null
without erroring, which is the failure this change exists to remove.

Verified against the running instance, not only in unit tests: filinq rendered
a 28 KB PDF to /admin/files/DocuDesk/filinq/beschikking.pdf whose bytes hash to
the checksum the composition reports, and filinq wrote
woo-redaction-probe2_anonymized.pdf for the redaction hand-off. With
`beschikking_template_adapter` set, `ConfiguredAdapter` binds the filinq
adapter; unset, it binds the mock.

The tests assert the CALL, not the return. The test they replace asserted
`status === 'queued'` and passed against a method that made no call, which is
why it never caught this. Each new guard was watched failing: restoring the
no-op, hashing the arguments mock-style, pointing the resolver at the stale
`docudesk` id, and skipping filinq's extraction each redden the assertion that
names them.

Berichtenbox is NOT touched. Integriq ships only `BerichtenboxClientMock`; the
live `BerichtenboxClientHttp` its own docblock names does not exist, pending
Logius BBK 1.7 credentials and a PKIoverheid certificate. Connecting to it
would move the mock one app to the left and flip the Integrations row from
Simulated to Configured while nothing still reaches Mijn Overheid.
The change that carried woo-case-type's tasks.md was archived, so
`openspec/changes/woo-case-type/tasks.md#task-8` names a path that is not
there. Thirty-six references across lib/ still carry it and are left alone
here, but the new hand-off must not add a thirty-seventh: it cites
`openspec/specs/woo-case-type/spec.md`, which exists and holds the
"Redaction via Docudesk integration" requirement this code now satisfies.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ b75b9a0

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 08:42 UTC

Download the full PDF report from the workflow artifacts.

The ratchet failed the change at 83.65% against a 93.38% base, and it was
right: eleven statements of new integration code had no test behind them, and
one of them was a whole public method. `resolveVersion()` reaches filinq's
TemplateService and nothing exercised it, so the version a beschikking records
could have come back wrong without a single test noticing. The mock it replaces
answered `v1` for every template on every date, which is what made an untested
version function easy to miss.

Now covered, and each assertion is on the call or on filinq's own value rather
than on a shape:

- resolveVersion reads filinq's version, refuses an unknown template, and
  refuses when filinq is absent
- render refuses without an acting user, sends no data reference when there is
  no case, reports zero pages for a non-PDF, and reports a filinq failure as a
  failure
- the redaction client accepts a numeric-string file id, refuses when the file
  lookup throws, runs without a session recording no acting user, and reports a
  throwing filinq as a refusal
- a document record with no identifier is skipped rather than sent
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ 418cf5c

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 08:53 UTC

Download the full PDF report from the workflow artifacts.

…scheduled, and find the ones nobody said it about

A `TimedJob` runs only when `appinfo/info.xml` names it. `BerichtenboxReadStatusJob`
is not named there, so it has never run on any instance, and nothing said so.
That is the ambiguity this branch has been removing all along: an unregistered
job and a job that runs daily and finds nothing leave identical evidence, an
empty log and a status that never changes, so the only way to tell them apart
was to open info.xml.

It stays unscheduled, because scheduling it today would poll integriq's
`BerichtenboxClientMock` every 24 hours and write back a read status the mock
invents an hour after send. A cron that appears to confirm citizens are reading
their post, while no post has left the instance, is worse than no cron. The
class now says that, and says register-it-with-the-transport.

LOOKING FOR THE MARKER FOUND TWO MORE. Five background jobs are absent from
info.xml; two of them, `DeadlineNotificationDispatchJob` and `StufRetryJob`, are
queued in code and are fine. The other three are timed jobs, and besides the
Berichtenbox poller they are `AppointmentReminderJob` and `ShareMaintenanceJob`
— neither registered nor referenced anywhere in lib/, so no appointment reminder
has ever been sent and share maintenance has never run. Both are recorded in
the new test's UNEXPLAINED map rather than fixed here: registering a job is a
behaviour change I cannot verify from this branch, and inventing a reason for
someone else's decision would be the same lie in a new place. The map entry
says plainly that nobody has established why.

The test demands that an unregistered timed job SAY SO, never that it be
registered, so adding a `<job>` entry satisfies it with no test edit and the day
a real transport lands nothing here is in the way.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ 59eaa65

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:21 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

E2E: no new failure against the development baseline

The bar here is not green — development itself is red — it is no new failure against that baseline. Measured on both sides rather than asserted.

development (run 34328242034) this PR (run 34334195655)
passed 300 300
failed 7 7
skipped 42 42
did not run 32 32
duration 26.6m 28.8m

Playwright actually started in both runs (Running N tests using matched once each), so neither tally is the empty output of a job that died before the suite — a red run that never started Playwright prints no at all, which greps as "no failures".

The failing sets are identical, compared with comm rather than by eye:

tests/e2e/case-actions-menu.spec.ts:321:6
tests/e2e/case-header.spec.ts:297:6
tests/e2e/case-identity.spec.ts:314:6
tests/e2e/case-timeline.spec.ts:98:6
tests/e2e/case-type-authoring-extras.spec.ts:494:6
tests/e2e/case-type-authoring-extras.spec.ts:645:6
tests/e2e/contacts-domain.spec.ts:362:6

Nothing new, and nothing fixed — this branch is backend-only and changes no default binding, so that is the expected result.

Two of those seven, case-identity:314 and case-timeline:98, are the subject of a separate open PR. If that lands first the baseline becomes 5, and this run should then be re-compared against the tree it actually ran on rather than against this list.

Every other check on this PR passes, including all six PHPUnit legs.

@rubenvdlinde
rubenvdlinde merged commit 404d40b into development Sep 9, 2026
47 of 49 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/connect-leaf-adapters branch September 9, 2026 09:58
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ b1e47b8

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:58 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Addendum: at job level this branch fails a strict subset of what development fails

failing jobs
development (34328242034) E2E Tests (Playwright), Hydra Gates, Quality Report
this PR (34334195655) E2E Tests (Playwright), Quality Report

Quality Report is the aggregate roll-up, not an independent finding — its log reads PLAYWRIGHT_RESULT: failure and Playwright: ran, result=failure, so it is downstream of the same seven pre-existing E2E failures. Worth noting it distinguishes "ran and failed" from "never ran" and recorded that Playwright ran, which matches the tally measured above.

development is currently failing Hydra Gates and this branch passes it. So this is not merely no-new-failure; it is one fewer failing job than the base.

(The two gate failures I saw locally, gate-22 manifest-validation and gate-53 effective-manifest-crossref, were my clone missing node_modules so Ajv could not resolve. CI has it, and CI's Hydra Gates passes here — which is the check that settles it, not my local run.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant