Skip to content

fix(register): two dangling objectDescriptionField values detached the whole voorzieningen register — E2E has run 0 tests since 08-14 - #529

Merged
rubenvdlinde merged 1 commit into
developmentfrom
S20/e2e-register-config-field-resolution
Aug 16, 2026
Merged

fix(register): two dangling objectDescriptionField values detached the whole voorzieningen register — E2E has run 0 tests since 08-14#529
rubenvdlinde merged 1 commit into
developmentfrom
S20/e2e-register-config-field-resolution

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

The E2E job has run zero Playwright tests since 2026-08-14 19:29Z

Not "some specs fail". 0 passed / 0 failed / 0 skipped — the job dies in
the seed step, before Playwright is invoked, on softwarecatalog's own gate in
tests/e2e/ci-seed.sh:

##[error]softwarecatalog has no register/schema mapping for:
  ['organisatie_schema', 'contactpersoon_schema', 'module_schema', 'contract_schema']
##[error]OpenRegister may be seeded, but the app cannot resolve it — every e2e
          fixture would fail with "voorzieningen register not configured".

That gate is right, and it is the reason this was diagnosable at all. The
register genuinely is unusable.

The chain, read from the failing job's own Nextcloud log

Run 31937311499 (0335cb20, push), step "Show Nextcloud log":

  1. Two schemas are rejected outright.
    view and bioMeasure declare "objectDescriptionField": "summary" while
    neither declares a summary property. OpenRegister's
    SchemaMapper::validateConfigField() throws on exactly that, and
    ImportHandler logs at ERROR:

    [ImportHandler] Failed to import schema: The value for objectDescriptionField ('summary') does not exist as a property in the schema.

  2. The register is then imported with almost no schema links.
    18 warnings, one per reference:

    [ImportHandler] Schema with slug <X> not found in schemasMap during register import. … This register will be created without this schema reference.

    The 18 are every schema of voorzieningen and vng-gemma except
    moduleVersion and sbomComponent — the only two declared after the last
    failing schema in file order. That is the fingerprint, and it is what
    promotes "two bad schemas" into "the register is empty" (see the note to
    OpenRegister at the bottom).

  3. configureVoorzieningen() writes an empty map. It builds the
    app-config from $targetRegister['schemas']; with the links gone it writes
    register and nothing else:

    [ci-seed] voorzieningen config: {"register": "14", "organisatie_schema": "", "contactpersoon_schema": "", … }
    
  4. ci-seed.sh refuses to continue rather than hand Playwright a
    half-seeded instance. Correct call — the alternative is ~20 spec failures
    blaming the fixtures.

Where the two values came from

