retrofit: reverse-spec organisatie-service (5 REQs / 8 methods)

lib/Service/OrganisatieService.php

@@ -70,6 +70,8 @@ public function __construct(
      * @param array $objectData The organization object data
      *
      * @return object|null The created organisation entity or null on failure
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-1
      */
     public function createOrganisationInOpenRegister(array $objectData): ?object
     {
@@ -135,6 +137,8 @@ public function createOrganisationInOpenRegister(array $objectData): ?object
      * @param array  $objectData       The organization object data
      *
      * @return bool True if update was successful
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-2
      */
     public function updateOrganizationStatus(string $organizationUuid, array $objectData): bool
     {
@@ -204,6 +208,8 @@ private function getOrganisationService(): ?\OCA\OpenRegister\Service\Organisati
      * @param array $objectData The organization object data.
      *
      * @return array The mapped data for OpenRegister.
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-3
      */
     private function mapOrganizationDataForOpenRegister(array $objectData): array
     {
@@ -232,6 +238,8 @@ private function mapOrganizationDataForOpenRegister(array $objectData): array
      * @param string $status The status from Software Catalog
      *
      * @return bool The mapped active status for OpenRegister
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-2
      */
     private function mapStatus(string $status): bool
     {
@@ -259,6 +267,9 @@ private function mapStatus(string $status): bool
      *
      * @return \OCA\OpenRegister\Db\Organisation The created organisation entity
      */
+    /**
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-1
+     */
     private function createOrganisationEntityInternal(
         \OCA\OpenRegister\Service\OrganisationService $organisationService,
         array $mappedData,
@@ -307,6 +318,8 @@ private function createOrganisationEntityInternal(
      * @param \OCA\OpenRegister\Service\OrganisationService $organisationService The organisation service
      *
      * @return string|null The active organisation UUID or null if not set
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-1
      */
     private function getActiveOrganisationUuid(
         \OCA\OpenRegister\Service\OrganisationService $organisationService
@@ -336,6 +349,8 @@ private function getActiveOrganisationUuid(
      * @param array  $usernames        Array of usernames to add
      *
      * @return bool True if successful
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-4
      */
     public function addUsersToOrganization(string $organizationUuid, array $usernames): bool
     {
@@ -409,6 +424,8 @@ public function addUsersToOrganization(string $organizationUuid, array $username
      * Gets admin group usernames.
      *
      * @return array Array of admin usernames
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md#task-5
      */
     public function getAdminGroupUsernames(): array
     {

openspec/changes/retrofit-2026-05-24-organisatie-service/design.md

@@ -0,0 +1,26 @@
+# Design — Retrofit Organisatie Service
+
+Retrofit change. Tasks describe retroactive annotation, not new implementation work.
+
+## Context
+
+`OrganisatieService` is the SC↔OR integration layer for organisation records. It is one of three organisation-touching surfaces in the codebase — distinct from `OrganizationSyncService` (sync pipeline, in Bucket 1) and `SoftwareCatalogue\OrganizationHandler` (group/role helpers, in Bucket 1). The split predates the coverage scan and has not been consolidated.
+
+## Decisions
+
+- **Cluster mode** — no existing capability covers this surface; the other two organisation services are already in Bucket 1 under `method-decomposition#REQ-DECOMP-007 / REQ-DECOMP-011`.
+- **5 REQs / 8 methods** — within the 5-REQ cap. Private helpers (`createOrganisationEntityInternal`, `getActiveOrganisationUuid`, `mapOrganizationDataForOpenRegister`) are folded into the public-method REQs. `mapStatus` is its own block inside REQ-002 because it has independent contract value (status enum mapping table).
+- **Status enum mapping spelled out as a table-style REQ.** Defaulting unknown statuses to `true` is a deliberate (if questionable) choice — captured verbatim, not "fixed".
+- **HOTFIX flagged in Notes.** The parent-organisation feature is disabled by comments. The REQ describes the *current* behaviour (no parent). Restoring requires RBAC work — not for this retrofit.
+
+## Out of scope
+
+- Consolidating the three organisation-touching services into one.
+- Restoring parent-organisation assignment (requires OR RBAC redesign).
+- Removing the duplicate exception logging in `addUsersToOrganization`.
+
+## References
+
+- Umbrella: ConductionNL/softwarecatalog#285
+- Coverage report: openspec/coverage-report.md (2026-05-24)
+- Source: lib/Service/OrganisatieService.php

openspec/changes/retrofit-2026-05-24-organisatie-service/proposal.md

@@ -0,0 +1,21 @@
+# Retrofit — organisatie-service
+
+Describes observed behavior of 8 methods in `OrganisatieService` as 5 new REQs under a new `organisatie-service` capability. Code already exists — this change retroactively specifies it.
+
+## Affected code units
+
+- lib/Service/OrganisatieService.php::createOrganisationInOpenRegister
+- lib/Service/OrganisatieService.php::updateOrganizationStatus
+- lib/Service/OrganisatieService.php::mapOrganizationDataForOpenRegister
+- lib/Service/OrganisatieService.php::mapStatus
+- lib/Service/OrganisatieService.php::createOrganisationEntityInternal
+- lib/Service/OrganisatieService.php::getActiveOrganisationUuid
+- lib/Service/OrganisatieService.php::addUsersToOrganization
+- lib/Service/OrganisatieService.php::getAdminGroupUsernames
+
+## Approach
+
+- Describe organisation lifecycle (create / status update / user assignment) and the disabled parent-organisation HOTFIX.
+- Notes flag the RBAC HOTFIX that disabled automatic parent-org assignment + the redundant exception logging in `addUsersToOrganization`.
+
+Source: openspec/coverage-report.md generated 2026-05-24. Umbrella: ConductionNL/softwarecatalog#285.

openspec/changes/retrofit-2026-05-24-organisatie-service/specs/organisatie-service/spec.md

@@ -0,0 +1,110 @@
+---
+status: draft
+retrofit: true
+---
+
+# Organisatie Service Specification
+
+## Purpose
+
+Captures observed behavior of `OCA\SoftwareCatalog\Service\OrganisatieService`, the integration layer that synchronises SoftwareCatalog organisation records into OpenRegister's `OrganisationService`. Covers creation, status transition, user assignment, and helper queries. Distinct from `OrganizationSyncService` (which orchestrates the full SC↔OR sync pipeline) and `OrganizationHandler` (sub-handler under `SoftwareCatalogueService`). Reverse-spec'd from existing code 2026-05-24.
+
+## ADDED Requirements
+
+### REQ-001: The system SHALL create an OpenRegister organisation entity from SoftwareCatalog object data
+
+`createOrganisationInOpenRegister(objectData)` MUST require a non-empty `objectData['id']` (logs error + returns `null` otherwise). It MUST map the input to OpenRegister format via `mapOrganizationDataForOpenRegister`, lookup `OrganisationService` from the OpenRegister app (returning `null` if the app is not enabled), and call `OrganisationService::createOrganisation` with `addCurrentUser: false` and the original SC UUID. On success it MUST return the created `Organisation` entity; on any exception it MUST log + return `null` (never propagate).
+
+#### Scenario: Missing id is rejected
+- GIVEN `objectData['id']` is null or empty string
+- WHEN `createOrganisationInOpenRegister(objectData)` is called
+- THEN it MUST log an error and return `null`
+- AND it MUST NOT invoke OpenRegister
+
+#### Scenario: OpenRegister disabled returns null
+- GIVEN the OpenRegister app is not enabled for the user
+- WHEN `createOrganisationInOpenRegister(['id' => 'uuid-1', 'naam' => 'Org'])` is called
+- THEN it MUST return `null`
+- AND it MUST log an error referencing OrganisationService unavailability
+
+#### Scenario: Successful creation returns entity
+- GIVEN OpenRegister is enabled and `OrganisationService::createOrganisation` succeeds
+- WHEN the public method is called with valid data
+- THEN the returned value MUST be the `OCA\OpenRegister\Db\Organisation` entity
+- AND the call to `createOrganisation` MUST pass `addCurrentUser: false` and the original SC UUID
+
+### REQ-002: The system SHALL update the active flag of an OpenRegister organisation from a SoftwareCatalog status
+
+`updateOrganizationStatus(organizationUuid, objectData)` MUST find the OpenRegister `Organisation` by SC UUID via `OrganisationMapper::findByUuid`, map `objectData['beoordeling']` (default `'actief'`) through `mapStatus` to a boolean, call `setActive` + `save`. On success it MUST return `true`; on any exception it MUST log + return `false` (never propagate).
+
+`mapStatus(status)` MUST normalise its input (lowercase + trim) and return: `true` for `actief` / `active`; `false` for `inactief` / `inactive` / `deactief`; `true` for any other value (default-active for unknown statuses).
+
+#### Scenario: Active status maps to true
+- WHEN `mapStatus('Actief')` is called
+- THEN the return value MUST be `true`
+
+#### Scenario: Inactive variants map to false
+- WHEN `mapStatus(' inactief ')` is called
+- THEN the return value MUST be `false`
+- AND `mapStatus('deactief')` MUST also return `false`
+
+#### Scenario: Unknown status defaults to active
+- WHEN `mapStatus('pending')` is called
+- THEN the return value MUST be `true`
+
+#### Scenario: Update success
+- GIVEN an organisation exists in OR with the supplied UUID
+- WHEN `updateOrganizationStatus('uuid-1', ['beoordeling' => 'inactief'])` is called
+- THEN the OR organisation's `active` flag MUST be `false` after the call
+- AND the method MUST return `true`
+
+### REQ-003: The system SHALL map SoftwareCatalog organisation data to the OpenRegister payload shape
+
+`mapOrganizationDataForOpenRegister(objectData)` MUST return an associative array with keys `naam`, `type`, `website`, `active`, `contactpersonen`, `deelnemers`. The `naam` MUST be resolved from `objectData['naam']` then `objectData['name']`, falling back to `Organisation <first-8-chars-of-id>` when both are missing or equal to `Unknown`. The `active` field MUST be the result of `mapStatus(status ?? beoordeling ?? 'actief')`.
+
+#### Scenario: Naam fallback uses id prefix
+- GIVEN `objectData = { id: 'abc12345-xxxx-...' }` with no `naam` or `name`
+- WHEN `mapOrganizationDataForOpenRegister` is called
+- THEN the returned `naam` MUST equal `"Organisation abc12345"`
+
+#### Scenario: Naam prefers naam over name
+- GIVEN `objectData = { naam: 'Org NL', name: 'Org EN', id: 'x' }`
+- WHEN the method is called
+- THEN the returned `naam` MUST equal `'Org NL'`
+
+### REQ-004: The system SHALL add users to an OpenRegister organisation and notify each one
+
+`addUsersToOrganization(organizationUuid, usernames)` MUST find the OR `Organisation` by SC UUID, merge the new usernames into the existing `users` array (dedup via `array_unique`), persist via `OrganisationMapper::save`, and for each supplied username call `SymfonyEmailService::sendUserUpdateEmail` with the user's `{username, email, name}` + the organisation's `jsonSerialize()`. On success it MUST return `true`; on any exception it MUST log + return `false` (never propagate).
+
+#### Scenario: User assignment merges and dedups
+- GIVEN the OR organisation already has users `['alice']`
+- WHEN `addUsersToOrganization('uuid', ['bob', 'alice'])` is called
+- THEN the OR organisation's `users` MUST equal `['alice', 'bob']` (order preserved, dedup applied)
+- AND `setUsers` + `save` MUST be called
+
+#### Scenario: Notification is sent per new user
+- GIVEN the merge produced 2 new usernames
+- WHEN the method runs
+- THEN `sendUserUpdateEmail` MUST be invoked once per supplied username with the user payload
+
+### REQ-005: The system SHALL list the usernames of the Nextcloud admin group
+
+`getAdminGroupUsernames()` MUST resolve the `admin` group via `IGroupManager::get('admin')`, return the UID of every user in the group as a flat array of strings. If the admin group cannot be found OR if any exception is thrown, it MUST return `[]` (never propagate).
+
+#### Scenario: Admin group resolved
+- GIVEN the `admin` group exists with members `[admin1, admin2]`
+- WHEN `getAdminGroupUsernames()` is called
+- THEN the returned array MUST equal `['admin1', 'admin2']`
+
+#### Scenario: Admin group missing returns empty
+- GIVEN `IGroupManager::get('admin')` returns `null`
+- WHEN `getAdminGroupUsernames()` is called
+- THEN the returned array MUST equal `[]`
+
+## Notes
+
+- **HOTFIX — parent-organisation assignment is disabled.** `createOrganisationEntityInternal` originally set every new organisation as a child of the active organisation, but this caused RBAC permission problems (users couldn't access their newly-created orgs due to hierarchical RBAC filtering). The code is commented out (lines 267-271). REQ-001 describes the *current* contract (no parent assignment); restoring the parent relationship requires the RBAC investigation noted in the file's TODO. Follow-up: track as a separate issue tied to OR RBAC work.
+- **`getActiveOrganisationUuid()` is unused.** Its only caller is the commented-out HOTFIX block. The class is annotated with `@SuppressWarnings(PHPMD.UnusedPrivateMethod)` to silence the lint warning. Not specified as a REQ; remove after HOTFIX resolution or keep as documentation of the disabled flow.
+- **`createOrganisationEntityInternal` is private plumbing** for REQ-001 — folded into the REQ-001 description, not exposed as its own scenario.
+- **Redundant logging in `addUsersToOrganization` catch block.** The exception is logged twice (lines 388-403) — the second call is dead code. Not in spec; track for cleanup.
+- **Acceptance Criteria:** Covered indirectly by the organisation-sync integration tests and Newman runs against `/api/organisations`; no direct unit coverage at retrofit time.

openspec/changes/retrofit-2026-05-24-organisatie-service/tasks.md

@@ -0,0 +1,7 @@
+# Tasks
+
+- [x] task-1: organisatie-service#REQ-001 — System SHALL create an OpenRegister organisation entity from SoftwareCatalog object data (retroactive annotation)
+- [x] task-2: organisatie-service#REQ-002 — System SHALL update the active flag of an OpenRegister organisation from a SoftwareCatalog status (retroactive annotation)
+- [x] task-3: organisatie-service#REQ-003 — System SHALL map SoftwareCatalog organisation data to the OpenRegister payload shape (retroactive annotation)
+- [x] task-4: organisatie-service#REQ-004 — System SHALL add users to an OpenRegister organisation and notify each one (retroactive annotation)
+- [x] task-5: organisatie-service#REQ-005 — System SHALL list the usernames of the Nextcloud admin group (retroactive annotation)