Repository navigation
fix(organisations): check the slug of the name OpenRegister receives, link command safeguards (REVIEW) - #1419
Merged
Conversation
…fuse-slug-adoption
…e match When an organisatie had no OpenRegister organisation yet, the sync looked one up by the slug its name gives and, on a match, wrote the organisatie's uuid onto that organisation: a registration named like an existing organisation took it over. ContactPersonHandler did the same for a contact person's organisation and then added the contact's user to it. The slug was also computed without OpenRegister's 100-character cut, so a long name missed the match and OpenRegister's own create overwrote the existing organisation. Now a taken slug is refused on both paths: nothing is created or changed, a neutral warning names the organisatie and the organisation holding the slug, and the sync counts it under slugConflicts. OpenRegister's OrganisationSlugConflictException (openregister#5462, a 409) is handled the same way. OrganisationSlug computes the slug exactly as OpenRegister does. `occ stackiq:organisation:link <organisatie> <organisation>` lets an administrator link the two explicitly. It refuses an unknown organisatie or organisation, an organisatie that already has one, and an organisation that belongs to another organisatie. Assisted-by: ClaudeCode:claude-opus-5-5
The sync checked the slug of the organisatie's own name and skipped an empty or "Unknown" one, while OpenRegister receives "Organisation <first 8 characters of the id>" for an empty, "Unknown" or "0" name. That name reached OpenRegister's create with no check. OrganisationSlug::nameFor() now gives the name both the check and the create use. A contact person whose organisation has no entity (refused slug, missing object) no longer ends the event handler or the batch sync with an Error on getActive(): both paths go through organisationIsActive(). occ stackiq:organisation:link refuses OpenRegister's default organisation, names what keeps the old uuid, and links only after confirmation or --yes. OrganisatieService drops two collaborators it never used. Assisted-by: ClaudeCode:claude-opus-5-5
…llback-name-slug # Conflicts: # appinfo/info.xml # tests/Unit/Service/OrganizationSyncEntityGoldenTest.php
Its constructor matched ReferencesAuditCommand's line for line, which gate 122 counts as a new clone. Assisted-by: ClaudeCode:claude-opus-5-5
rubenvdlinde
marked this pull request as ready for review
November 3, 2026 22:30
Contributor
Quality Report — ConductionNL/stackiq @
|
| 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 | ✅ 1743/1743 · 8 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-04 02:35 UTC
Download the full PDF report from the workflow artifacts.
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.
REVIEW: touches who an organisation belongs to. Held for a reviewer. The sensitive detail is in PRIVATE-FIXES.md under SQ6. Stacked on #1418, so this diff shows #1418 until that merges.
Slug check on the name OpenRegister receives
What: the sync checked the slug of the organisatie's own name and skipped an empty or "Unknown" one, while OpenRegister receives a fallback name for an empty, "Unknown" or "0" name.
OrganisationSlug::nameFor()is now the one rule, used by the check and by the create.ContactPersonHandleralready checked the name it sends; its tests are unchanged and green.Also pinned: a slug with a hyphen at position 100 keeps it, as OpenRegister's does (RW134's surviving mutant).
Tests: failing first on the old code. The fallback capture test creates an organisation there and refuses here. New
OrganisationSlugTest.Note: the
uniqid()fallback for a missing id is gone. It could not run, becausecreateOrganisationInOpenRegister()returns before mapping when the id is empty.No Error on a missing organisation entity
What:
getActive()on a null entity raised an Error thatcatch (\Exception)does not catch. The event path had it, and so did the batch pathprovisionContactUser, which the task did not name. Both now useorganisationIsActive(), and the contact is skipped and logged.Golden: two recordings changed from
Error: Call to a member function getActive() on nullto no escape plus the skip log line. That diff is the fix.findBySlugentries were added where the check now runs.Tests:
testTheEventPathNeedsAnActiveOrganisationEntity, 5 cases, 3 of them failing first with the Error.Link command
What:
occ stackiq:organisation:linkrefuses OpenRegister's default organisation, names what keeps the old uuid (object stamps, members' active organisation, child organisations' parent, other apps) and links only after confirmation or--yes. A run without a terminal and without--yeslinks nothing. The default is asked of OpenRegister'sgetDefaultOrganisationUuid(), and a failure there refuses the link.Limit: the references are named, not counted or moved.
OrganisatieService
Dropped two collaborators it never used (
OrganizationHandler,IAppConfig), which phpmd's coupling limit required once it usedOrganisationSlug. Added the class@spectag.Verified
Mutants, capped, control survived: slug 7 of 7, OrganisatieService 1 of 1, command 6 of 6, sync 12 of 12.
Linters on touched files after merging development: phpcs, phpstan, psalm, phpmd clean.
One heavy run on the combined tree of both held SQ6 PRs, 8e4e2e04 (development 3604e5b merged in): full PHPUnit under pcov with --fail-on-risky, exit 0, 1756 tests, 17377 assertions, 8 skipped. The 8 skips predate this work:
OrganisationUserWorkflowTest(6) andContactPersonHandlerTest(2), stale tests a refactor outran.Hydra gates --scope-to-diff --base origin/development: 42 of 42 applicable ran. Two results to read: gate 122 found one new clone, this PR's link command constructor against
ReferencesAuditCommand, fixed in 37053c8, then the ratchet re-ran with 0 findings in 49 changed units (30 inherited). Gate 116 failed because ajv was not installed in that tree; run on its own with ajv present it reports 0 findings, andconnections.jsonis not in this diff. The full suite was not repeated for the one-line constructor change: its test file and phpcs were re-run.Not changed: the race between the slug lookup and the create. openregister#5462's 409 closes it and this branch handles that 409.
🤖 Generated with Claude Code