feat(organisations): adopt a leaf app's organisations without losing them - #3340
Merged
Conversation
…them Several apps grew their own organisation schema before OR's Organisation carried the fields they needed. The slug is global per organisation, so those copies collide: SchemaMapper::find() matches LOWER(slug) across every app and hands back whichever row it reaches first. Task 1 added the columns, which made reuse possible and moved nobody's rows. This moves them, under two rules that were learned rather than chosen. The uuid is the idempotency key and is preserved. dossiq's migrate-partners arrived at the same rule and wrote down why: a leaf row is free to carry no slug at all, and two rows sharing a name are routine, so a name-derived key skips the second as already migrated and silently merges two distinct legal entities. Where the same legal entity already exists under a different uuid the rows are not collapsed into one. The adopted row is created and pointed at the existing one through mergedInto, so both uuids keep resolving and the merge is a fact recorded on a row rather than data thrown away. Matching runs on OIN, then RSIN, then KVK, normalised for punctuation because the same OIN is typed with and without spaces and dots, and never on a name. The lowest id is canonical so a repeated run picks the same survivor, and a candidate that was itself merged away loses to a live one. Properties Organisation has no column for are named before the write. OpenRegister discards an undeclared property and answers 200 with the object, so an adoption that loses fields is otherwise indistinguishable from one that did not. Dry-run by default. Verified on the dev instance end to end: adoption, a second run adopting nothing, a merge matched across punctuation, and the negative control where clearing the shared OIN stops the merge being reported.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-02 12:33 UTC
Download the full PDF report from the workflow artifacts.
The plan for task 5.2 was to map stackiq's fields onto Organisation where they fit and rehome the rest. Reading the entity rather than assuming, none of the three candidates fit. contacts is linked Nextcloud Contacts app data, not a list of contact people. children is not stored at all: the setter says it is loaded on demand for API responses, so a written value is dropped and recomputed. type has a closed vocabulary and a collaboration SUBtype is a different axis from it. So all nine stackiq properties belong on a stackiq-owned schema and none on Organisation, which is a different change from the one that was planned. Also records the site counts that decide the sequencing: 22 for opencatalogi, most of them stored references that keep resolving, against 235 for stackiq, which treats the slug as a first-class object type.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-02 17:48 UTC
Download the full PDF report from the workflow artifacts.
The coverage guard was right again: the pure rules were pinned and the command that runs them was not, so execute(), adoptRow(), reportUndeclared() and existingOrganisations() shipped uncovered. Ten cases through CommandTester, and the ones that matter are the refusals. Without --register the command says so rather than reading whatever it can find. The default is a dry run, because the alternative default is a command that writes to every organisation on the instance the first time somebody types its name to see what it does. A row with no uuid is skipped rather than adopted, since without an idempotency key it would be duplicated on every run. One case is worth its own line: two rows in the SAME run sharing a legal identifier now merge, because the candidate set grows as rows are adopted. That path had no coverage and is exactly where a by-value parameter would have silently done nothing.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-03 08:19 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
1 participant
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.
Why
Several apps grew their own
organizationschema before OpenRegister's Organisation carried the fields they needed. The slug is global per organisation, so those copies collide:SchemaMapper::find()matchesLOWER(slug)across every app and hands back whichever row it reaches first.Task 1 of
consolidate-organisation-on-oradded the columns. That made reuse possible and moved nobody's rows. Task 5.1 was marked DECISION REQUIRED and has now been decided: adopt the leaf rows, and auto-merge on a legal identifier.The two rules, and why they are these rules
The uuid is the idempotency key, and it is preserved. dossiq's
migrate-partnersarrived at the same rule and wrote down why: a leaf row is free to carry no slug at all, and two rows sharing a name are routine, so a name-derived key would skip the second as "already migrated" and silently merge two distinct legal entities. Preserving the uuid also keeps every stored reference resolving, including the ones no migration can reach.A duplicate records a merge; it does not collapse. Where the same legal entity already exists under a different uuid, the adopted row is created and pointed at the existing one through
mergedInto. Both uuids keep resolving, and the merge is a fact on a row rather than data thrown away.Matching runs on OIN, then RSIN, then KVK, and never on a name. Identifiers are normalised for punctuation, because the same OIN gets typed with and without spaces and dots. The lowest id is canonical so a repeated run picks the same survivor, and a candidate that was itself merged away loses to a live one.
Properties with no column are named before the write. OpenRegister discards a property its schema does not declare and answers 200 with the object, so an adoption that loses fields looks exactly like one that did not. The command reports them. On opencatalogi that is
tooiIdentifier, and only that.Verification
Dry-run by default. Proven on the dev instance end to end, not only in tests:
0000-0001.0022 20647000against00000001002220647000) and recorded the merge15 unit tests pin the rules, including the two that must not fire: a shared name never merges, and an empty identifier is not a match.
phpcs, phpmd, psalm and phpstan clean.
Closes task 5.1. Adds REQ-ORG-106 to the change's spec.
What is still open
Task 5.2 is the leaf side. opencatalogi maps 9-for-9 onto Organisation apart from
tooiIdentifier. Stackiq's 21 properties leave 8 with no column (xml,contactsUid,contactpersonen,deelnames,participants,samenwerkingtype,registeredBy,publicationDate/depublicationDate), so its schema needs a field ruling before it can be retired.