Skip to content

fix(samenwerkverzoek): honour the ObjectServiceInterface contract - #855

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/or-contract-saveobject
Aug 15, 2026
Merged

fix(samenwerkverzoek): honour the ObjectServiceInterface contract#855
rubenvdlinde merged 2 commits into
developmentfrom
fix/or-contract-saveobject

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

All six PHPUnit matrix cells are red on development. Two separate defects against OpenRegister's published ObjectServiceInterface — and the first is a production bug, not a test artefact.

1. A TypeError on every call (production)

initiateSamenwerking() and respondToSamenwerking() are both declared : array, but ObjectServiceInterface::saveObject() returns an ObjectEntityInterface. Returning it straight out of a method declared : array is a TypeError on every invocation — including from DsoController::respondSamenwerking(). That is why it surfaced as an ERROR rather than a failed assertion.

Both call sites now normalise through an asArray() helper, mirroring the helper of the same name already present in CaseCollaborationService, and staying defensive so either shape works.

2. A test pinned to the old argument order

testInitiateSamenwerkingCreatesObject expected saveObject('procest-register', …) — the old ($register, $schema, $object) order. The contract takes $object first, and the caller uses named arguments, so the double receives ($object, $extend, $register, $schema).

The sibling test at line 231 had already been corrected and carried a comment saying exactly this; this one was missed. It now also asserts the register and schema it was silently ignoring.

Verification

Run in a PHP 8.4 container with OpenRegister's real Contract interfaces preloaded. That detail matters: a plain standalone run cannot resolve OCA\OpenRegister\Contract\ObjectServiceInterface and errors identically on both sides, so it cannot discriminate. With them preloaded the environment reproduces CI's numbers exactly.

tree tests errors failures risky
pristine development 1931 2 1 1
this branch 1931 0 0 0

Exactly the three SamenwerkverzoekServiceTest cases move to passing. Nothing introduced.

All six PHPUnit matrix cells are red on `development`. Two separate
defects against OpenRegister's published ObjectServiceInterface, and the
first one is a production bug, not a test artefact.

1. `initiateSamenwerking()` and `respondToSamenwerking()` are both
   declared `: array`, but `saveObject()` returns an
   `ObjectEntityInterface`. Returning it straight out is a TypeError on
   every call — including from DsoController::respondSamenwerking(),
   which is why it surfaced as an ERROR rather than a failed assertion.
   Both now normalise through an `asArray()` helper, mirroring the
   helper of the same name already in CaseCollaborationService, and
   staying defensive so either shape works.

2. `testInitiateSamenwerkingCreatesObject` pinned the OLD argument order
   ($register, $schema, $object). The contract takes $object FIRST, and
   the caller uses named arguments, so the double receives
   ($object, $extend, $register, $schema). The sibling test at line 231
   had already been corrected and carried a comment saying exactly this
   — this one was missed. It now also asserts the register and schema it
   was silently ignoring.

Verified in a PHP 8.4 container with OpenRegister's real Contract
interfaces preloaded, which reproduces CI exactly (a plain standalone
run cannot resolve them and errors identically on both sides, so it
cannot discriminate):

  pristine development   1931 tests, 2 errors, 1 failure, 1 risky
  this branch            1931 tests, 0 errors, 0 failures

Exactly the three SamenwerkverzoekServiceTest cases move to passing, and
nothing is introduced.
The helper I added took `mixed $value`, and that pushed
SamenwerkverzoekService's CouplingBetweenObjects from 12 to 13, which is
the hydra-gates threshold. phpmd is GREEN on this branch's merge-base
(34a35dc) and red on the branch, so it was mine.

`ObjectServiceInterface::saveObject()` returns an `ObjectEntityInterface`,
which extends `JsonSerializable`, so the defensive is_array() branch was
never reachable. Calling `->jsonSerialize()` at the two sites is both
simpler and adds no type reference at all.

Same behaviour: full suite still 1931 tests, 0 errors, 0 failures.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Update — phpmd was mine, and it is fixed.

The first push turned quality / PHP Quality (phpmd) red. That check is green on this branch's merge-base (34a35dce), and the merge-base is the exact commit the last completed development run measured — so this was not pre-existing debt, it was my change.

Cause: the asArray(mixed $value) helper I added pushed SamenwerkverzoekService's CouplingBetweenObjects from 12 to 13, the hydra-gates threshold. The mixed type hint is counted as a class reference.

Fix: drop the helper. ObjectServiceInterface::saveObject() returns an ObjectEntityInterface, which extends JsonSerializable, so the defensive is_array() branch was never reachable->jsonSerialize() at the two call sites is simpler and adds no type reference at all.

Behaviour is unchanged: the full suite is still 1931 tests, 0 errors, 0 failures (baseline development is 2 errors, 1 failure, 1 risky).

⚠️ Note for anyone reproducing locally: the copied vendor/ lacks conduction/hydra-gates, so phpmd cannot be run against that ruleset locally — it reports "Cannot find specified rule-set" rather than a clean result.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ d86236c

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

Quality workflow — 2026-08-15 23:18 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit de0c975 into development Aug 15, 2026
41 of 42 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/or-contract-saveobject branch August 15, 2026 23:57
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ 5e925c1

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

Quality workflow — 2026-08-16 00:08 UTC

Download the full PDF report from the workflow artifacts.

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