Commit 386771dc (#513, "translate 12 pre-existing Dutch property names"),
merged 2026-08-14 21:29 CEST:

schema before #513 after #513 effect
view property key summary; objectDescriptionField: "summary" property key renamed to omschrijving; config value left at "summary" dangling
bioMeasure property key omschrijving; objectDescriptionField: "omschrijving" key unchanged; config value rewritten to "summary" dangling

Two dangling references, produced in opposite directions by the same commit —
a key rename that did not follow its reference, and a value rename that should
never have touched a reference at all.

Bisected against the runs adjacent to that merge, not last-green-vs-later-red:

run sha time (UTC) voorzieningen config seed
31830739382 f4e57ebb 08-14 18:53 organisatie_schema: "22", … all populated exit 0
31833457673 386771dc 08-14 19:29 all "" exit 1
… every run since all "" exit 1

Corroborating detail from the same two logs: OpenRegister's schema count drops
37 → 35, and the two that vanish are exactly view and bioMaatregel.

The change

Point each objectDescriptionField at the property its schema actually
declares — viewomschrijving (its description property, title
"summary"), bioMeasureomschrijving (restoring the pre-#513 value).

Nothing is renamed here. #513's summary → omschrijving key rename is a
backwards translation applied to 43 sites across this file; undoing it is a
data migration and belongs with the vocabulary programme, not with an E2E fix.
Flagged on the fleet board.

Evidence — before/after, same instrument

OpenRegister's own three acceptance forms (Twig template / pipe fallback list /
plain name) applied to the shipped register file:

schemas fields checked failures
before 20 38 2view.objectDescriptionField, bioMeasure.objectDescriptionField
after 20 38 0

tests/Unit/Settings/RegisterConfigFieldResolutionTest.php makes that a
standing check. Run in a throwaway php:8.3-cli container (host is PHP 8.2):

# against the unfixed JSON — the positive control for the whole PR
Tests: 4, Assertions: 21, Failures: 1
  - view.objectDescriptionField: 'summary' does not exist as a property
  - bioMeasure.objectDescriptionField: 'summary' does not exist as a property

# after the fix
OK (4 tests, 21 assertions)

phpcs --standard=phpcs.xml tests/Unit/Settings/RegisterConfigFieldResolutionTest.php
  exit 0 — 1 file measured

The test carries its own positive control (testTheResolutionCheckCanFail)
asserting each of the three failure shapes is detected and each of the three
accepted shapes is not, plus a measured-field floor so a broken traversal
cannot report a silent zero.

It also records the two objectSummaryField values that dangle today
(element, relation). OpenRegister does not validate that key, so they are
inert — asserted as a known set rather than as zero, so adding a new one fails
the build while the existing debt stays visible and counted.

What this PR does NOT claim

It does not claim the E2E suite passes. It claims the suite will run — the
seed gate is what has been blocking it. The specs' first real verdict in two
days arrives with this PR's own CI, and any failures it exposes are pre-existing
and get their own work.

Parity

Baseline for judging this PR: 31937311499 (0335cb20, push, conclusion: failure) — 13 failing jobs.
The 4 PHP Quality (phpcs/phpmd/psalm/phpstan) + 6 PHPUnit failures there are
the refactor/adr-084-type-hint-the-contract regression (#519), owned by slot
R1, and are present on development without this branch. This PR contains no
lib/ PHP. Its own tests/bootstrap.php cannot even load locally for the same
reason (Interface "OCA\OpenRegister\Contract\ObjectEntityInterface" not found),
which is why the test above was run with --bootstrap vendor/autoload.php.

🔴 A note for OpenRegister — one bad schema detaches an entire register

ImportHandler::importFromJson() (two-pass schema import) does:

$savedSchemasMap = $this->schemasMap;
$this->schemasMap = [];              // temporarily empty, to block $ref resolution
$schema = $this->importSchema(...);  // ← throws
$this->schemasMap = $savedSchemasMap;// ← never reached

The catch logs and continues but does not restore the map, so every schema
imported before the failing one is erased. That is why two rejected schemas
produced 18 missing register links rather than 2, and it will do the same to any
app whose configuration file has a single bad schema. Not fixed here —
OpenRegister is another slot's repo and mid-wave — but it is the amplifier, and
it is worth a three-line fix.

…e whole voorzieningen register

The E2E (Playwright) job on `development` has not run a single test since
2026-08-14 19:29Z. It fails in the seed step, before Playwright starts, on
softwarecatalog's own honest gate:

    softwarecatalog has no register/schema mapping for:
      ['organisatie_schema', 'contactpersoon_schema', 'module_schema', 'contract_schema']

That gate is correct and the register really is unusable. The chain, read
from the failing job's own Nextcloud log (run 31937311499):

1. `view` and `bioMeasure` declare `objectDescriptionField: "summary"` while
   neither schema has a `summary` property. OpenRegister's
   `SchemaMapper::validateConfigField()` throws for exactly this, and
   `ImportHandler` logs
   `Failed to import schema: The value for objectDescriptionField ('summary')
   does not exist as a property in the schema.`
2. Both schemas are therefore absent from the import's `schemasMap`, so the
   `voorzieningen` and `vng-gemma` registers are imported without them —
   OpenRegister logs 18 `not found in schemasMap` warnings.
3. `SettingsService::configureVoorzieningen()` iterates the register's
   schemas to build the app-config map. With the links gone it writes
   `register` and nothing else, leaving every `*_schema` key empty.
4. `tests/e2e/ci-seed.sh` refuses to run Playwright against that. Correctly —
   the alternative is ~20 spec failures blaming the fixtures.

Where the two values came from: commit 386771d (#513, "translate 12
pre-existing Dutch property names") renamed the `view` schema's `summary`
property KEY to `omschrijving` and left `objectDescriptionField` pointing at
the old key, and separately rewrote `bioMeasure`'s `objectDescriptionField`
VALUE from `omschrijving` to `summary` while its property key stayed
`omschrijving`. Two dangling references, opposite directions, one commit.

This points both at the property each schema actually declares. It does not
rename anything: a property rename here is a data migration and belongs with
the vocabulary programme, not with an E2E fix.

Evidence, same instrument both sides — OpenRegister's own three acceptance
forms applied to the shipped register file, 20 schemas / 38 configuration
fields measured:

  before: 2 failures (view.objectDescriptionField, bioMeasure.objectDescriptionField)
  after:  0 failures

The new test reproduces that measurement in PHPUnit and carries a positive
control asserting the check can fail, so a future rename cannot silently
detach the register again. Run in a php:8.3-cli container (the host is 8.2):

  before fix: Tests: 4, Assertions: 21, Failures: 1  (naming both schemas)
  after fix:  OK (4 tests, 21 assertions)
  phpcs --standard=phpcs.xml on the new file: exit 0, 1 file measured

The test also records the two `objectSummaryField` values that dangle today
(`element`, `relation`). OpenRegister does not validate that key, so they are
inert — asserted as a known set rather than zero, so adding a new one fails
while the existing debt stays visible.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

First honest E2E tally in two days — and it caught a live production fatal

The seed fix works. ci-seed now prints app-level voorzieningen mapping OKregisters + schemas provisioneddone, and Playwright executed for the first time since 2026-08-14 19:29Z.

Run 31940827485, job 95151081157:

last honest tally (08-14 14:00) now
passed 94 40
failed 0 47
skipped 4 4
did not run 0 7
total slots 98 98

⚠️ 7 did not run, so 47 is a floor, not a total. Same 98 slots on both sides, so the suite did not grow — but the two tallies are not fully comparable and I am not claiming otherwise.

The 47 decompose exactly, and 42 are one server-side fatal

42  Error: softwarecatalog 5xx responses:   (expectNoAppErrors, _helpers.ts:80)
 4  expect(locator).toBeVisible() failed
 1  expect(received).toMatch(expected)
--
47

The only 5xx URL in the entire run — 176 occurrences — is 500 /index.php/apps/softwarecatalog/api/me. The Nextcloud log dump in the same job names the cause:

Too few arguments to function OCA\SoftwareCatalog\Controller\ContactpersonenController::__construct(),
11 passed in .../lib/AppInfo/Application.php on line 730 and exactly 13 expected

Application.php:726 says so in its own comment — "Register ContactpersonenController with explicit dependencies for /me endpoint" — and the factory passes 11 named arguments to a constructor that now takes 13. That is the refactor/adr-084-type-hint-the-contract composition-root defect (#519), and it is not a lint finding: /api/me 500s for every user on development right now, and the SPA calls it on every page load.

Two failures were stacked, and the outer one hid the inner one

The seed has been broken since 08-14, so the E2E job ran zero tests. When ADR-084 shipped a runtime fatal at 08:46 today, E2E could not see it — the column read "E2E red" identically before and after, while the meaning changed completely.

A job that is red for reason A cannot report reason B. Unblocking a broken harness is what makes the next regression visible.

Parity — this PR is not the cause of any of it

  • Baseline 31937311499 (0335cb20, push): 13 not-success.
  • This PR: the same 4 PHP Quality + 6 PHPUnit (ADR-084) plus E2E.
  • This PR contains no lib/ PHP — two changed characters in a JSON file, plus one new test file. It cannot construct a controller. It removed a blocker; the 47 are what the blocker was hiding.

E2E should go green on this branch once the ContactpersonenController registration is fixed: 42 of 47 die on that single 500, and the remaining 5 are a small, separately-diagnosable tail.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ b7084d6

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue-demi
test-l10n
format
composer ✅ 130/130
npm ✅ 704/704
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-16 11:06 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit ece273a into development Aug 16, 2026
31 of 44 checks passed
@rubenvdlinde
rubenvdlinde deleted the S20/e2e-register-config-field-resolution branch August 16, 2026 13:17
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