fix(Organisation): remove duplicate property declarations

[rubenvdlinde]

Summary

lib/Db/Organisation.php declared seven integration-link properties twice. PHP fatals on the first redeclaration, blocking every API call that touches the Organisation entity with HTTP 500 and no body — affecting every consumer (decidesk, opencatalogi, pipelinq, procest, mydash, …).

PHP Fatal error: Cannot redeclare OCA\OpenRegister\Db\Organisation::$mail
at /var/www/html/custom_apps/openregister/lib/Db/Organisation.php#320

Root cause

Two property blocks declared the same names:

Line range	Style
287-306	short docblocks (/** @var array|null Linked mail app data */)
315-362	longer docblocks (/** Linked mail entity IDs for this organisation. */)

Likely a copy-paste leftover from a rename-then-restore merge. Both blocks declared protected ?array $X = null for the same seven names: mail, contacts, notes, todos, calendar, talk, deck. Deleting the second block restores the file.

Test plan

• apache2ctl graceful (clears OPcache)
• curl -u admin:admin '/apps/openregister/api/objects?register=decidesk&schema=decision&_limit=1' returns HTTP 200 with {"results":[],"total":0,...} instead of HTTP 500
• CI green (PHPCS/PHPMD/PHPStan/Psalm)
• Reviewer to confirm no other call sites depend on the duplicate-block docblocks

Discovered while

Validating the decidesk JSON manifest pilot — the 500s on every openregister API call were the first thing that broke when running decidesk against a freshly-imported register.

[github-actions]

Quality Report — ConductionNL/openregister @ 8440a66

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
composer			✅	✅ 147/147
npm			✅	✅ 599/599
PHPUnit					❌
Newman					❌
Playwright					⏭️

---

Quality workflow — 2026-04-28 14:26 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

🔴 Blocker — This PR removes the ONLY property declarations, not a duplicate

development HEAD has these properties as a single block at lines 299-341:

299: protected ?array $mail = null;
306: protected ?array $contacts = null;
313: protected ?array $notes = null;
320: protected ?array $todos = null;
327: protected ?array $calendar = null;
334: protected ?array $talk = null;
341: protected ?array $deck = null;

After this PR is applied, grep -n 'protected ?array \$mail' lib/Db/Organisation.php returns nothing — all seven declarations are gone. The PR description's premise of "two blocks, deleted the duplicate" cannot be reproduced from the actual development history; the duplication must have existed only on a local rebased branch.

Impact: Without these property declarations:

• On PHP 8.2: E_DEPRECATED warnings on every dynamic-property access (Nextcloud's Entity doesn't carry #[AllowDynamicProperties]).
• On PHP 8.3+: dynamic-property deprecation surfaces. QBMapper::fromRow() writing $this->mail = $decodedJson may silently no-op or throw depending on NC version. Entity::__call → getter('mail') reflection on a non-existent property returns null.
• API consumers (decidesk, opencatalogi, pipelinq, procest, mydash) receive null for _mail, _contacts, _notes, _todos, _calendar, _talk, _deck on every Organisation response — same data-loss the original 15835bd574 commit fixed, just silent instead of fatal.

Suggested fix: Pull development HEAD, run grep -n 'protected ?array \$mail' lib/Db/Organisation.php — if the seven declarations exist (they do), the duplication has already been resolved upstream and this PR is unnecessary. Close it. If for some reason they need to be re-added, restore the block exactly as it appears on development.

[WilcoLouwerse]

🔴 Blocker — Three subsystems still reference the deleted properties

Even after this deletion, the following remain wired to the now-missing properties:

Location	Lines	Reads
@method annotations	77-90	getMail() … getDeck() documented as callable; resolved through Entity::__call → getter('mail') → reflection → returns null
addType() registrations	316-322	ORM type system tries to hydrate mail/contacts/.../deck on fromRow() via setter, hitting an undeclared property
jsonSerialize()	751-757	Directly reads $this->mail, $this->contacts, $this->notes, $this->todos, $this->calendar, $this->talk, $this->deck — all return null

All three subsystems remain wired to seven properties that no longer exist on the class.

Impact: Confirms the data-loss path from Blocker #1. A deletion-only PR that removes property declarations while leaving every dependent subsystem intact is never correct — either all of them remove together (signalling "these fields are deprecated and removed") or none do.

Suggested fix: If the intent is to keep the seven integration-link fields (the apparent goal — addType and jsonSerialize still register them), restore the declarations. If the intent is genuinely to remove them, also delete the seven @method annotations, the seven addType() calls, and the seven jsonSerialize() entries — and document the schema change.

[WilcoLouwerse]

🟡 Concern — Commit message describes a state that doesn't exist on development

The message references "lines 287-306 (short docblocks)" and "lines 315-362 (longer docblocks)". A git log --follow lib/Db/Organisation.php trace on the actual development HEAD shows only one block was ever committed — via 15835bd574 ("Added missing property declarations"). The line numbers in the commit message match neither the pre-PR file nor any state on development.

Impact: The PR was likely authored from a local-rebase artifact: branch forked before 15835bd574, then the author manually added property declarations locally (creating Block A); upstream then received 15835bd574 (creating Block B during rebase). The author saw two blocks and deleted what they thought was the redundant one — which turned out to be the canonical upstream block. The misleading commit message will also confuse future archaeology / git bisect.

Suggested fix: If the PR is closed as unnecessary, document the corrected diagnosis in the close comment so future reviewers understand the artefact. If the PR is reworked, update the commit message to reflect the actual state.

[WilcoLouwerse]

🟢 Minor — No automated guard against re-introduction

This is a deletion-only fix; nothing prevents the same duplicate-declaration bug from being re-introduced by a future merge conflict resolution (which is exactly how such duplicates typically appear).

Impact: Low probability, zero cost to guard.

Suggested fix: A tests/Unit/Db/OrganisationTest.php test that instantiates Organisation, calls setMail(['foo']), then asserts getMail() === ['foo']. One method covers (a) re-introduction of duplicate fatals and (b) the silent-undeclared-property regression this PR currently introduces.

[WilcoLouwerse]

Do not merge — this PR removes the only property declarations on the class, and addType, @method, and jsonSerialize all still reference the now-missing properties. Verified by checking out the PR head locally: protected ?array $mail/contacts/notes/todos/calendar/talk/deck are all gone, while $this->mail etc. are still read in jsonSerialize(). The duplication described in the commit message can't be reproduced from development's history — this looks like a local-rebase artefact. Recommended action: pull development, confirm the seven property declarations exist as a single block at lines 299-341, and close this PR as unnecessary.