test(l10n): ratchet the untranslated schema strings - #859
Merged
Conversation
Every string inside a form comes from the OpenRegister schema, not from the manifest: `fieldsFromSchema()` runs a property `title` and `description` through the injected `cnTranslate`, which CnAppRoot binds to THIS app's id. So a schema title is a key in THIS catalogue — and when the key is absent, `t()` hands the source string back and the field renders in English inside an otherwise translated form. Nothing errors, and no existing check looks. Measured across the fleet on 2026-08-23: 30,459 schema strings had no catalogue key. Far too much to translate in one pass, and the descriptions need rewriting for the person filling in the form before translating them is even worth doing — humaniq's own pass rewrote 592 of 739 before a word was translated. So this is a RATCHET, not a gate: it records how many strings are currently uncovered and fails only when that number GROWS. The debt is measured and cannot expand, while burning it down stays an ordinary PR. Same shape as the JSDoc baseline in @conduction/nextcloud-vue. Counted: schema titles, property titles, property descriptions, and the VALUES of `x-enum-labels`. NOT counted: enum values themselves (stored contract values, several non-English by design, never rendered once a property declares its labels) and `x-notes` (engineering rationale, never rendered). Verified must-fail: adding one untranslated title takes the count past the baseline and exits 1, naming the file and property and the command that lists what is uncovered. Lower the baseline as strings get translated: npm run check:schema-l10n -- --update
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 23, 2026 20:40
…is repo Two things the fleet CI caught. `build-l10n-js.js` discovers locales by globbing `l10n/*.json`, which now also matches `l10n/.schema-l10n-baseline.json` — the ratchet's own state file, kept there so prettier ignores it. The generator read it as a locale named `.schema-l10n-baseline` and exited 1 for having no `translations`. Dotfiles are never locale catalogues, so it skips them. Also prettier-normalised both scripts to this repo's config; several apps run a format check over scripts/.
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ❌ | ||||
| check-l10n-js | ❌ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 538/538 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-23 21:02 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 538/538 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-23 22:10 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
pushed a commit
that referenced
this pull request
Aug 24, 2026
… baseline
`check:schema-l10n` was failing this PR, and it was failing on `development`
too: 1718 uncovered against a baseline of 1675, i.e. 43 over, before this branch
existed. A schema string with no catalogue key renders in English inside an
otherwise Dutch form — so this is not a bookkeeping gate, it is 43 labels a
Dutch user reads in the wrong language.
The previous commit translated the 15 strings this branch INTRODUCES, taking it
from 43 to 29. That made the branch better than its base and still red. This
closes the rest: 56 schema property titles — the strings that render as form
labels, so the highest value per string — now at 1648, under the 1675 baseline.
Safe to translate BECAUSE THEY ARE TITLES. A property title is a display string;
translating a stored value is what breaks colour maps and filters. Nothing here
touches an enum value, a slug, or anything a filter compares against.
Some were already Dutch in the schema ("Aanlever-deadline",
"Achterbanraadpleging", "Afdoening evidence"). Those get an identity entry in
en.json and a cleaned Dutch form in nl.json rather than being left uncovered —
uncovered is uncovered whatever language the source happens to be in.
BUILT, not just catalogued: `npm run l10n:build` regenerates l10n/*.js, which is
the artefact the browser actually loads. A check that reads the JSON cannot see
whether the .js was rebuilt, and shipping the one without the other is how 8,137
translations once reached nobody.
Verified: check:schema-l10n exits 0, test:l10n OK, check:l10n-js up to date.
⚠️ I misread this earlier and said it was another session's branch to fix. That
branch (#859) MERGED on 2026-08-23 — it introduced the baseline FILE mechanism
this now satisfies. The remaining gap was ordinary untranslated debt, not a
missing mechanism, and waiting for it would have waited forever.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 24, 2026
…ssible (#874) * feat(governance): make an Awb 7:13 objection advisory committee expressible A bezwaaradviescommissie is a governance body in everything but where it lives. decidiq already owns governance bodies — 17 seeded, `Membership` and `Post` for the roster, `bodyType: advisory-body` already in the enum — so another app keeping a parallel committee schema is duplication, not architecture. Four fields closed the measured gap. Each stands for a specific thing the existing model could not say, checked against the live schemas rather than assumed: GovernanceBody.active — whether the body may take new work. THE one field the consuming app's live code reads and throws on ("Committee is archived and cannot accept new bezwaaren"). Nothing here carried it; `x-openregister.active` is SCHEMA-level and says nothing about an individual body. GovernanceBody.quorum — how MANY members, as a number. `quorumRule` is a calculation method ("majority") and cannot express a statutory minimum. Both stay; each description now names the other. GovernanceBody.jurisdiction — territorial/subject-matter competence. GovernanceBody.statutoryBasis — the instrument the body sits under. Membership.external — sits from OUTSIDE the administrative organ, which Awb 7:13(2) requires of the chair. `independenceStatus` cannot carry it: that is corporate-governance independence, a different question with a different answer set. Each description now says what it is NOT, and a test asserts they do — two fields that read alike and mean different things is how the wrong one gets used. `quorum` has NO default, and a test asserts the ABSENCE. A default of 0 would read as "no members are required for a valid sitting" — a confident wrong answer where "not specified" is the truth. `statutoryBasis` is free text, not a BAC/VKK/VTH enum. A closed vocabulary drawn from one country's law cannot express the instruments behind bodies in another, and a caller forced to choose from a list that does not fit will choose a wrong value — which is worse, because it then reads as data. THE WRITE SEAM was the part no schema field fixes. decidiq's cross-app API was read-only, so no other app had a supported way to place a body here, and the alternative is reaching into this register directly — which ADR-022 and ADR-066 forbid. `governance-bodies` becomes the ONE writable resource, allowlisted in `RESOURCE_WRITABLE`, with tests proving meetings, decisions, votes, persons, minutes and motions all stay refused. The wildcard route is what makes that assertion necessary. AUTHORIZATION IS DELEGATED TO OPENREGISTER'S RBAC, and the obvious alternative was not available: `SCOPE_MAP` in this controller reads as an access control and enforces NOTHING — it is declared and referenced by no method, so no request has ever been checked against it. Specifying a `governance-bodies:write` scope would have named a gate that does not exist. It is now labelled as decorative so the next reader does not add an entry believing it will gate anything. TWO ROUTES, TWO NAMES, and that cost a real failure to learn. Registering `api#write` for both POST and PUT collides on the route IDENTIFIER, and the collision does not surface as a duplicate-route warning — it throws while the table is built and takes down EVERY route in the app. Measured on the running instance: the whole /api/v1 surface answered 500, including endpoints this change never touches, while the app still reported itself enabled. Verified fixed by re-testing that `POST /api/v1/decisions` still reaches the decision-hub handler rather than the generic writer. LIVE-VERIFIED end to end, not only in the suite: POST creates a committee (201) with all four fields round-tripping; PUT updates it (200); POST to a read-only resource answers 404 "Unknown resource"; and PUT REPLACES rather than patches — a partial body is refused naming the missing required properties, which is the safe direction but is now documented rather than discovered. Suite 1197 green, phpcs 0, phpmd 0, phpstan [OK]. Scope note: migrating another app's committees onto these bodies is that app's change, not this one. This makes the target exist. * feat(approval-routes): a reusable sign-off route, and an engine that advances it `DecisionStage` modelled the stages of a decision's route and NOTHING in this app ever wrote one. Measured: zero create/save/update of `decision-stage` across lib/, six seeded rows, two readers (EIDASSignatureService, DecisionIntegrationService), and a route tab whose own header declares "Posture: read-only". A schema without an engine is a description of a capability, not the capability. This adds the three things missing around it: ApprovalRoute the reusable TEMPLATE a route is instantiated from. `DecisionStage` is bound to a Decision, so there was nowhere to say "every collegeadvies travels these four steps". ProcessTemplate is not that: it is a state machine (states, transitions, guards), which answers what states a decision may be IN, not who signs and in what order. ApprovalAction the APPEND-ONLY trail. A stage is one mutable row; a route needs many actions against one step — an advice, a return, a re-submission, a delegate acting under mandate. The stages say where the route IS; the actions say what happened, including the attempts a return undid. ApprovalRouteService the engine, and the point of the change. DecisionStage grows additively: `stageType` += `endorsement` (parafering: a sign-off that is not itself the decision), `outcome` += approved/endorsed/ returned/skipped, and `mandatory`. `required` is unchanged, so no stored stage becomes invalid. `returned` behaves unlike every other outcome and that is why it had to be new: it RE-OPENS an earlier stage rather than ending this one. `rejected` and `deferred` both end a stage and neither reopens one, so no existing value could express "send it back to step 2". FAIL-CLOSED, deliberately and with tests on the refusals rather than only the happy path. An action by an actor the active stage does not name is refused; a skip of a mandatory stage is refused; a return pointing forwards is refused; an action on a finished route is refused. Each refusal test asserts that NOTHING changed — no action row, no stage move. An engine that records an action and does not move is the exact failure this replaces: the consuming app had one route API whose every call returned 400 and one approval bridge that short-circuited on a property nothing ever set, and both reported success. Two details that would each have produced a quietly broken engine: A `role`-typed step does NOT write its role token into `assignedPerson`. That field means "this person", and a role name there would make every actor check compare a uid against a role and refuse everyone. Asserted. A return CLEARS the outcome of the stages it resets. A stage that is pending again while still showing an outcome reads as decided to every consumer that looks at the outcome rather than the status. `mandatory` is COPIED onto the stage at instantiation rather than read from the route at decision time — otherwise editing a template would change the rules under a subject already travelling it. Same reason the steps themselves are copied. The controller takes the ACTOR FROM THE SESSION, never from the body. Reading it from the request would let any caller sign off as anyone, which is the one thing a sign-off route exists to prevent. Persistence is split into ApprovalRouteStore so the engine holds only the rules. That was prompted by phpmd (class complexity 58 vs a threshold of 55) and is the better shape regardless. l10n: the 15 schema strings this change introduces are translated in en/nl and BUILT into l10n/*.js — the artifact the browser actually loads, which the source-reading check cannot see.⚠️ `check:schema-l10n` is RED ON DEVELOPMENT ITSELF: 43 strings above the hardcoded baseline of 1675, before this branch existed. This change reduces it to 29 by translating its own strings properly. The rest is being fixed on `test/schema-l10n-ratchet`, which replaces the hardcoded number with a baseline file plus an --update mode; duplicating that here would conflict with it. Suite 1212 green, phpcs 0, phpmd 0, phpstan [OK]. The UI write affordances on DecisionRouteTab are deliberately deferred — the tab is read-only today, and giving it verbs is a separate change now that the engine it would drive exists and its shape is settled. * fix(filinq): PDF delegation named a namespace the document app no longer uses `RegulatorExportRenderer` probes for a document-app PDF service and falls back when none is found. Both candidates named `OCA\Docudesk\Service\*`; the document app renamed to `OCA\Filinq` with no compatibility alias, so every candidate missed and the renderer silently took its fallback path. The pattern here was already right — a candidate LIST rather than a pinned class — which is why this was a quiet degradation rather than a crash. It just listed one app's old name twice. A list is only resilient if it spans the rename. Measured on a running instance: OCA\Docudesk\Service\PdfRenderService MISSING OCA\Docudesk\Service\PdfService MISSING OCA\Filinq\Service\PdfService EXISTS OCA\Filinq\Service\PdfConversionService EXISTS Note `PdfRenderService` has no Filinq counterpart at all — the class list moved as well as the namespace, so a mechanical namespace swap would have produced two candidates of which one still could not resolve. The Docudesk entries stay until no supported install ships them; an unresolvable candidate costs one class_exists() call. Found by a fleet sweep after the same defect was confirmed in dossiq; openregister (→ Keepiq) and softwarecatalog (→ Decidiq) carried it too. * i18n(schema): translate 56 form labels, taking the ratchet back under baseline `check:schema-l10n` was failing this PR, and it was failing on `development` too: 1718 uncovered against a baseline of 1675, i.e. 43 over, before this branch existed. A schema string with no catalogue key renders in English inside an otherwise Dutch form — so this is not a bookkeeping gate, it is 43 labels a Dutch user reads in the wrong language. The previous commit translated the 15 strings this branch INTRODUCES, taking it from 43 to 29. That made the branch better than its base and still red. This closes the rest: 56 schema property titles — the strings that render as form labels, so the highest value per string — now at 1648, under the 1675 baseline. Safe to translate BECAUSE THEY ARE TITLES. A property title is a display string; translating a stored value is what breaks colour maps and filters. Nothing here touches an enum value, a slug, or anything a filter compares against. Some were already Dutch in the schema ("Aanlever-deadline", "Achterbanraadpleging", "Afdoening evidence"). Those get an identity entry in en.json and a cleaned Dutch form in nl.json rather than being left uncovered — uncovered is uncovered whatever language the source happens to be in. BUILT, not just catalogued: `npm run l10n:build` regenerates l10n/*.js, which is the artefact the browser actually loads. A check that reads the JSON cannot see whether the .js was rebuilt, and shipping the one without the other is how 8,137 translations once reached nobody. Verified: check:schema-l10n exits 0, test:l10n OK, check:l10n-js up to date.⚠️ I misread this earlier and said it was another session's branch to fix. That branch (#859) MERGED on 2026-08-23 — it introduced the baseline FILE mechanism this now satisfies. The remaining gap was ordinary untranslated debt, not a missing mechanism, and waiting for it would have waited forever. * fix(gates): a real authorisation guard, typed OR injection, a registered icon Four Hydra gates failed on this branch. All four were mine and all four were right; one of them was a genuine security hole. GATE-7 (no-admin-idor) — `ApprovalRouteController::instantiate` carried `#[NoAdminRequired]` and no authorisation. It had an authentication preamble, which the gate explicitly does not count and should not: "is anyone logged in" was already settled by the attribute. The actual effect was that ANY signed-in user could start a sign-off route against ANY subject, writing DecisionStage rows onto somebody else's object. Fixed with `assertSubjectAccessible()`, which asks the question that matters — can this caller reach this subject — and delegates the answer to OpenRegister, reading as the acting user so OR's register RBAC and multitenancy decide. A user who cannot reach the subject gets nothing back, and nothing back is a refusal. `subjectSchema` becomes required, because the check cannot be performed without it. The gate no longer lists this method. GATE-66 (openregister-dependency-shape) — `ApprovalRouteStore` fetched ObjectService from the container by string. ADR-083 rule 1: a dependency resolved that way is declared nowhere a reader or a gate can see it. Now injected as a typed `ObjectServiceInterface`, which `Application::register()` already aliases — an ALIAS, so it still resolves lazily and an instance without OpenRegister still boots. That change made a second thing true: `findAll()` is typed `: array`, so the `is_array()` guard below it became unreachable. Removed rather than kept — phpstan proved it dead, and it was caution against the untyped lookup that no longer exists. GATE-60 (icon-vocabulary) — `GestureTapButton` is not in the canonical vocabulary, and ADR-077 rule 3 means it renders with NO icon at all, not a fallback. Swapped for `ClipboardCheckOutline`, which is registered and reads correctly for a recorded sign-off. GATE-16 (spec-coverage) — `ApprovalRouteStore::save()` and `::findAll()` are new public methods without `@spec`. Added. TESTS follow the real contract now, and that mattered twice. `ObjectServiceInterface::saveObject()`'s SECOND parameter is `?array $extend`, not the register, and it returns an ENTITY rather than an array. A callback shaped like the call site (which uses named arguments and skips `$extend`) received the wrong values in the wrong order. The stateful fake is now reached through a typed mock that mirrors the real signature — the type satisfies the constructor and the state survives, which is what lets these tests assert the route ADVANCING rather than merely that a call was made. Suite 1212 green, phpcs 0, phpmd 0, phpstan [OK], psalm clean, l10n ratchet PASS. Still failing locally and NOT mine: gate-7 lists 3 VotingController methods (cast/proxy/revokeProxy) — pre-existing, and CI counts diff-scoped so they did not block this PR. They are real findings and are being taken up separately. * fix(migration): the template migration ran as Anonymous and migrated nothing Found by running `occ upgrade` on a live instance, not by any test. `MigrateLegacyTemplatesToDecisionTemplate` calls `saveObject()` with no system identity. A migration executes during an upgrade, where there is no session — so OpenRegister sees the actor as 'Anonymous' and refuses `create` on DecisionTemplate. Every one of the 14 legacy templates failed: Repair warning: Failed to migrate process-template 20ac9a95-…: User 'Anonymous' does not have permission to 'create' objects in schema 'DecisionTemplate' (× 8 process) Repair warning: Failed to migrate vve-decision-template e2e93cb6-…: …same… (× 6 vve) AND THE UPGRADE STILL SAID "Update successful". Each failure was reported with `$output->warning()`, which does not fail an upgrade, and the step's own summary line then read "0 migrated, 14 skipped" — a sentence that is true, undramatic, and scrolls past. The legacy templates are still there and the unified ones were never written, on every instance that has run this. Wrapped in `runAsSystem()`. The wrap is around the WHOLE traversal rather than each save, so the index build happens inside the same identity scope too; that meant lifting the body into `migrateAll()`, which is the only reason `run()` changed shape. THE TEST FAKE NEEDED THE METHOD, and that is the interesting half. The suite was green throughout — 16 tests asserting the mapping in detail — because its anonymous ObjectService fake simply had no `runAsSystem()`, and nothing required it to. A fake that does not model the contract cannot fail when production violates it: the wrapper could be deleted tomorrow and these tests would still pass while every real upgrade migrated nothing. The fake now implements it, so removing the wrap breaks the suite. 16/16 green, suite 1219 green, phpcs 0, phpstan [OK]. Pre-existing debt, found while getting this branch's own CI green. Not related to approval routes; fixed here because an upgrade that silently migrates nothing is worse than one that fails loudly. --------- Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
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.
The gap
Every string inside a form comes from the OpenRegister schema, not the manifest.
fieldsFromSchema()runs a propertytitleanddescriptionthrough the injectedcnTranslate, whichCnAppRootbinds to this app's id — so a schema title is a key in this catalogue. When the key is absent,t()hands the source string back and the field renders in English inside an otherwise translated form. Nothing errors, and no existing check looks at this surface.Measured across the fleet on 2026-08-23:
Why a ratchet and not a gate
30,459 is far too much to translate in one pass — and the descriptions need rewriting for the person filling in the form before translating them is even worth doing. humaniq's own pass rewrote 592 of 739 descriptions (median length 108 → 76 characters) before a single word was translated; translating them as they stood would have cemented developer-facing copy in two languages.
So this records the current uncovered count and fails only when it grows. The debt is measured and cannot expand, while burning it down stays an ordinary PR. Same shape as the JSDoc baseline in
@conduction/nextcloud-vue.What counts
Counted: schema
title, propertytitle, propertydescription, and the VALUES ofx-enum-labels.Not counted: enum values themselves — they are stored contract values, several non-English by design (
ingediend), and are never rendered once a property declares its labels — andx-notes, which holds engineering rationale that is never rendered.Verification
Must-fail control — inject one untranslated title:
Lower the baseline as strings get translated:
npm run check:schema-l10n -- --update