fix(merge): organisation merge re-points nothing — probe a magic accessor with property_exists

[rubenvdlinde]

Closes #490.

The defect

MergeOrganisatieService::repointBySelfOrganisation() decided whether an object was owned by the source organisation with method_exists($entity, 'getOrganisation').

OpenRegister's ObjectEntity declares that accessor only as an @method docblock tag over protected ?string $organisation (ObjectEntity.php:89 / :260), so it is served by OCP\AppFramework\Db\Entity::__call(). The probe is therefore always false, $owningOrganisation stayed null, and the following line continued every object.

Consequence: contract and compliancy were never re-pointed, while tombstoneSource() still retired the source organisation. A merge reported success having moved nothing, leaving live objects owned by an organisation that no longer exists. The dry-run/execute parity check could not catch it — both arms were equally broken, so they agreed.

Why the suite was green

tests/Stubs/Db/ObjectEntity.php declared getOrganisation() concretely, so method_exists was true in the suite and false in production. The double inverted the exact predicate under test.

That is now fixed at the source: the stub extends the real OCP\AppFramework\Db\Entity, declares organisation as a property rather than an accessor, and carries a warning explaining what adding an accessor there costs. The merge suite's entity() helper builds a faithful concrete subclass, and testTheEntityDoubleMatchesTheRealObjectEntityAccessorShape asserts that premise so the fixture cannot silently drift back.

The instrument

property_exists() — which is what Entity::getter() itself decides on.

is_callable() is explicitly not used: it is true for any name on a __call class, so a probe swap would make the branch unconditionally true and move the failure into a runtime BadFunctionCallException. The accessor call is wrapped and its result type-checked in the same edit, and method_exists is kept as a second arm for an entity that genuinely declares the accessor.

Reading @self.organisation off jsonSerialize() was considered and rejected: getObjectArray() types organisation as array|string|null, so an expanded organisation would silently fail the UUID comparison in the caller.

Also fixed — same probe, same family

ReviewService::entityUuid() and IntakeService::entityUuid() (byte-identical copies) probed method_exists($entity, 'getUuid'). saveObject() returns an object, so the is_array() fallback below could never rescue it: both returned null for every real save, putting uuid: null in the submit response and in the audit log.

The other seven getUuid probes in this repo are dead code with a working fallback — ObjectEntity::getObject() always injects the uuid under id, so their $data['id'] arm already holds. Read, classified by what the false branch does, and deliberately left alone.

Evidence

Both revert predictions were written down before reverting, and reverted with the editor rather than a script:

• Reverting only the merge probe was predicted to turn six named tests red. Observed: six failures, exactly those six. Confirmed by grep which line carried the revert.
• Reverting only the two entityUuid probes was predicted to turn the two new tests red and nothing else. Observed: exactly that, over the full 667-test suite.

Full unit suite passes at 667 tests / 2332 assertions. phpcs, phpmd, psalm and phpstan all exit clean.

Known remaining debt, not hidden

The stub still declares getUuid() concretely because eight other test files configure it on a mock. A double routed through saveObject()'s : ObjectEntity return type therefore cannot express the magic getUuid shape, so the two new entityUuid tests exercise the private method directly against a faithful Entity subclass. Reshaping that declaration across all eight files is a separate change.

Docblock correction

The class docblock credited the @self.organisation write path to SaveObject::applyCallerSuppliedFields(). No such method exists anywhere in OpenRegister — grepped across the whole tree with a positive control. The real acceptance path is SaveObject::setSelfMetadata(), which honours a caller-supplied @self.organisation for an admin or a verified member of the target organisation; a merge is admin-triggered, so the admin arm applies.

[github-actions]

Quality Report — ConductionNL/softwarecatalog @ 14ecc5e

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
composer			✅	✅ 128/128
npm			✅	✅ 718/718
PHPUnit					❌
Newman					⏭️
Playwright					❌
Hydra gates					❌

---

Quality workflow — 2026-08-11 23:53 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/softwarecatalog @ dd7f0c5

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
composer			✅	✅ 128/128
npm			✅	✅ 718/718
PHPUnit					✅
Newman					⏭️
Playwright					✅
Hydra gates					✅

---

Quality workflow — 2026-08-12 00:07 UTC

Download the full PDF report from the workflow artifacts.