[rubenvdlinde]

REVIEW: contact person and organisation data. This PR restructures the code that creates contact person objects, keeps their link to the organisation, creates their user accounts and sets their owner. It is meant to change nothing; please review it as such before it merges.

What

OrganizationSyncService::createOrUpdateContactPersonObject (298 lines, McCabe 23, 5 parameters) and updateContactPersonObjectOwner (89 lines, 5 parameters) are split into named steps:

step	does
findExistingContactPerson	fetch the contact an id or uuid points at
createContactPersonObject	create the contact without its organisation field, then write that field back
completeContactPerson	count it, ensureContactOrganisation, log, and provision a user when it has no username
provisionContactUser	find the organisation entity (findOrganisationEntityForContact, createBackupOrganisationEntity) and create the user only when it is active, else logSkippedUserCreation
createUserForContact, linkCreatedUser, saveContactUsername	create the account, store the uid on the contact, add the user to the organisation, make the user the owner
stageContactOwner, ownerOrganisation	the owner update: @self owner and organisation, the organisation field safety net, the entity's owner and organisation
persistContactObject	the one save-as-is call the four writes shared

Both methods now take one array: the sync run (organizationUuid, register, contactSchema, and the caller's stats held by reference so every step counts into them as it goes, also when a later step throws), or the owner (userUID, register, contactSchema, organizationUuidOverride). Both are private; their three call sites in this class are updated. Every new step takes at most 2 parameters, McCabe at most 6.

What stays exactly as it was, on purpose:

• which user the account is created for and who becomes owner: the same values reach createUserAccount, addUserToOrganizationEntity, setOwner and setOrganisation;
• an organisation entity that is missing and cannot be created still makes getActive() fail on null, and that Error still escapes the method as before (it is not an \Exception, so neither catch takes it). Fixing that is a behaviour change for a separate PR;
• the organisation override still wins over the contact's own organisation, and an empty override still falls back.

One redundant check went: after find-or-create the contact object can no longer be null (PHPStan flagged !== null as always true on the new types).

SIG (gate 122 reader, lizard 1.24.3, five-star profile)

php	development	this PR	4-star	5-star
size >15	84.11%	84.07%	47.1%	23.6%
size >30	62.85%	62.42%	23.1%	11.6%
size >60	36.99%	36.09%	8.3%	4.2%
McCabe >5	52.63%	51.77%	20.2%	10.1%
McCabe >10	20.88%	20.18%	7.3%	3.7%
McCabe >25	3.61%	3.60%	1.1%	0.55%
params >=3	20.58%	19.70%	15.0%	7.5%
params >=5	4.69%	3.82%	3.3%	1.65%
params >=7	0.72%	0.72%	0.9%	0.45%
duplication	11.56%	11.40%	5.6%	2.8%

No check flips on its own. Together with #1374 (merged), #1376, #1377 and the scope file #1351, php params >=5 reaches 3.28%, under the 3.3% 4-star limit (margin 6 unit lines). This PR carries most of that drop (298 + 89 lines leave the band), so the flip waits on this review.

Verified

• Golden test tests/Unit/Service/OrganizationSyncContactPersonGoldenTest.php, 23 cases recorded on the old code: every branch of both methods (no email, existing or failed fetch, create with and without organisation, missing organisation entity with a backup that finds none, fails or builds one, inactive, non-true and throwing getActive(), a generic mapper error, user creation failing, the username or owner save failing, a failed create, no organisation uuid at all, and seven owner-update cases). Each case records every logger, object service (by parameter name, defaults left out), mapper, handler and settings call, the stats and the escaping exception. Re-run plain on old and new: 23/23.
• Capped mutants (prlimit --as=1610612736 timeout 120): 25 of 26 killed over three rounds, the no-op control survived each round (a first-round survivor, the backup entity's return value, led to an e-mail service double so that case completes). The survivor is equivalent: logSkippedUserCreation's null branch cannot be reached, because a null entity already fails in provisionContactUser.
• pcov with --fail-on-risky on the golden and the existing OrganizationSyncService test: 29 tests OK, 0 risky.
• php -l, phpcs (0 errors), phpstan (0 errors), psalm (0 errors), phpmd both rule sets (0 violations) on the changed file.
• gate 122 ratchet: 0 findings (19 inherited); diff-check gates, phpcs, phpstan PASS with 0 new (its phpunit step exits 255 on phpunit.xml's server bootstrap, as on development).
• Full unit suite once on this head under pcov with --fail-on-risky: 1,529 tests, the same 20 errors as development (Doctrine\DBAL\ParameterType missing locally), 0 risky.
• development (with refactor(store): split the related-data fetch and make the stackiq plugin measurable #1370, refactor(cmdb-import): look error texts up in a table #1371, refactor(store): drive the focused schema selections from key tables #1372 and ci(quality): block on the SIG unit ratchet and commit the SIG score floor #1373) merged in before the push; gate 123 (sig-floor) on the merged head: PASS, 29 floored checks held.

AI disclosure: prepared with Claude Code (claude-opus-5-5) and reviewed by the lane before opening.

[rubenvdlinde]

Merged development (a019035, includes #1374) into this branch. Re-run on the merged head: golden 23/23; full unit suite under pcov with --fail-on-risky 1,529 tests, the same 20 errors as development, 0 risky; gate 122 ratchet 0; gate 123 PASS (29 floored checks held).

[github-actions]

Quality Report — ConductionNL/stackiq @ a2d444b

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
format		❌
check-schema-l10n		❌
check-l10n-js		✅
composer			❌	✅ 127/127
npm			❌	✅ 807/807
app:check-code	⏭️
info.xml	✅
REUSE	❌
lockfile sync		✅
PHPUnit					✅ 1509/1509 · 20 skipped
Newman					⏭️
Playwright					⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates					❌

---

Quality workflow — 2026-11-02 23:22 UTC

Download the full PDF report from the workflow artifacts.