From 69018aee5c5a0e92601ab5fce36395f08b6af845 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 14:58:37 +0100 Subject: [PATCH 1/7] test(organization-sync): golden for the related contact person sync Assisted-by: ClaudeCode:claude-opus-5-5 --- ...anizationSyncRelatedContactsGoldenTest.php | 642 ++++++++ .../organizationSyncRelatedContacts.json | 1374 +++++++++++++++++ 2 files changed, 2016 insertions(+) create mode 100644 tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php create mode 100644 tests/Unit/Service/golden/organizationSyncRelatedContacts.json diff --git a/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php b/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php new file mode 100644 index 000000000..9928614e1 --- /dev/null +++ b/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php @@ -0,0 +1,642 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/specs/organization-sync/spec.md + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Service; + +use OCA\OpenRegister\Contract\ObjectEntityInterface; +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\OpenRegister\Db\Organisation; +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\Stackiq\Service\OrganisatieService; +use OCA\Stackiq\Service\OrganizationSyncService; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Service\Stackiq\ContactPersonHandler; +use OCA\Stackiq\Service\SymfonyEmailService; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IUser; +use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; + +/** + * Golden cases for the related contact person sync. + * + * @covers \OCA\Stackiq\Service\OrganizationSyncService + * + * @spec openspec/specs/organization-sync/spec.md + */ +class OrganizationSyncRelatedContactsGoldenTest extends TestCase { + + /** + * Everything the doubles saw, in order. + * + * @var array + */ + private array $trace = []; + + /** + * Per-case behaviour of the doubles. + * + * @var array + */ + private array $plan = []; + + /** + * The golden file. + * + * @return string + */ + private static function golden(): string { + return __DIR__ . '/golden/organizationSyncRelatedContacts.json'; + + }//end golden() + + /** + * A contact or organisation object double that records its writes. + * + * @param array $object The object data. + * @param string $uuid The uuid. + * + * @return ObjectEntityInterface + */ + private function entity(array $object, string $uuid): ObjectEntityInterface { + $trace = &$this->trace; + return new class($object, $uuid, $trace) implements ObjectEntityInterface { + /** + * The owner set on the entity. + * + * @var string|null + */ + private ?string $owner = null; + + /** + * The organisation set on the entity. + * + * @var string|null + */ + private ?string $organisation = 'org-of-entity'; + + /** + * Build the double. + * + * @param array $object The object data. + * @param string $uuid The uuid. + * @param array $trace The shared trace. + */ + public function __construct(private array $object, private string $uuid, private array &$trace) { + } + + /** + * @return mixed + */ + public function getId() { + return 7; + } + + /** + * @return string|null + */ + public function getUuid(): ?string { + return $this->uuid; + } + + /** + * @return array + */ + public function getObject(): array { + return $this->object; + } + + /** + * @return string|null + */ + public function getRegister(): ?string { + return 'reg-entity'; + } + + /** + * @return string|null + */ + public function getSchema(): ?string { + return 'schema-entity'; + } + + /** + * @return string|null + */ + public function getOrganisation(): ?string { + return $this->organisation; + } + + /** + * @return string|null + */ + public function getOwner(): ?string { + return $this->owner; + } + + /** + * @param mixed $object The data. + * + * @return void + */ + public function setObject($object=null) { + $this->trace[] = ['entity.setObject', $this->uuid, $object]; + $this->object = $object; + } + + /** + * @param string|null $owner The owner. + * + * @return void + */ + public function setOwner(?string $owner): void { + $this->trace[] = ['entity.setOwner', $this->uuid, $owner]; + $this->owner = $owner; + } + + /** + * @param string|null $organisation The organisation. + * + * @return void + */ + public function setOrganisation(?string $organisation): void { + $this->trace[] = ['entity.setOrganisation', $this->uuid, $organisation]; + $this->organisation = $organisation; + } + + /** + * @return mixed + */ + public function jsonSerialize(): mixed { + return $this->object; + } + + /** + * @param string $name The method. + * @param array $args The arguments. + * + * @return mixed + */ + public function __call(string $name, array $args) { + $this->trace[] = ['entity.' . $name, $this->uuid, $args]; + return null; + } + }; + + }//end entity() + + /** + * Arguments by parameter name, leaving out the ones left at their default. + * + * @param string $method The ObjectServiceInterface method. + * @param array $args The positional arguments the double received. + * + * @return array + */ + private static function named(string $method, array $args): array { + $named = []; + foreach ((new \ReflectionMethod(ObjectServiceInterface::class, $method))->getParameters() as $i => $param) { + if (array_key_exists($i, $args) === false) { + continue; + } + + if ($param->isDefaultValueAvailable() === true && $param->getDefaultValue() === $args[$i]) { + continue; + } + + $value = $args[$i]; + if ($value instanceof ObjectEntityInterface) { + $value = 'entity:' . $value->getUuid(); + } + + $named[$param->getName()] = $value; + } + + return $named; + + }//end named() + + /** + * Build the service with recording doubles driven by $this->plan. + * + * @return OrganizationSyncService + */ + private function service(): OrganizationSyncService { + $trace = &$this->trace; + $logger = new class($trace) extends AbstractLogger { + /** + * @param array $trace The shared trace. + */ + public function __construct(private array &$trace) { + } + + /** + * @param mixed $level The level. + * @param string|\Stringable $message The message. + * @param array $context The context. + * + * @return void + */ + public function log($level, string|\Stringable $message, array $context=[]): void { + foreach (['trace', 'file', 'line'] as $volatile) { + if (array_key_exists($volatile, $context) === true) { + $context[$volatile] = '(' . $volatile . ')'; + } + } + + $this->trace[] = ['log.' . $level, (string) $message, $context]; + } + }; + + $objects = $this->createMock(originalClassName: ObjectServiceInterface::class); + $objects->method('find')->willReturnCallback( + function (...$args) { + $this->trace[] = ['objects.find', self::named(method: 'find', args: $args)]; + $answer = array_shift($this->plan['find']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + $objects->method('searchObjects')->willReturnCallback( + function (...$args) { + $this->trace[] = ['objects.searchObjects', self::named(method: 'searchObjects', args: $args)]; + $answer = array_shift($this->plan['search']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + $objects->method('saveObject')->willReturnCallback( + function (...$args) { + $this->trace[] = ['objects.saveObject', self::named(method: 'saveObject', args: $args)]; + $answer = array_shift($this->plan['save']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return ($answer ?? $this->entity(object: ['saved' => true], uuid: 'saved-uuid')); + } + ); + + $mapper = $this->createMock(originalClassName: OrganisationMapper::class); + $mapper->method('findByUuid')->willReturnCallback( + function (string $uuid) { + $this->trace[] = ['mapper.findByUuid', $uuid]; + $answer = array_shift($this->plan['org']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + + $handler = $this->createMock(originalClassName: ContactPersonHandler::class); + $handler->method('isFirstContactForOrganization')->willReturnCallback( + function ($contact, $data) { + $this->trace[] = ['handler.isFirst', $contact->getUuid(), $data]; + return $this->plan['first'] ?? false; + } + ); + $handler->method('createUserAccount')->willReturnCallback( + function ($contact, $isFirst) { + $this->trace[] = ['handler.createUserAccount', $contact->getUuid(), $isFirst]; + return ($this->plan['user'] ?? null); + } + ); + $handler->method('addUserToOrganizationEntity')->willReturnCallback( + function (...$args) { + $args[0] = $args[0]->getUuid(); + $this->trace[] = ['handler.addUserToOrganizationEntity', $args]; + return true; + } + ); + + $settings = $this->createMock(originalClassName: SettingsService::class); + $settings->method('getVoorzieningenConfig')->willReturnCallback( + function () { + $this->trace[] = ['settings.getVoorzieningenConfig']; + $answer = ($this->plan['config'] ?? ['register' => 'reg-v', 'organisatie_schema' => 'org-schema']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + + // Built without the constructor: it type-hints IDBConnection, whose + // mock needs Doctrine, and the methods under test never touch it. + $reflection = new \ReflectionClass(OrganizationSyncService::class); + $service = $this->getMockBuilder(OrganizationSyncService::class) + ->disableOriginalConstructor() + ->onlyMethods(['processSpecificContactPerson']) + ->getMock(); + $service->method('processSpecificContactPerson')->willReturnCallback( + function ($contactObject) { + $this->trace[] = ['service.processSpecificContactPerson', $contactObject->getUuid(), $contactObject->getObject()]; + $answer = array_shift($this->plan['process']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return ($answer ?? ['contactPersonsProcessed' => 1, 'usersCreated' => 0, 'usersUpdated' => 1, 'errors' => []]); + } + ); + $properties = [ + 'logger' => $logger, + 'settingsService' => $settings, + 'contactpersonHandler' => $handler, + 'objectService' => $objects, + 'organisationMapper' => $mapper, + 'organisationService' => $this->organisationService(), + 'emailService' => $this->createMock(originalClassName: SymfonyEmailService::class), + ]; + foreach ($properties as $name => $value) { + $reflection->getProperty($name)->setValue($service, $value); + } + + return $service; + + }//end service() + + /** + * An organisation service double that records the organisations it creates. + * + * @return OrganisatieService + */ + private function organisationService(): OrganisatieService { + $service = $this->createMock(originalClassName: OrganisatieService::class); + $service->method('createOrganisationInOpenRegister')->willReturnCallback( + function (array $data) { + $this->trace[] = ['organisations.create', $data]; + return self::organisation(active: true); + } + ); + return $service; + + }//end organisationService() + + /** + * An organisation entity double. + * + * @param mixed $active What getActive() answers, or a Throwable it throws. + * + * @return Organisation + */ + private static function organisation(mixed $active): Organisation { + return new class($active) extends Organisation { + /** + * @param mixed $active The answer. + */ + public function __construct(private mixed $active) { + } + + /** + * @return mixed + */ + public function getActive(): mixed { + if ($this->active instanceof \Throwable) { + throw $this->active; + } + + return $this->active; + } + + /** + * @return string + */ + public function getName(): string { + return 'Org'; + } + + /** + * @param string $name The method. + * @param array $args The arguments. + * + * @return mixed + */ + public function __call(string $name, array $args) { + return null; + } + }; + + }//end organisation() + + /** + * A user double. + * + * @param string $uid The uid. + * + * @return IUser + */ + private function user(string $uid): IUser { + $user = $this->createMock(originalClassName: IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + + }//end user() + + /** + * The cases: organisation uuid and the doubles' answers. + * + * @return array + */ + public static function relatedCases(): array { + $config = ['register' => '5', 'contactpersoon_schema' => '9', 'organisatie_schema' => '7']; + return [ + 'no contact schema' => ['org-1', fn ($t) => ['config' => ['register' => '5']]], + 'no register' => ['org-1', fn ($t) => ['config' => ['contactpersoon_schema' => '9']]], + 'found by organization field' => [ + 'org-1', + fn ($t) => [ + 'config' => $config, + 'search' => [[ + $t->entity(['email' => 'a@x.nl', 'organization' => 'org-1'], 'c-a'), + $t->entity(['e-mailadres' => 'b@x.nl'], 'c-b'), + $t->entity(['voornaam' => 'C', 'organization' => 'org-1'], 'c-c'), + ], + ], + 'find' => [$t->entity(['email' => 'c@x.nl', 'organization' => 'org-1'], 'c-c')], + 'process' => [null, ['contactPersonsProcessed' => 1, 'usersCreated' => 1, 'usersUpdated' => 0, 'errors' => ['user b failed']], []], + ], + ], + 'found by organisation field' => [ + 'org-2', + fn ($t) => [ + 'config' => $config, + 'search' => [[], [$t->entity(['email' => 'd@x.nl', 'organization' => 'org-2'], 'c-d')]], + ], + ], + 'fallback to the contactpersonen array' => [ + 'org-3', + fn ($t) => [ + 'config' => $config, + 'search' => [[], []], + 'find' => [ + $t->entity(['contactpersonen' => ['c-e', '', 42, 'c-gone', 'c-null', 'c-f']], 'org-3'), + $t->entity(['email' => 'e@x.nl'], 'c-e'), + new \RuntimeException('contact gone'), + null, + $t->entity(['email' => 'f@x.nl', 'organization' => 'org-other'], 'c-f'), + ], + ], + ], + 'fallback org has no contacts' => [ + 'org-4', + fn ($t) => [ + 'config' => $config, + 'search' => [[], []], + 'find' => [$t->entity(['naam' => 'Org 4'], 'org-4')], + ], + ], + 'fallback org missing' => [ + 'org-5', + fn ($t) => [ + 'config' => $config, + 'search' => [[], []], + 'find' => [null], + ], + ], + 'fallback org lookup throws' => [ + 'org-6', + fn ($t) => [ + 'config' => $config, + 'search' => [[], []], + 'find' => [new \RuntimeException('org gone')], + ], + ], + 'refetch throws, process throws' => [ + 'org-7', + fn ($t) => [ + 'config' => $config, + 'search' => [[$t->entity(['organization' => 'org-7'], 'c-g'), $t->entity(['email' => 'h@x.nl'], 'c-h')]], + 'find' => [new \RuntimeException('refetch failed')], + 'process' => [new \RuntimeException('process failed'), null], + ], + ], + 'refetch returns null' => [ + 'org-8', + fn ($t) => [ + 'config' => $config, + 'search' => [[$t->entity(['organization' => 'org-8'], 'c-i')]], + 'find' => [null], + ], + ], + 'save of the organisation field throws' => [ + 'org-9', + fn ($t) => [ + 'config' => $config, + 'search' => [[$t->entity(['email' => 'j@x.nl'], 'c-j')]], + 'save' => [new \RuntimeException('save failed')], + ], + ], + 'search throws' => [ + 'org-10', + fn ($t) => [ + 'config' => $config, + 'search' => [new \RuntimeException('search failed')], + ], + ], + 'settings throw' => ['org-11', fn ($t) => ['config' => new \RuntimeException('settings down')]], + ]; + + }//end relatedCases() + + /** + * Each related contact case matches its recording. + * + * @param string $organizationUuid The organisation uuid. + * @param \Closure $plan Builds the doubles' answers. + * + * @return void + * + * @dataProvider relatedCases + */ + public function testRelatedContacts(string $organizationUuid, \Closure $plan): void { + $this->trace = []; + $this->plan = array_merge(['find' => [], 'save' => [], 'org' => [], 'search' => [], 'process' => []], $plan($this)); + $service = $this->service(); + $stats = ['contactPersonsProcessed' => 0, 'usersCreated' => 0, 'usersUpdated' => 0, 'errors' => ['earlier error']]; + $this->callRelated(service: $service, organizationUuid: $organizationUuid, stats: $stats); + $this->check(key: 'related: ' . $this->dataName(), value: ['stats' => $stats, 'trace' => $this->trace]); + + }//end testRelatedContacts() + + /** + * Call processRelatedContactPersons with the organisation uuid and the stats. + * + * @param OrganizationSyncService $service The service. + * @param string $organizationUuid The organisation uuid. + * @param array $stats The stats, updated in place. + * + * @return void + */ + private function callRelated(OrganizationSyncService $service, string $organizationUuid, array &$stats): void { + $method = new \ReflectionMethod(OrganizationSyncService::class, 'processRelatedContactPersons'); + $args = [$organizationUuid]; + if ($method->getNumberOfParameters() === 3) { + $args[] = $this->entity(object: ['naam' => 'Org'], uuid: $organizationUuid); + } + + $args[] = &$stats; + $method->invokeArgs($service, $args); + + }//end callRelated() + + /** + * Compare a case with the golden file, or record it. + * + * @param string $key The case. + * @param array $value What the case observed. + * + * @return void + */ + private function check(string $key, array $value): void { + $json = json_decode(json_encode($value, JSON_PARTIAL_OUTPUT_ON_ERROR), true); + $file = self::golden(); + $saved = []; + if (file_exists($file) === true) { + $saved = json_decode((string) file_get_contents($file), true); + } + + if (getenv('RECORD_GOLDEN') !== false) { + $saved[$key] = $json; + if (is_dir(dirname($file)) === false) { + mkdir(dirname($file), 0777, true); + } + + file_put_contents($file, json_encode($saved, (JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)) . "\n"); + $this->addToAssertionCount(count: 1); + return; + } + + $this->assertArrayHasKey(key: $key, array: $saved, message: 'No golden for ' . $key); + $this->assertSame(expected: $saved[$key], actual: $json); + + }//end check() + +}//end class diff --git a/tests/Unit/Service/golden/organizationSyncRelatedContacts.json b/tests/Unit/Service/golden/organizationSyncRelatedContacts.json new file mode 100644 index 000000000..57d807780 --- /dev/null +++ b/tests/Unit/Service/golden/organizationSyncRelatedContacts.json @@ -0,0 +1,1374 @@ +{ + "related: no contact schema": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-1" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.warning", + "[FLOW] Related contact processing skipped - configuration missing", + { + "organizationId": "org-1", + "register": "5", + "contactSchema": "" + } + ] + ] + }, + "related: no register": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-1" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.warning", + "[FLOW] Related contact processing skipped - configuration missing", + { + "organizationId": "org-1", + "register": "", + "contactSchema": "9" + } + ] + ] + }, + "related: found by organization field": { + "stats": { + "contactPersonsProcessed": 2, + "usersCreated": 1, + "usersUpdated": 1, + "errors": [ + "earlier error", + "user b failed" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-1" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-1", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-1", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-1", + "_limit": 500 + } + } + ], + [ + "log.info", + "👥 PROCESSING RELATED CONTACT PERSONS", + { + "app": "stackiq", + "organizationId": "org-1", + "contactCount": 3 + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-1", + "contactId": "c-a", + "contactEmail": "a@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-a", + { + "email": "a@x.nl", + "organization": "org-1" + } + ], + [ + "entity.setObject", + "c-b", + { + "e-mailadres": "b@x.nl", + "organization": "org-1" + } + ], + [ + "objects.saveObject", + { + "object": { + "e-mailadres": "b@x.nl", + "organization": "org-1" + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "c-b", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "[FLOW] Set missing organisatie field on related contact", + { + "contactId": "c-b", + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-1", + "contactId": "c-b", + "contactEmail": "b@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-b", + { + "e-mailadres": "b@x.nl", + "organization": "org-1" + } + ], + [ + "log.info", + "[FLOW] Contact data incomplete, re-fetching full object", + { + "organizationId": "org-1", + "contactId": "c-c" + } + ], + [ + "objects.find", + { + "id": "c-c", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-1", + "contactId": "c-c", + "contactEmail": "c@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-c", + { + "email": "c@x.nl", + "organization": "org-1" + } + ] + ] + }, + "related: found by organisation field": { + "stats": { + "contactPersonsProcessed": 1, + "usersCreated": 0, + "usersUpdated": 1, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-2" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-2", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-2", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-2", + "_limit": 500 + } + } + ], + [ + "log.info", + "[FLOW] Retrying search with organisation field", + { + "organizationId": "org-2", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-2" + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-2" + } + } + ], + [ + "log.info", + "👥 PROCESSING RELATED CONTACT PERSONS", + { + "app": "stackiq", + "organizationId": "org-2", + "contactCount": 1 + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-2", + "contactId": "c-d", + "contactEmail": "d@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-d", + { + "email": "d@x.nl", + "organization": "org-2" + } + ] + ] + }, + "related: fallback to the contactpersonen array": { + "stats": { + "contactPersonsProcessed": 2, + "usersCreated": 0, + "usersUpdated": 2, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-3" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-3", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-3", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-3", + "_limit": 500 + } + } + ], + [ + "log.info", + "[FLOW] Retrying search with organisation field", + { + "organizationId": "org-3", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-3" + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-3" + } + } + ], + [ + "log.info", + "[FLOW] No contacts found by organisatie field, checking org contactpersonen array", + { + "organizationId": "org-3" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "objects.find", + { + "id": "org-3", + "register": "5", + "schema": "7", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.info", + "[FLOW] Raw org contactpersonen array", + { + "organizationId": "org-3", + "contactpersonen": [ + "c-e", + "", + 42, + "c-gone", + "c-null", + "c-f" + ], + "count": 6 + } + ], + [ + "objects.find", + { + "id": "c-e", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "objects.find", + { + "id": "c-gone", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.warning", + "[FLOW] Could not fetch contact from contactpersonen array", + { + "organizationId": "org-3", + "contactUuid": "c-gone", + "exception": "contact gone" + } + ], + [ + "objects.find", + { + "id": "c-null", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "objects.find", + { + "id": "c-f", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.info", + "👥 PROCESSING RELATED CONTACT PERSONS", + { + "app": "stackiq", + "organizationId": "org-3", + "contactCount": 2 + } + ], + [ + "entity.setObject", + "c-e", + { + "email": "e@x.nl", + "organization": "org-3" + } + ], + [ + "objects.saveObject", + { + "object": { + "email": "e@x.nl", + "organization": "org-3" + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "c-e", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "[FLOW] Set missing organisatie field on related contact", + { + "contactId": "c-e", + "organizationUuid": "org-3" + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-3", + "contactId": "c-e", + "contactEmail": "e@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-e", + { + "email": "e@x.nl", + "organization": "org-3" + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-3", + "contactId": "c-f", + "contactEmail": "f@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-f", + { + "email": "f@x.nl", + "organization": "org-other" + } + ] + ] + }, + "related: fallback org has no contacts": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-4" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-4", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-4", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-4", + "_limit": 500 + } + } + ], + [ + "log.info", + "[FLOW] Retrying search with organisation field", + { + "organizationId": "org-4", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-4" + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-4" + } + } + ], + [ + "log.info", + "[FLOW] No contacts found by organisatie field, checking org contactpersonen array", + { + "organizationId": "org-4" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "objects.find", + { + "id": "org-4", + "register": "5", + "schema": "7", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.info", + "[FLOW] Raw org contactpersonen array", + { + "organizationId": "org-4", + "contactpersonen": [], + "count": 0 + } + ], + [ + "log.info", + "[FLOW] No related contact persons found (including contactpersonen fallback)", + { + "organizationId": "org-4" + } + ] + ] + }, + "related: fallback org missing": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-5" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-5", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-5", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-5", + "_limit": 500 + } + } + ], + [ + "log.info", + "[FLOW] Retrying search with organisation field", + { + "organizationId": "org-5", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-5" + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-5" + } + } + ], + [ + "log.info", + "[FLOW] No contacts found by organisatie field, checking org contactpersonen array", + { + "organizationId": "org-5" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "objects.find", + { + "id": "org-5", + "register": "5", + "schema": "7", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.info", + "[FLOW] Raw org contactpersonen array", + { + "organizationId": "org-5", + "contactpersonen": [], + "count": 0 + } + ], + [ + "log.info", + "[FLOW] No related contact persons found (including contactpersonen fallback)", + { + "organizationId": "org-5" + } + ] + ] + }, + "related: fallback org lookup throws": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-6" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-6", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-6", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-6", + "_limit": 500 + } + } + ], + [ + "log.info", + "[FLOW] Retrying search with organisation field", + { + "organizationId": "org-6", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-6" + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "_limit": 500, + "organisation": "org-6" + } + } + ], + [ + "log.info", + "[FLOW] No contacts found by organisatie field, checking org contactpersonen array", + { + "organizationId": "org-6" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "objects.find", + { + "id": "org-6", + "register": "5", + "schema": "7", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.warning", + "[FLOW] Could not re-fetch org for contactpersonen fallback", + { + "organizationId": "org-6", + "exception": "org gone" + } + ], + [ + "log.info", + "[FLOW] No related contact persons found (including contactpersonen fallback)", + { + "organizationId": "org-6" + } + ] + ] + }, + "related: refetch throws, process throws": { + "stats": { + "contactPersonsProcessed": 1, + "usersCreated": 0, + "usersUpdated": 1, + "errors": [ + "earlier error", + "Related contact c-g: process failed" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-7" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-7", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-7", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-7", + "_limit": 500 + } + } + ], + [ + "log.info", + "👥 PROCESSING RELATED CONTACT PERSONS", + { + "app": "stackiq", + "organizationId": "org-7", + "contactCount": 2 + } + ], + [ + "log.info", + "[FLOW] Contact data incomplete, re-fetching full object", + { + "organizationId": "org-7", + "contactId": "c-g" + } + ], + [ + "objects.find", + { + "id": "c-g", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.warning", + "[FLOW] Failed to re-fetch contact object", + { + "organizationId": "org-7", + "contactId": "c-g", + "exception": "refetch failed" + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-7", + "contactId": "c-g", + "contactEmail": "unknown" + } + ], + [ + "service.processSpecificContactPerson", + "c-g", + { + "organization": "org-7" + } + ], + [ + "log.error", + "[FLOW] Failed to process related contact person", + { + "organizationId": "org-7", + "contactId": "c-g", + "exception": "process failed" + } + ], + [ + "entity.setObject", + "c-h", + { + "email": "h@x.nl", + "organization": "org-7" + } + ], + [ + "objects.saveObject", + { + "object": { + "email": "h@x.nl", + "organization": "org-7" + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "c-h", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "[FLOW] Set missing organisatie field on related contact", + { + "contactId": "c-h", + "organizationUuid": "org-7" + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-7", + "contactId": "c-h", + "contactEmail": "h@x.nl" + } + ], + [ + "service.processSpecificContactPerson", + "c-h", + { + "email": "h@x.nl", + "organization": "org-7" + } + ] + ] + }, + "related: refetch returns null": { + "stats": { + "contactPersonsProcessed": 1, + "usersCreated": 0, + "usersUpdated": 1, + "errors": [ + "earlier error" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-8" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-8", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-8", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-8", + "_limit": 500 + } + } + ], + [ + "log.info", + "👥 PROCESSING RELATED CONTACT PERSONS", + { + "app": "stackiq", + "organizationId": "org-8", + "contactCount": 1 + } + ], + [ + "log.info", + "[FLOW] Contact data incomplete, re-fetching full object", + { + "organizationId": "org-8", + "contactId": "c-i" + } + ], + [ + "objects.find", + { + "id": "c-i", + "register": "5", + "schema": "9", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.info", + "[FLOW] Processing related contact person", + { + "organizationId": "org-8", + "contactId": "c-i", + "contactEmail": "unknown" + } + ], + [ + "service.processSpecificContactPerson", + "c-i", + { + "organization": "org-8" + } + ] + ] + }, + "related: save of the organisation field throws": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error", + "Related contact c-j: save failed" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-9" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-9", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-9", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-9", + "_limit": 500 + } + } + ], + [ + "log.info", + "👥 PROCESSING RELATED CONTACT PERSONS", + { + "app": "stackiq", + "organizationId": "org-9", + "contactCount": 1 + } + ], + [ + "entity.setObject", + "c-j", + { + "email": "j@x.nl", + "organization": "org-9" + } + ], + [ + "objects.saveObject", + { + "object": { + "email": "j@x.nl", + "organization": "org-9" + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "c-j", + "silent": true, + "_validation": false + } + ], + [ + "log.error", + "[FLOW] Failed to process related contact person", + { + "organizationId": "org-9", + "contactId": "c-j", + "exception": "save failed" + } + ] + ] + }, + "related: search throws": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error", + "Failed to process related contact persons: search failed" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-10" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.info", + "[FLOW] Searching for related contact persons with organisatie field", + { + "organizationId": "org-10", + "register": "5", + "contactSchema": "9", + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-10", + "_limit": 500 + } + } + ], + [ + "objects.searchObjects", + { + "query": { + "@self": { + "register": 5, + "schema": 9 + }, + "organization": "org-10", + "_limit": 500 + } + } + ], + [ + "log.error", + "[FLOW] Failed to process related contact persons", + { + "organizationId": "org-10", + "exception": "search failed", + "file": "(file)", + "line": "(line)" + } + ] + ] + }, + "related: settings throw": { + "stats": { + "contactPersonsProcessed": 0, + "usersCreated": 0, + "usersUpdated": 0, + "errors": [ + "earlier error", + "Failed to process related contact persons: settings down" + ] + }, + "trace": [ + [ + "log.debug", + "Finding related contact persons", + { + "app": "stackiq", + "organizationId": "org-11" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.error", + "[FLOW] Failed to process related contact persons", + { + "organizationId": "org-11", + "exception": "settings down", + "file": "(file)", + "line": "(line)" + } + ] + ] + } +} From 0d62d74f1953f43391d705d404a6f255f608f556 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 15:01:50 +0100 Subject: [PATCH 2/7] refactor(organization-sync): split the related contact person sync into find, fetch and process steps processRelatedContactPersons drops its unused organisation object parameter and hands the work to findRelatedContacts, contactsFromOrganisationList, fetchListedContact, processRelatedContact and completeRelatedContact, each with at most two parameters (the sync run array, stats held by reference). Lookups, saves, log lines and stats are unchanged (golden of the previous commit). Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/OrganizationSyncService.php | 518 ++++++++++++++---------- 1 file changed, 293 insertions(+), 225 deletions(-) diff --git a/lib/Service/OrganizationSyncService.php b/lib/Service/OrganizationSyncService.php index f8dd147d8..20d41cc61 100644 --- a/lib/Service/OrganizationSyncService.php +++ b/lib/Service/OrganizationSyncService.php @@ -1761,11 +1761,7 @@ public function processSpecificOrganization($organizationObject): array { ] ); - $this->processRelatedContactPersons( - organizationUuid: $organizationUuid, - organizationObject: $organizationObject, - stats: $stats - ); + $this->processRelatedContactPersons(organizationUuid: $organizationUuid, stats: $stats); }//end if if (empty($organisationEntity) === true) { @@ -1956,14 +1952,11 @@ private function processNestedContactPersons($organizationObject, array &$stats) * Process related contactpersoon objects that have this organization in their organisation property * * @param string $organizationUuid The organization UUID. - * @param \OCA\OpenRegister\Db\ObjectEntity $organizationObject The organization object. - * @param array $stats The statistics array to update. + * @param array $stats The statistics array to update. * * @return void - * - * @SuppressWarnings(PHPMD.UnusedFormalParameter) $organizationObject reserved for future contact person enrichment */ - private function processRelatedContactPersons(string $organizationUuid, $organizationObject, array &$stats): void { + private function processRelatedContactPersons(string $organizationUuid, array &$stats): void { try { $this->logger->debug( 'Finding related contact persons', @@ -1975,274 +1968,349 @@ private function processRelatedContactPersons(string $organizationUuid, $organiz // Get configuration. $voorzieningenConfig = $this->settingsService->getVoorzieningenConfig(); - $register = ($voorzieningenConfig['register'] ?? ''); - $contactSchema = ($voorzieningenConfig['contactpersoon_schema'] ?? ''); + $run = [ + 'organizationUuid' => $organizationUuid, + 'register' => ($voorzieningenConfig['register'] ?? ''), + 'contactSchema' => ($voorzieningenConfig['contactpersoon_schema'] ?? ''), + ]; - if (empty($register) === true || empty($contactSchema) === true) { + if (empty($run['register']) === true || empty($run['contactSchema']) === true) { $this->logger->warning( '[FLOW] Related contact processing skipped - configuration missing', [ 'organizationId' => $organizationUuid, - 'register' => $register, - 'contactSchema' => $contactSchema, + 'register' => $run['register'], + 'contactSchema' => $run['contactSchema'], ] ); return; } - // Find all contactpersoon objects that have this organization in their organisation property. + $relatedContacts = $this->findRelatedContacts(run: $run); + if (empty($relatedContacts) === true) { + return; + } - // Search for contactpersoon objects with this organization reference. - // Try both 'organization' and 'organisation' field names. - $query = [ - '@self' => [ - 'register' => (int)$register, - 'schema' => (int)$contactSchema, - ], - 'organization' => $organizationUuid, - '_limit' => 500, - ]; + $this->logger->info( + '👥 PROCESSING RELATED CONTACT PERSONS', + [ + 'app' => 'stackiq', + 'organizationId' => $organizationUuid, + 'contactCount' => count($relatedContacts), + ] + ); + + $run['stats'] = &$stats; + foreach ($relatedContacts as $contactObject) { + $this->processRelatedContact(contactObject: $contactObject, run: $run); + } + } catch (\Exception $e) { + $this->logger->error( + '[FLOW] Failed to process related contact persons', + [ + 'organizationId' => $organizationUuid, + 'exception' => $e->getMessage(), + 'file' => $e->getFile(), + 'line' => $e->getLine(), + ] + ); + $stats['errors'][] = 'Failed to process related contact persons: ' . $e->getMessage(); + }//end try + + }//end processRelatedContactPersons() + + /** + * Find the contactpersoon objects of an organisation: by their organization field, then by + * their organisation field, then from the organisation's own contactpersonen array. + * + * @param array $run The sync run: `organizationUuid`, `register` and `contactSchema`. + * + * @return array The related contact objects, empty when none are found + */ + private function findRelatedContacts(array $run): array { + $organizationUuid = $run['organizationUuid']; + + // Search for contactpersoon objects with this organization reference. + // Try both 'organization' and 'organisation' field names. + $query = [ + '@self' => [ + 'register' => (int)$run['register'], + 'schema' => (int)$run['contactSchema'], + ], + 'organization' => $organizationUuid, + '_limit' => 500, + ]; + + $this->logger->info( + '[FLOW] Searching for related contact persons with organisatie field', + [ + 'organizationId' => $organizationUuid, + 'register' => $run['register'], + 'contactSchema' => $run['contactSchema'], + 'query' => $query, + ] + ); + + $relatedContacts = $this->objectService->searchObjects($query); + + // If not found, try with 'organisation' field. + if (empty($relatedContacts) === true) { + $query['organisation'] = $organizationUuid; + unset($query['organization']); $this->logger->info( - '[FLOW] Searching for related contact persons with organisatie field', + '[FLOW] Retrying search with organisation field', [ 'organizationId' => $organizationUuid, - 'register' => $register, - 'contactSchema' => $contactSchema, 'query' => $query, ] ); $relatedContacts = $this->objectService->searchObjects($query); + } - // If not found, try with 'organisation' field. - if (empty($relatedContacts) === true) { - $query['organisation'] = $organizationUuid; - unset($query['organization']); - - $this->logger->info( - '[FLOW] Retrying search with organisation field', - [ - 'organizationId' => $organizationUuid, - 'query' => $query, - ] - ); + if (empty($relatedContacts) === false) { + return $relatedContacts; + } - $relatedContacts = $this->objectService->searchObjects($query); - } + // Fallback: fetch contacts by UUID from the org's contactpersonen array. + // This handles the case where contacts were linked via inversedBy (org → contact). + // but the contact's own organisatie field was never set. + $this->logger->info( + '[FLOW] No contacts found by organisatie field, checking org contactpersonen array', + ['organizationId' => $organizationUuid] + ); - if (empty($relatedContacts) === true) { - // Fallback: fetch contacts by UUID from the org's contactpersonen array. - // This handles the case where contacts were linked via inversedBy (org → contact). - // but the contact's own organisatie field was never set. - $this->logger->info( - '[FLOW] No contacts found by organisatie field, checking org contactpersonen array', - ['organizationId' => $organizationUuid] - ); + $relatedContacts = $this->contactsFromOrganisationList(run: $run); + if (empty($relatedContacts) === true) { + $this->logger->info( + '[FLOW] No related contact persons found (including contactpersonen fallback)', + ['organizationId' => $organizationUuid] + ); + } - // Re-fetch the org object without _extend to get raw UUIDs. - try { - $voorzieningenConfig2 = $this->settingsService->getVoorzieningenConfig(); - $orgRegister = ($voorzieningenConfig2['register'] ?? ''); - $orgSchema = ($voorzieningenConfig2['organisatie_schema'] ?? ''); - - $rawOrgObject = $this->objectService->find( - id: $organizationUuid, - register: $orgRegister, - schema: $orgSchema, - _rbac: false, - _multitenancy: false - ); + return $relatedContacts; + }//end findRelatedContacts() - $rawOrgData = []; - if ($rawOrgObject !== null) { - $rawOrgData = $rawOrgObject->getObject(); - } + /** + * The contact objects listed in the organisation's raw contactpersonen array. + * + * @param array $run The sync run: `organizationUuid`, `register` and `contactSchema`. + * + * @return array The contact objects that could be fetched + */ + private function contactsFromOrganisationList(array $run): array { + $organizationUuid = $run['organizationUuid']; + $contacts = []; - $contactUuids = ($rawOrgData['contactpersonen'] ?? []); + // Re-fetch the org object without _extend to get raw UUIDs. + try { + $voorzieningenConfig = $this->settingsService->getVoorzieningenConfig(); + $rawOrgObject = $this->objectService->find( + id: $organizationUuid, + register: ($voorzieningenConfig['register'] ?? ''), + schema: ($voorzieningenConfig['organisatie_schema'] ?? ''), + _rbac: false, + _multitenancy: false + ); - $this->logger->info( - '[FLOW] Raw org contactpersonen array', - [ - 'organizationId' => $organizationUuid, - 'contactpersonen' => $contactUuids, - 'count' => count($contactUuids), - ] - ); + $rawOrgData = []; + if ($rawOrgObject !== null) { + $rawOrgData = $rawOrgObject->getObject(); + } - if (empty($contactUuids) === false) { - foreach ($contactUuids as $contactUuid) { - if (is_string($contactUuid) === false || empty($contactUuid) === true) { - continue; - } + $contactUuids = ($rawOrgData['contactpersonen'] ?? []); - try { - $contactObj = $this->objectService->find( - id: $contactUuid, - register: $register, - schema: $contactSchema, - _rbac: false, - _multitenancy: false - ); - if ($contactObj !== null) { - $relatedContacts[] = $contactObj; - } - } catch (\Exception $fetchEx) { - $this->logger->warning( - '[FLOW] Could not fetch contact from contactpersonen array', - [ - 'organizationId' => $organizationUuid, - 'contactUuid' => $contactUuid, - 'exception' => $fetchEx->getMessage(), - ] - ); - }//end try - }//end foreach - }//end if - } catch (\Exception $orgEx) { - $this->logger->warning( - '[FLOW] Could not re-fetch org for contactpersonen fallback', - [ - 'organizationId' => $organizationUuid, - 'exception' => $orgEx->getMessage(), - ] - ); - }//end try + $this->logger->info( + '[FLOW] Raw org contactpersonen array', + [ + 'organizationId' => $organizationUuid, + 'contactpersonen' => $contactUuids, + 'count' => count($contactUuids), + ] + ); - if (empty($relatedContacts) === true) { - $this->logger->info( - '[FLOW] No related contact persons found (including contactpersonen fallback)', - ['organizationId' => $organizationUuid] - ); - return; + foreach ($contactUuids as $contactUuid) { + if (is_string($contactUuid) === false || empty($contactUuid) === true) { + continue; } - }//end if - $this->logger->info( - '👥 PROCESSING RELATED CONTACT PERSONS', + $contactObj = $this->fetchListedContact(contactUuid: $contactUuid, run: $run); + if ($contactObj !== null) { + $contacts[] = $contactObj; + } + } + } catch (\Exception $orgEx) { + $this->logger->warning( + '[FLOW] Could not re-fetch org for contactpersonen fallback', [ - 'app' => 'stackiq', 'organizationId' => $organizationUuid, - 'contactCount' => count($relatedContacts), + 'exception' => $orgEx->getMessage(), ] ); + }//end try - foreach ($relatedContacts as $contactObject) { - try { - $contactUuid = $contactObject->getUuid(); - $contactData = $contactObject->getObject(); + return $contacts; + }//end contactsFromOrganisationList() - // Check if contact data is complete (has email). - $email = ($contactData['email'] ?? $contactData['e-mailadres'] ?? ''); - if (empty($email) === true && empty($contactUuid) === false) { - // Re-fetch the full contact object if email is missing. - $this->logger->info( - '[FLOW] Contact data incomplete, re-fetching full object', - [ - 'organizationId' => $organizationUuid, - 'contactId' => $contactUuid, - ] - ); + /** + * Fetch one contact listed in an organisation's contactpersonen array. + * + * @param string $contactUuid The contact uuid. + * @param array $run The sync run: `organizationUuid`, `register` and `contactSchema`. + * + * @return object|null The contact object, or null when it cannot be fetched + */ + private function fetchListedContact(string $contactUuid, array $run): ?object { + try { + return $this->objectService->find( + id: $contactUuid, + register: $run['register'], + schema: $run['contactSchema'], + _rbac: false, + _multitenancy: false + ); + } catch (\Exception $fetchEx) { + $this->logger->warning( + '[FLOW] Could not fetch contact from contactpersonen array', + [ + 'organizationId' => $run['organizationUuid'], + 'contactUuid' => $contactUuid, + 'exception' => $fetchEx->getMessage(), + ] + ); + return null; + }//end try + }//end fetchListedContact() - try { - $fullContactObject = $this->objectService->find( - id: $contactUuid, - register: $register, - schema: $contactSchema, - _rbac: false, - _multitenancy: false - ); - if ($fullContactObject !== null) { - $contactObject = $fullContactObject; - $contactData = $contactObject->getObject(); - $email = ($contactData['email'] ?? $contactData['e-mailadres'] ?? ''); - } - } catch (\Exception $e) { - $this->logger->warning( - '[FLOW] Failed to re-fetch contact object', - [ - 'organizationId' => $organizationUuid, - 'contactId' => $contactUuid, - 'exception' => $e->getMessage(), - ] - ); - }//end try - }//end if + /** + * Process one related contact: complete it, give it its organisation, then sync it. + * + * @param object $contactObject The contact object. + * @param array $run The sync run: `organizationUuid`, `register`, `contactSchema` and, + * held by reference, `stats`. + * + * @return void + */ + private function processRelatedContact(object $contactObject, array $run): void { + $organizationUuid = $run['organizationUuid']; + try { + $contactUuid = $contactObject->getUuid(); + $contactObject = $this->completeRelatedContact(contactObject: $contactObject, run: $run); + $contactData = $contactObject->getObject(); + $email = ($contactData['email'] ?? $contactData['e-mailadres'] ?? ''); - // Ensure the contact has the organisatie field set before processing. - // (may be missing when contacts were linked via inversedBy from the org side). - if (empty($contactData['organization']) === true) { - $contactData['organization'] = $organizationUuid; - $contactObject->setObject($contactData); - // Published contract instead of OpenRegister's Db layer; see - // ContactpersonenController for the reasoning. - $this->objectService->saveObject( - object: $contactData, - register: $contactObject->getRegister(), - schema: $contactObject->getSchema(), - uuid: $contactObject->getUuid(), - silent: true, - _validation: false - ); - $this->logger->info( - '[FLOW] Set missing organisatie field on related contact', - [ - 'contactId' => $contactUuid, - 'organizationUuid' => $organizationUuid, - ] - ); - } + // Ensure the contact has the organisatie field set before processing. + // (may be missing when contacts were linked via inversedBy from the org side). + if (empty($contactData['organization']) === true) { + $contactData['organization'] = $organizationUuid; + $contactObject->setObject($contactData); + // Published contract instead of OpenRegister's Db layer; see + // ContactpersonenController for the reasoning. + $this->objectService->saveObject( + object: $contactData, + register: $contactObject->getRegister(), + schema: $contactObject->getSchema(), + uuid: $contactObject->getUuid(), + silent: true, + _validation: false + ); + $this->logger->info( + '[FLOW] Set missing organisatie field on related contact', + [ + 'contactId' => $contactUuid, + 'organizationUuid' => $organizationUuid, + ] + ); + }//end if - $contactEmailValue = $email; - if ($contactEmailValue === false || $contactEmailValue === '' || $contactEmailValue === null) { - $contactEmailValue = 'unknown'; - } + if ($email === false || $email === '' || $email === null) { + $email = 'unknown'; + } - $this->logger->info( - '[FLOW] Processing related contact person', - [ - 'organizationId' => $organizationUuid, - 'contactId' => $contactUuid, - 'contactEmail' => $contactEmailValue, - ] - ); + $this->logger->info( + '[FLOW] Processing related contact person', + [ + 'organizationId' => $organizationUuid, + 'contactId' => $contactUuid, + 'contactEmail' => $email, + ] + ); - // Process the contact person through processSpecificContactPerson. - $contactStats = $this->processSpecificContactPerson(contactObject: $contactObject); + // Process the contact person through processSpecificContactPerson. + $contactStats = $this->processSpecificContactPerson(contactObject: $contactObject); - // Merge stats. - $stats['contactPersonsProcessed'] += ($contactStats['contactPersonsProcessed'] ?? 0); - $stats['usersCreated'] += ($contactStats['usersCreated'] ?? 0); - $stats['usersUpdated'] += ($contactStats['usersUpdated'] ?? 0); - if (empty($contactStats['errors']) === false) { - $stats['errors'] = array_merge($stats['errors'], $contactStats['errors']); - } - } catch (\Exception $e) { - $this->logger->error( - '[FLOW] Failed to process related contact person', - [ - 'organizationId' => $organizationUuid, - 'contactId' => $contactObject->getUuid(), - 'exception' => $e->getMessage(), - ] - ); - $stats['errors'][] = "Related contact {$contactObject->getUuid()}: " . $e->getMessage(); - }//end try - }//end foreach + // Merge stats. + $run['stats']['contactPersonsProcessed'] += ($contactStats['contactPersonsProcessed'] ?? 0); + $run['stats']['usersCreated'] += ($contactStats['usersCreated'] ?? 0); + $run['stats']['usersUpdated'] += ($contactStats['usersUpdated'] ?? 0); + if (empty($contactStats['errors']) === false) { + $run['stats']['errors'] = array_merge($run['stats']['errors'], $contactStats['errors']); + } } catch (\Exception $e) { $this->logger->error( - '[FLOW] Failed to process related contact persons', + '[FLOW] Failed to process related contact person', [ 'organizationId' => $organizationUuid, + 'contactId' => $contactObject->getUuid(), 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), ] ); - $stats['errors'][] = 'Failed to process related contact persons: ' . $e->getMessage(); + $run['stats']['errors'][] = "Related contact {$contactObject->getUuid()}: " . $e->getMessage(); }//end try + }//end processRelatedContact() - }//end processRelatedContactPersons() + /** + * A related contact with its full data: one without an email is fetched again by uuid. + * + * @param object $contactObject The contact object as found. + * @param array $run The sync run: `organizationUuid`, `register` and `contactSchema`. + * + * @return object The re-fetched contact, or the one given when it is complete or cannot be fetched + */ + private function completeRelatedContact(object $contactObject, array $run): object { + $contactUuid = $contactObject->getUuid(); + $contactData = $contactObject->getObject(); + + // Check if contact data is complete (has email). + $email = ($contactData['email'] ?? $contactData['e-mailadres'] ?? ''); + if (empty($email) === false || empty($contactUuid) === true) { + return $contactObject; + } + + // Re-fetch the full contact object if email is missing. + $this->logger->info( + '[FLOW] Contact data incomplete, re-fetching full object', + [ + 'organizationId' => $run['organizationUuid'], + 'contactId' => $contactUuid, + ] + ); + + try { + $fullContactObject = $this->objectService->find( + id: $contactUuid, + register: $run['register'], + schema: $run['contactSchema'], + _rbac: false, + _multitenancy: false + ); + if ($fullContactObject !== null) { + return $fullContactObject; + } + } catch (\Exception $e) { + $this->logger->warning( + '[FLOW] Failed to re-fetch contact object', + [ + 'organizationId' => $run['organizationUuid'], + 'contactId' => $contactUuid, + 'exception' => $e->getMessage(), + ] + ); + }//end try + + return $contactObject; + }//end completeRelatedContact() /** * Create or update a contact person object and its user account. From b3a48c0fac8590c768eb9e17731b4b04825bee7b Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 15:04:17 +0100 Subject: [PATCH 3/7] test(organization-sync): golden for ensuring the organisation entity of an organisatie object The OrganisationMapper stub gains findBySlug, as on OpenRegister's mapper. Assisted-by: ClaudeCode:claude-opus-5-5 --- tests/Stubs/Db/OrganisationMapper.php | 9 + .../OrganizationSyncEntityGoldenTest.php | 636 ++++++ .../golden/organizationSyncEntity.json | 1852 +++++++++++++++++ 3 files changed, 2497 insertions(+) create mode 100644 tests/Unit/Service/OrganizationSyncEntityGoldenTest.php create mode 100644 tests/Unit/Service/golden/organizationSyncEntity.json diff --git a/tests/Stubs/Db/OrganisationMapper.php b/tests/Stubs/Db/OrganisationMapper.php index 140cdefb0..e76182296 100644 --- a/tests/Stubs/Db/OrganisationMapper.php +++ b/tests/Stubs/Db/OrganisationMapper.php @@ -31,6 +31,15 @@ abstract class OrganisationMapper { */ abstract public function findByUuid(string $uuid): Organisation; + /** + * Find an organisation by slug. + * + * @param string $slug The organisation slug. + * + * @return Organisation + */ + abstract public function findBySlug(string $slug): Organisation; + /** * Persist an organisation entity. * diff --git a/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php b/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php new file mode 100644 index 000000000..4d9a86a69 --- /dev/null +++ b/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php @@ -0,0 +1,636 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/specs/organization-sync/spec.md + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Service; + +use OCA\OpenRegister\Contract\ObjectEntityInterface; +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\OpenRegister\Db\Organisation; +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\Stackiq\Service\OrganisatieService; +use OCA\Stackiq\Service\OrganizationSyncService; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Service\Stackiq\ContactPersonHandler; +use OCA\Stackiq\Service\SymfonyEmailService; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IUser; +use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; + +/** + * Golden cases for the organisation entity sync. + * + * @covers \OCA\Stackiq\Service\OrganizationSyncService + * + * @spec openspec/specs/organization-sync/spec.md + */ +class OrganizationSyncEntityGoldenTest extends TestCase { + + /** + * Everything the doubles saw, in order. + * + * @var array + */ + private array $trace = []; + + /** + * Per-case behaviour of the doubles. + * + * @var array + */ + private array $plan = []; + + /** + * The golden file. + * + * @return string + */ + private static function golden(): string { + return __DIR__ . '/golden/organizationSyncEntity.json'; + + }//end golden() + + /** + * A contact or organisation object double that records its writes. + * + * @param array $object The object data. + * @param string $uuid The uuid. + * + * @return ObjectEntityInterface + */ + private function entity(array $object, string $uuid): ObjectEntityInterface { + $trace = &$this->trace; + return new class($object, $uuid, $trace) implements ObjectEntityInterface { + /** + * The owner set on the entity. + * + * @var string|null + */ + private ?string $owner = null; + + /** + * The organisation set on the entity. + * + * @var string|null + */ + private ?string $organisation = 'org-of-entity'; + + /** + * Build the double. + * + * @param array $object The object data. + * @param string $uuid The uuid. + * @param array $trace The shared trace. + */ + public function __construct(private array $object, private string $uuid, private array &$trace) { + } + + /** + * @return mixed + */ + public function getId() { + return 7; + } + + /** + * @return string|null + */ + public function getUuid(): ?string { + return $this->uuid; + } + + /** + * @return array + */ + public function getObject(): array { + return $this->object; + } + + /** + * @return string|null + */ + public function getRegister(): ?string { + return 'reg-entity'; + } + + /** + * @return string|null + */ + public function getSchema(): ?string { + return 'schema-entity'; + } + + /** + * @return string|null + */ + public function getOrganisation(): ?string { + return $this->organisation; + } + + /** + * @return string|null + */ + public function getOwner(): ?string { + return $this->owner; + } + + /** + * @param mixed $object The data. + * + * @return void + */ + public function setObject($object=null) { + $this->trace[] = ['entity.setObject', $this->uuid, $object]; + $this->object = $object; + } + + /** + * @param string|null $owner The owner. + * + * @return void + */ + public function setOwner(?string $owner): void { + $this->trace[] = ['entity.setOwner', $this->uuid, $owner]; + $this->owner = $owner; + } + + /** + * @param string|null $organisation The organisation. + * + * @return void + */ + public function setOrganisation(?string $organisation): void { + $this->trace[] = ['entity.setOrganisation', $this->uuid, $organisation]; + $this->organisation = $organisation; + } + + /** + * @return mixed + */ + public function jsonSerialize(): mixed { + return $this->object; + } + + /** + * @param string $name The method. + * @param array $args The arguments. + * + * @return mixed + */ + public function __call(string $name, array $args) { + $this->trace[] = ['entity.' . $name, $this->uuid, $args]; + return null; + } + }; + + }//end entity() + + /** + * Arguments by parameter name, leaving out the ones left at their default. + * + * @param string $method The ObjectServiceInterface method. + * @param array $args The positional arguments the double received. + * + * @return array + */ + private static function named(string $method, array $args): array { + $named = []; + foreach ((new \ReflectionMethod(ObjectServiceInterface::class, $method))->getParameters() as $i => $param) { + if (array_key_exists($i, $args) === false) { + continue; + } + + if ($param->isDefaultValueAvailable() === true && $param->getDefaultValue() === $args[$i]) { + continue; + } + + $value = $args[$i]; + if ($value instanceof ObjectEntityInterface) { + $value = 'entity:' . $value->getUuid(); + } + + $named[$param->getName()] = $value; + } + + return $named; + + }//end named() + + /** + * Build the service with recording doubles driven by $this->plan. + * + * @return OrganizationSyncService + */ + private function service(): OrganizationSyncService { + $trace = &$this->trace; + $logger = new class($trace) extends AbstractLogger { + /** + * @param array $trace The shared trace. + */ + public function __construct(private array &$trace) { + } + + /** + * @param mixed $level The level. + * @param string|\Stringable $message The message. + * @param array $context The context. + * + * @return void + */ + public function log($level, string|\Stringable $message, array $context=[]): void { + foreach (['trace', 'file', 'line'] as $volatile) { + if (array_key_exists($volatile, $context) === true) { + $context[$volatile] = '(' . $volatile . ')'; + } + } + + $this->trace[] = ['log.' . $level, (string) $message, $context]; + } + }; + + $objects = $this->createMock(originalClassName: ObjectServiceInterface::class); + $objects->method('find')->willReturnCallback( + function (...$args) { + $this->trace[] = ['objects.find', self::named(method: 'find', args: $args)]; + $answer = array_shift($this->plan['find']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + $objects->method('saveObject')->willReturnCallback( + function (...$args) { + $this->trace[] = ['objects.saveObject', self::named(method: 'saveObject', args: $args)]; + $answer = array_shift($this->plan['save']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return ($answer ?? $this->entity(object: ['saved' => true], uuid: 'saved-uuid')); + } + ); + + $mapper = $this->createMock(originalClassName: OrganisationMapper::class); + $mapper->method('findBySlug')->willReturnCallback( + function (string $slug) { + $this->trace[] = ['mapper.findBySlug', $slug]; + $answer = array_shift($this->plan['slug']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + $mapper->method('save')->willReturnCallback( + function ($entity) { + $this->trace[] = ['mapper.save', $entity->getUuid(), $entity->getActive()]; + return $entity; + } + ); + $mapper->method('findByUuid')->willReturnCallback( + function (string $uuid) { + $this->trace[] = ['mapper.findByUuid', $uuid]; + $answer = array_shift($this->plan['org']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + + $handler = $this->createMock(originalClassName: ContactPersonHandler::class); + $handler->method('isFirstContactForOrganization')->willReturnCallback( + function ($contact, $data) { + $this->trace[] = ['handler.isFirst', $contact->getUuid(), $data]; + return $this->plan['first'] ?? false; + } + ); + $handler->method('createUserAccount')->willReturnCallback( + function ($contact, $isFirst) { + $this->trace[] = ['handler.createUserAccount', $contact->getUuid(), $isFirst]; + return ($this->plan['user'] ?? null); + } + ); + $handler->method('addUserToOrganizationEntity')->willReturnCallback( + function (...$args) { + $args[0] = $args[0]->getUuid(); + $this->trace[] = ['handler.addUserToOrganizationEntity', $args]; + return true; + } + ); + + $settings = $this->createMock(originalClassName: SettingsService::class); + $settings->method('getVoorzieningenConfig')->willReturnCallback( + function () { + $this->trace[] = ['settings.getVoorzieningenConfig']; + $answer = ($this->plan['config'] ?? ['register' => 'reg-v', 'organisatie_schema' => 'org-schema']); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + + // Built without the constructor: it type-hints IDBConnection, whose + // mock needs Doctrine, and the methods under test never touch it. + $reflection = new \ReflectionClass(OrganizationSyncService::class); + $service = $reflection->newInstanceWithoutConstructor(); + $properties = [ + 'logger' => $logger, + 'settingsService' => $settings, + 'contactpersonHandler' => $handler, + 'objectService' => $objects, + 'organisationMapper' => $mapper, + 'organisationService' => $this->organisationService(), + 'emailService' => $this->emailService(), + ]; + foreach ($properties as $name => $value) { + $reflection->getProperty($name)->setValue($service, $value); + } + + return $service; + + }//end service() + + /** + * An organisation service double that records the organisations it creates. + * + * @return OrganisatieService + */ + private function organisationService(): OrganisatieService { + $service = $this->createMock(originalClassName: OrganisatieService::class); + $service->method('createOrganisationInOpenRegister')->willReturnCallback( + function (array $data) { + $this->trace[] = ['organisations.create', $data]; + return array_key_exists('created', $this->plan) === true ? $this->plan['created'] : $this->orgEntity(uuid: 'new-entity', active: true); + } + ); + return $service; + + }//end organisationService() + + /** + * An organisation entity double. + * + * @param mixed $active What getActive() answers, or a Throwable it throws. + * + * @return Organisation + */ + private static function organisation(mixed $active): Organisation { + return new class($active) extends Organisation { + /** + * @param mixed $active The answer. + */ + public function __construct(private mixed $active) { + } + + /** + * @return mixed + */ + public function getActive(): mixed { + if ($this->active instanceof \Throwable) { + throw $this->active; + } + + return $this->active; + } + + /** + * @return string + */ + public function getName(): string { + return 'Org'; + } + + /** + * @param string $name The method. + * @param array $args The arguments. + * + * @return mixed + */ + public function __call(string $name, array $args) { + return null; + } + }; + + }//end organisation() + + /** + * A user double. + * + * @param string $uid The uid. + * + * @return IUser + */ + private function user(string $uid): IUser { + $user = $this->createMock(originalClassName: IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + + }//end user() + + /** + * An e-mail service double that records the mails and answers per plan. + * + * @return SymfonyEmailService + */ + private function emailService(): SymfonyEmailService { + $mail = $this->createMock(originalClassName: SymfonyEmailService::class); + foreach (['sendOrganizationRegistrationEmail', 'sendOrganizationActivationEmail'] as $method) { + $mail->method($method)->willReturnCallback( + function (array $data) use ($method) { + $this->trace[] = ['mail.' . $method, $data['name'] ?? null]; + $answer = ($this->plan['mail'] ?? true); + if ($answer instanceof \Throwable) { + throw $answer; + } + + return $answer; + } + ); + } + + return $mail; + + }//end emailService() + + /** + * An organisation entity double that records its writes. + * + * @param string $uuid The entity uuid. + * @param bool $active Whether it is active. + * + * @return Organisation + */ + public function orgEntity(string $uuid, bool $active): Organisation { + $trace = &$this->trace; + return new class($uuid, $active, $trace) extends Organisation { + /** + * @param string $entityUuid The uuid. + * @param bool $isActive Whether it is active. + * @param array $trace The shared trace. + */ + public function __construct(private string $entityUuid, private bool $isActive, private array &$trace) { + } + + /** + * @return int|null + */ + public function getId(): ?int { + return 41; + } + + /** + * @return string + */ + public function getUuid(): string { + return $this->entityUuid; + } + + /** + * @param string $uuid The uuid. + * + * @return void + */ + public function setUuid(string $uuid): void { + $this->trace[] = ['entity.setUuid', $this->entityUuid, $uuid]; + $this->entityUuid = $uuid; + } + + /** + * @return bool + */ + public function getActive(): bool { + return $this->isActive; + } + + /** + * @param bool $active Whether it is active. + * + * @return void + */ + public function setActive(bool $active): void { + $this->trace[] = ['entity.setActive', $this->entityUuid, $active]; + $this->isActive = $active; + } + + /** + * @return string + */ + public function getName(): string { + return 'Entity ' . $this->entityUuid; + } + }; + + }//end orgEntity() + + /** + * The cases: the organisatie object, the e-mail flag and the doubles' answers. + * + * @return array + */ + public static function entityCases(): array { + $missing = fn () => new \OCP\AppFramework\Db\DoesNotExistException('missing'); + return [ + 'full fetch fails, entity in sync' => [['name' => 'Gemeente A', 'status' => 'actief'], true, fn ($t) => ['find' => [new \RuntimeException('find failed')], 'org' => [$t->orgEntity('o-1', true)]]], + 'full object replaces the given one' => [['name' => 'A'], true, fn ($t) => ['find' => [$t->entity(['name' => 'Gemeente B', 'status' => 'Active', '@self' => ['owner' => 'o-1'], 'organisation' => 'o-1'], 'o-1')], 'org' => [$t->orgEntity('o-1', true)]]], + 'existing entity deactivated' => [['name' => 'Gemeente C', 'status' => 'inactief'], true, fn ($t) => ['org' => [$t->orgEntity('o-1', true)]]], + 'existing entity activated, mail sent' => [['name' => 'Gemeente D'], true, fn ($t) => ['org' => [$t->orgEntity('o-1', false)]]], + 'existing entity activated, mail not sent' => [['name' => 'Gemeente E', 'status' => 'ACTIVE'], true, fn ($t) => ['org' => [$t->orgEntity('o-1', false)], 'mail' => false]], + 'existing entity activated, mail throws' => [['name' => 'Gemeente E2'], true, fn ($t) => ['org' => [$t->orgEntity('o-1', false)], 'mail' => new \RuntimeException('smtp down')]], + 'existing entity activated, no mails' => [['name' => 'Gemeente F'], false, fn ($t) => ['org' => [$t->orgEntity('o-1', false)]]], + 'adopted by slug' => [['name' => 'Gemeente G & Zn!', 'status' => 'actief'], true, fn ($t) => ['org' => [$missing()], 'slug' => [$t->orgEntity('o-old', true)]]], + 'not found, created with mail' => [['name' => 'Gemeente H'], true, fn ($t) => ['org' => [$missing()], 'slug' => [$missing()]]], + 'not found, created, mail not sent' => [['name' => 'Gemeente H2'], true, fn ($t) => ['org' => [$missing()], 'slug' => [$missing()], 'mail' => false]], + 'not found, created without mail' => [['name' => 'Gemeente I'], false, fn ($t) => ['org' => [$missing()], 'slug' => [$missing()]]], + 'no name, created' => [['status' => 'actief'], true, fn ($t) => ['org' => [$missing()]]], + 'creation fails' => [['name' => 'Gemeente J'], true, fn ($t) => ['org' => [$missing()], 'slug' => [$missing()], 'created' => null]], + 'mapper fails' => [['name' => 'Gemeente K'], true, fn ($t) => ['org' => [new \RuntimeException('db down')]]], + 'owner save fails' => [['name' => 'Gemeente L'], true, fn ($t) => ['org' => [$t->orgEntity('o-1', true)], 'save' => [new \RuntimeException('save failed')]]], + 'settings fail' => [['name' => 'Gemeente M'], true, fn ($t) => ['config' => new \RuntimeException('settings down')]], + ]; + + }//end entityCases() + + /** + * Each organisation entity case matches its recording. + * + * @param array $object The organisatie object data. + * @param bool $sendEmails Whether e-mails are sent. + * @param \Closure $plan Builds the doubles' answers. + * + * @return void + * + * @dataProvider entityCases + */ + public function testEnsureEntity(array $object, bool $sendEmails, \Closure $plan): void { + $this->trace = []; + $this->plan = array_merge(['find' => [null], 'save' => [], 'org' => [], 'slug' => []], $plan($this)); + $service = $this->service(); + $stats = ['entitiesCreated' => 0, 'entitiesUpdated' => 0]; + $entity = $service->ensureOrganisationEntityPublic($this->entity(object: $object, uuid: 'o-1'), $stats, $sendEmails); + $returned = null; + if ($entity !== null) { + $returned = [$entity->getUuid(), $entity->getActive()]; + } + + $this->check(key: 'entity: ' . $this->dataName(), value: ['returned' => $returned, 'stats' => $stats, 'trace' => $this->trace]); + + }//end testEnsureEntity() + + /** + * Compare a case with the golden file, or record it. + * + * @param string $key The case. + * @param array $value What the case observed. + * + * @return void + */ + private function check(string $key, array $value): void { + $json = json_decode(json_encode($value, JSON_PARTIAL_OUTPUT_ON_ERROR), true); + $file = self::golden(); + $saved = []; + if (file_exists($file) === true) { + $saved = json_decode((string) file_get_contents($file), true); + } + + if (getenv('RECORD_GOLDEN') !== false) { + $saved[$key] = $json; + if (is_dir(dirname($file)) === false) { + mkdir(dirname($file), 0777, true); + } + + file_put_contents($file, json_encode($saved, (JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)) . "\n"); + $this->addToAssertionCount(count: 1); + return; + } + + $this->assertArrayHasKey(key: $key, array: $saved, message: 'No golden for ' . $key); + $this->assertSame(expected: $saved[$key], actual: $json); + + }//end check() + +}//end class diff --git a/tests/Unit/Service/golden/organizationSyncEntity.json b/tests/Unit/Service/golden/organizationSyncEntity.json new file mode 100644 index 000000000..02b4560ab --- /dev/null +++ b/tests/Unit/Service/golden/organizationSyncEntity.json @@ -0,0 +1,1852 @@ +{ + "entity: full fetch fails, entity in sync": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.warning", + "Could not fetch full organisation object, using provided object", + { + "organisatieId": "o-1", + "error": "find failed" + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente A", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": false + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente A", + "status": "actief", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente A", + "status": "actief", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: full object replaces the given one": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente B", + "status": "Active" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": false + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "log.debug", + "OrganizationSyncService: Owner and organisation already set correctly, skipping update", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1" + } + ] + ] + }, + "entity: existing entity deactivated": { + "returned": [ + "o-1", + false + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 1 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente C", + "status": "inactief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": false, + "needsUpdate": true + } + ], + [ + "log.debug", + "Updating entity status", + { + "app": "stackiq", + "organisatieId": "o-1", + "oldActive": true, + "newActive": false + } + ], + [ + "entity.setActive", + "o-1", + false + ], + [ + "mapper.save", + "o-1", + false + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente C", + "status": "inactief", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente C", + "status": "inactief", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: existing entity activated, mail sent": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 1 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente D", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": true + } + ], + [ + "log.debug", + "Updating entity status", + { + "app": "stackiq", + "organisatieId": "o-1", + "oldActive": false, + "newActive": true + } + ], + [ + "entity.setActive", + "o-1", + true + ], + [ + "mapper.save", + "o-1", + true + ], + [ + "log.info", + "[FLOW] Sending organization activation email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationActivationEmail", + "Gemeente D" + ], + [ + "log.info", + "📧 Organization activation email sent successfully", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente D", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente D", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: existing entity activated, mail not sent": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 1 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente E", + "status": "ACTIVE" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": true + } + ], + [ + "log.debug", + "Updating entity status", + { + "app": "stackiq", + "organisatieId": "o-1", + "oldActive": false, + "newActive": true + } + ], + [ + "entity.setActive", + "o-1", + true + ], + [ + "mapper.save", + "o-1", + true + ], + [ + "log.info", + "[FLOW] Sending organization activation email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationActivationEmail", + "Gemeente E" + ], + [ + "log.info", + "📧 Organization activation email not sent (disabled or not configured)", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente E", + "status": "ACTIVE", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente E", + "status": "ACTIVE", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: existing entity activated, mail throws": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 1 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente E2", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": true + } + ], + [ + "log.debug", + "Updating entity status", + { + "app": "stackiq", + "organisatieId": "o-1", + "oldActive": false, + "newActive": true + } + ], + [ + "entity.setActive", + "o-1", + true + ], + [ + "mapper.save", + "o-1", + true + ], + [ + "log.info", + "[FLOW] Sending organization activation email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationActivationEmail", + "Gemeente E2" + ], + [ + "log.warning", + "OrganizationSyncService: Organization activation email failed", + { + "organizationId": "unknown", + "organizationName": "Gemeente E2", + "error": "smtp down" + } + ], + [ + "log.info", + "📧 Organization activation email not sent (disabled or not configured)", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente E2", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente E2", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: existing entity activated, no mails": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 1 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente F", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": true + } + ], + [ + "log.debug", + "Updating entity status", + { + "app": "stackiq", + "organisatieId": "o-1", + "oldActive": false, + "newActive": true + } + ], + [ + "entity.setActive", + "o-1", + true + ], + [ + "mapper.save", + "o-1", + true + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente F", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente F", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: adopted by slug": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 1 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente G & Zn!", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-g-zn" + ], + [ + "log.info", + "OrganizationSyncService: Found existing entity by slug, updating UUID to match object", + { + "app": "stackiq", + "organisatieId": "o-1", + "oldEntityUuid": "o-old", + "slug": "gemeente-g-zn" + } + ], + [ + "entity.setUuid", + "o-old", + "o-1" + ], + [ + "mapper.save", + "o-1", + true + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente G & Zn!", + "status": "actief", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente G & Zn!", + "status": "actief", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "ownerSet": "o-1", + "organisationSet": "o-1" + } + ] + ] + }, + "entity: not found, created with mail": { + "returned": [ + "new-entity", + true + ], + "stats": { + "entitiesCreated": 1, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente H", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-h" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente H" + } + ], + [ + "organisations.create", + { + "name": "Gemeente H" + } + ], + [ + "log.debug", + "New organisation entity created", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "active": true, + "name": "Entity new-entity" + } + ], + [ + "log.info", + "[FLOW] Sending organization registration email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationRegistrationEmail", + "Gemeente H" + ], + [ + "log.info", + "📧 Organization registration email sent successfully", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente H", + "organisation": "new-entity", + "@self": { + "owner": "new-entity", + "organisation": "new-entity" + } + } + ], + [ + "entity.setOwner", + "o-1", + "new-entity" + ], + [ + "entity.setOrganisation", + "o-1", + "new-entity" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente H", + "organisation": "new-entity", + "@self": { + "organisation": "new-entity" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "ownerSet": "new-entity", + "organisationSet": "new-entity" + } + ] + ] + }, + "entity: not found, created, mail not sent": { + "returned": [ + "new-entity", + true + ], + "stats": { + "entitiesCreated": 1, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente H2", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-h2" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente H2" + } + ], + [ + "organisations.create", + { + "name": "Gemeente H2" + } + ], + [ + "log.debug", + "New organisation entity created", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "active": true, + "name": "Entity new-entity" + } + ], + [ + "log.info", + "[FLOW] Sending organization registration email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationRegistrationEmail", + "Gemeente H2" + ], + [ + "log.info", + "📧 Organization registration email not sent (disabled or not configured)", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente H2", + "organisation": "new-entity", + "@self": { + "owner": "new-entity", + "organisation": "new-entity" + } + } + ], + [ + "entity.setOwner", + "o-1", + "new-entity" + ], + [ + "entity.setOrganisation", + "o-1", + "new-entity" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente H2", + "organisation": "new-entity", + "@self": { + "organisation": "new-entity" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "ownerSet": "new-entity", + "organisationSet": "new-entity" + } + ] + ] + }, + "entity: not found, created without mail": { + "returned": [ + "new-entity", + true + ], + "stats": { + "entitiesCreated": 1, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente I", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-i" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente I" + } + ], + [ + "organisations.create", + { + "name": "Gemeente I" + } + ], + [ + "log.debug", + "New organisation entity created", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "active": true, + "name": "Entity new-entity" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente I", + "organisation": "new-entity", + "@self": { + "owner": "new-entity", + "organisation": "new-entity" + } + } + ], + [ + "entity.setOwner", + "o-1", + "new-entity" + ], + [ + "entity.setOrganisation", + "o-1", + "new-entity" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente I", + "organisation": "new-entity", + "@self": { + "organisation": "new-entity" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "ownerSet": "new-entity", + "organisationSet": "new-entity" + } + ] + ] + }, + "entity: no name, created": { + "returned": [ + "new-entity", + true + ], + "stats": { + "entitiesCreated": 1, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Unknown", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Unknown" + } + ], + [ + "organisations.create", + { + "status": "actief" + } + ], + [ + "log.debug", + "New organisation entity created", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "active": true, + "name": "Entity new-entity" + } + ], + [ + "log.info", + "[FLOW] Sending organization registration email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationRegistrationEmail", + null + ], + [ + "log.info", + "📧 Organization registration email sent successfully", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "status": "actief", + "organisation": "new-entity", + "@self": { + "owner": "new-entity", + "organisation": "new-entity" + } + } + ], + [ + "entity.setOwner", + "o-1", + "new-entity" + ], + [ + "entity.setOrganisation", + "o-1", + "new-entity" + ], + [ + "objects.saveObject", + { + "object": { + "status": "actief", + "organisation": "new-entity", + "@self": { + "organisation": "new-entity" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "ownerSet": "new-entity", + "organisationSet": "new-entity" + } + ] + ] + }, + "entity: creation fails": { + "returned": null, + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente J", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-j" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente J" + } + ], + [ + "organisations.create", + { + "name": "Gemeente J" + } + ], + [ + "log.error", + "❌ ORGANISATION ENTITY CREATION FAILED", + { + "app": "stackiq", + "organisatieId": "o-1" + } + ] + ] + }, + "entity: mapper fails": { + "returned": null, + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente K", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.error", + "💥 ENSURE ORGANISATION ENTITY EXCEPTION", + { + "app": "stackiq", + "organisatieId": 7, + "exception": "db down", + "file": "(file)", + "line": "(line)", + "trace": "(trace)" + } + ] + ] + }, + "entity: owner save fails": { + "returned": [ + "o-1", + true + ], + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente L", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Existing entity found", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "shouldBeActive": true, + "needsUpdate": false + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente L", + "organisation": "o-1", + "@self": { + "owner": "o-1", + "organisation": "o-1" + } + } + ], + [ + "entity.setOwner", + "o-1", + "o-1" + ], + [ + "entity.setOrganisation", + "o-1", + "o-1" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente L", + "organisation": "o-1", + "@self": { + "organisation": "o-1" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.error", + "OrganizationSyncService: Failed to update organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "o-1", + "exception": "save failed", + "file": "(file)", + "line": "(line)" + } + ] + ] + }, + "entity: settings fail": { + "returned": null, + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente M", + "status": "Unknown" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.error", + "💥 ENSURE ORGANISATION ENTITY EXCEPTION", + { + "app": "stackiq", + "organisatieId": 7, + "exception": "settings down", + "file": "(file)", + "line": "(line)", + "trace": "(trace)" + } + ] + ] + } +} From 80859ef15ecef39162ea6b6f324ae97cb19bb6a1 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 15:08:00 +0100 Subject: [PATCH 4/7] refactor(organization-sync): split ensuring the organisation entity into refresh, adopt-by-slug and create steps ensureOrganisationEntity takes the organisatie object and one run array (stats held by reference, and an explicit sendEmails at every call site) and hands the work to fullOrganisationObject, refreshOrganisationEntity, adoptOrganisationEntityBySlug, createOrganisationEntity, mailOrganisation and makeEntityOwner. The public wrapper keeps its signature. Lookups, saves, mails, log lines and stats are unchanged (golden of the previous commit). Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/OrganizationSyncService.php | 438 +++++++++++++----------- 1 file changed, 240 insertions(+), 198 deletions(-) diff --git a/lib/Service/OrganizationSyncService.php b/lib/Service/OrganizationSyncService.php index 20d41cc61..7cc7e4d0a 100644 --- a/lib/Service/OrganizationSyncService.php +++ b/lib/Service/OrganizationSyncService.php @@ -56,6 +56,8 @@ * @SuppressWarnings(PHPMD.Superglobals) * @SuppressWarnings(PHPMD.CamelCaseVariableName) * @SuppressWarnings(PHPMD.CamelCaseParameterName) + * + * @spec openspec/specs/organization-sync/spec.md#requirement-the-system-shall-process-individual-organisationcontact-records-and-ensure-their-openregister-entity-req-002 */ class OrganizationSyncService { @@ -428,7 +430,7 @@ public function performOrganizationsSync(int $batchSize = 50, int $maxExecutionS _multitenancy: false ); - $this->ensureOrganisationEntity(organisationObject: $object, stats: $stats, sendEmails: false); + $this->ensureOrganisationEntity(organisationObject: $object, run: ['stats' => &$stats, 'sendEmails' => false]); $stats['organizationsProcessed']++; } catch (\Exception $e) { $this->handleSyncError(operation: 'OrganizationSync', identifier: (string)$row['uuid'], exception: $e, stats: $stats); @@ -892,7 +894,7 @@ private function processOrganisationObject( ); // Step 1: Ensure organisation entity exists. - $organisationEntity = $this->ensureOrganisationEntity(organisationObject: $organisationObject, stats: $stats); + $organisationEntity = $this->ensureOrganisationEntity(organisationObject: $organisationObject, run: ['stats' => &$stats, 'sendEmails' => true]); if ($organisationEntity === null) { $this->logger->warning( 'OrganizationSyncService: Could not ensure organisation entity', @@ -946,8 +948,7 @@ private function processOrganisationObject( public function ensureOrganisationEntityPublic(object $organisationObject, array &$stats, bool $sendEmails = true): ?object { return $this->ensureOrganisationEntity( organisationObject: $organisationObject, - stats: $stats, - sendEmails: $sendEmails + run: ['stats' => &$stats, 'sendEmails' => $sendEmails] ); }//end ensureOrganisationEntityPublic() @@ -956,40 +957,16 @@ public function ensureOrganisationEntityPublic(object $organisationObject, array * Ensures organisation entity exists for organisatie object. * * @param object $organisationObject The organisatie object. - * @param array $stats Statistics array (passed by reference). - * @param bool $sendEmails Whether to send emails. + * @param array $run The sync run: `stats` (held by reference) and `sendEmails`, + * whether to send registration and activation e-mails. * * @return object|null The organisation entity or null on failure. - * - * @SuppressWarnings(PHPMD.BooleanArgumentFlag) $sendEmails is a simple notification toggle */ - private function ensureOrganisationEntity(object $organisationObject, array &$stats, bool $sendEmails = true): ?object { + private function ensureOrganisationEntity(object $organisationObject, array $run): ?object { try { // Get the full object data - the passed object might not have all fields populated. $organisationId = $organisationObject->getUuid(); - - // Fetch the complete object from the database to ensure we have all data. - try { - $fullObject = $this->objectService->find( - id: $organisationId, - register: $organisationObject->getRegister(), - schema: $organisationObject->getSchema(), - _rbac: false, - _multitenancy: false - ); - if (empty($fullObject) === false) { - $organisationObject = $fullObject; - } - } catch (\Exception $e) { - $this->logger->warning( - 'Could not fetch full organisation object, using provided object', - [ - 'organisatieId' => $organisationId, - 'error' => $e->getMessage(), - ] - ); - }//end try - + $organisationObject = $this->fullOrganisationObject(organisationObject: $organisationObject); $objectData = $organisationObject->getObject(); $this->logger->debug( @@ -1004,197 +981,262 @@ private function ensureOrganisationEntity(object $organisationObject, array &$st // Get configuration for object updates. $voorzieningenConfig = $this->settingsService->getVoorzieningenConfig(); - $register = ($voorzieningenConfig['register'] ?? ''); - $organizationSchema = ($voorzieningenConfig['organisatie_schema'] ?? ''); + $run['organisationId'] = $organisationId; + $run['organisationObject'] = $organisationObject; + $run['objectData'] = $objectData; + $run['register'] = ($voorzieningenConfig['register'] ?? ''); + $run['organizationSchema'] = ($voorzieningenConfig['organisatie_schema'] ?? ''); // Try to find existing organisation entity. - try { $organisationEntity = $this->organisationMapper->findByUuid($organisationId); + return $this->refreshOrganisationEntity(organisationEntity: $organisationEntity, run: $run); + } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { + return $this->adoptOrganisationEntityBySlug(run: $run) ?? $this->createOrganisationEntity(run: $run); + } + } catch (\Exception $e) { + $this->logger->error( + '💥 ENSURE ORGANISATION ENTITY EXCEPTION', + [ + 'app' => 'stackiq', + 'organisatieId' => $organisationObject->getId(), + 'exception' => $e->getMessage(), + 'file' => $e->getFile(), + 'line' => $e->getLine(), + 'trace' => $e->getTraceAsString(), + ] + ); + return null; + }//end try - // Entity exists - update it if needed. - $status = strtolower(($objectData['status'] ?? 'actief')); - $shouldBeActive = in_array($status, ['actief', 'active']) === true; + }//end ensureOrganisationEntity() - $this->logger->debug( - 'Existing entity found', - [ - 'app' => 'stackiq', - 'organisatieId' => $organisationId, - 'entityId' => $organisationEntity->getId(), - 'shouldBeActive' => $shouldBeActive, - 'needsUpdate' => $organisationEntity->getActive() !== $shouldBeActive, - ] - ); + /** + * The organisatie object as stored, or the one given when it cannot be fetched. + * + * @param object $organisationObject The organisatie object as passed in. + * + * @return object The complete organisatie object + */ + private function fullOrganisationObject(object $organisationObject): object { + $organisationId = $organisationObject->getUuid(); + try { + $fullObject = $this->objectService->find( + id: $organisationId, + register: $organisationObject->getRegister(), + schema: $organisationObject->getSchema(), + _rbac: false, + _multitenancy: false + ); + if (empty($fullObject) === false) { + return $fullObject; + } + } catch (\Exception $e) { + $this->logger->warning( + 'Could not fetch full organisation object, using provided object', + [ + 'organisatieId' => $organisationId, + 'error' => $e->getMessage(), + ] + ); + }//end try - if ($organisationEntity->getActive() !== $shouldBeActive) { - $this->logger->debug( - 'Updating entity status', - [ - 'app' => 'stackiq', - 'organisatieId' => $organisationId, - 'oldActive' => $organisationEntity->getActive(), - 'newActive' => $shouldBeActive, - ] - ); + return $organisationObject; + }//end fullOrganisationObject() - $wasActive = $organisationEntity->getActive(); - $organisationEntity->setActive($shouldBeActive); - $this->organisationMapper->save($organisationEntity); - $stats['entitiesUpdated']++; + /** + * Bring an existing organisation entity's active flag in line with the object's status, + * mail an organisation that became active, and make the entity the object's owner. + * + * @param object $organisationEntity The existing organisation entity. + * @param array $run The ensure run (see ensureOrganisationEntity). + * + * @return object The organisation entity + */ + private function refreshOrganisationEntity(object $organisationEntity, array $run): object { + $organisationId = $run['organisationId']; - // Send activation email if organization became active. - if ($sendEmails !== false && $shouldBeActive === true && $wasActive === false) { - $this->logger->info( - '[FLOW] Sending organization activation email', - ['organisatieId' => $organisationId] - ); - $emailSent = $this->sendOrganizationActivationEmail(organizationData: $objectData); - if (empty($emailSent) === false) { - $this->logger->info( - '📧 Organization activation email sent successfully', - ['organisatieId' => $organisationId] - ); - } + // Entity exists - update it if needed. + $status = strtolower(($run['objectData']['status'] ?? 'actief')); + $shouldBeActive = in_array($status, ['actief', 'active']) === true; - if (empty($emailSent) === true) { - $this->logger->info( - '📧 Organization activation email not sent (disabled or not configured)', - ['organisatieId' => $organisationId] - ); - } - }//end if - }//end if + $this->logger->debug( + 'Existing entity found', + [ + 'app' => 'stackiq', + 'organisatieId' => $organisationId, + 'entityId' => $organisationEntity->getId(), + 'shouldBeActive' => $shouldBeActive, + 'needsUpdate' => $organisationEntity->getActive() !== $shouldBeActive, + ] + ); - // Update organisatie object owner to organisation entity UUID. - $this->updateOrganisationObjectOwner( - organisationObject: $organisationObject, - organisationEntity: $organisationEntity, - register: $register, - organizationSchema: $organizationSchema - ); + if ($organisationEntity->getActive() !== $shouldBeActive) { + $this->logger->debug( + 'Updating entity status', + [ + 'app' => 'stackiq', + 'organisatieId' => $organisationId, + 'oldActive' => $organisationEntity->getActive(), + 'newActive' => $shouldBeActive, + ] + ); - return $organisationEntity; - } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - // Entity not found by UUID — try finding by slug before creating. - // This handles the case where the entity exists but with a different UUID (e.g. from slug collision). - $orgName = ($objectData['name'] ?? ''); - if (empty($orgName) === false) { - $slug = strtolower(preg_replace('/[^a-z0-9]+/', '-', strtolower($orgName))); - $slug = trim($slug, '-'); - try { - $organisationEntity = $this->organisationMapper->findBySlug($slug); - $this->logger->info( - 'OrganizationSyncService: Found existing entity by slug, updating UUID to match object', - [ - 'app' => 'stackiq', - 'organisatieId' => $organisationId, - 'oldEntityUuid' => $organisationEntity->getUuid(), - 'slug' => $slug, - ] - ); + $wasActive = $organisationEntity->getActive(); + $organisationEntity->setActive($shouldBeActive); + $this->organisationMapper->save($organisationEntity); + $run['stats']['entitiesUpdated']++; - // Update the entity's UUID to match the object UUID so future lookups work. - $organisationEntity->setUuid($organisationId); - $this->organisationMapper->save($organisationEntity); - $stats['entitiesUpdated']++; + // Send activation email if organization became active. + if ($run['sendEmails'] !== false && $shouldBeActive === true && $wasActive === false) { + $this->mailOrganisation(kind: 'activation', run: $run); + } + }//end if - // Update organisatie object owner to this entity. - $this->updateOrganisationObjectOwner( - organisationObject: $organisationObject, - organisationEntity: $organisationEntity, - register: $register, - organizationSchema: $organizationSchema - ); + // Update organisatie object owner to organisation entity UUID. + $this->makeEntityOwner(organisationEntity: $organisationEntity, run: $run); - return $organisationEntity; - } catch (\OCP\AppFramework\Db\DoesNotExistException $slugEx) { - // Not found by slug either — proceed with creation. - }//end try - }//end if + return $organisationEntity; + }//end refreshOrganisationEntity() - $this->logger->debug( - 'Creating new organisation entity', - [ - 'app' => 'stackiq', - 'organisatieId' => $organisationId, - 'name' => ($objectData['name'] ?? 'Unknown'), - ] - ); + /** + * Adopt an entity that has the organisation's slug but another uuid: it takes the object's uuid. + * + * @param array $run The ensure run (see ensureOrganisationEntity). + * + * @return object|null The adopted entity, or null when the object has no name or no entity has its slug + */ + private function adoptOrganisationEntityBySlug(array $run): ?object { + // Entity not found by UUID — try finding by slug before creating. + // This handles the case where the entity exists but with a different UUID (e.g. from slug collision). + $orgName = ($run['objectData']['name'] ?? ''); + if (empty($orgName) === true) { + return null; + } - $organisationEntity = $this->organisationService->createOrganisationInOpenRegister($objectData); - if (empty($organisationEntity) === false) { - $stats['entitiesCreated']++; - $this->logger->debug( - 'New organisation entity created', - [ - 'app' => 'stackiq', - 'organisatieId' => $organisationId, - 'entityId' => $organisationEntity->getId(), - 'active' => $organisationEntity->getActive(), - 'name' => $organisationEntity->getName(), - ] - ); + $slug = strtolower(preg_replace('/[^a-z0-9]+/', '-', strtolower($orgName))); + $slug = trim($slug, '-'); + try { + $organisationEntity = $this->organisationMapper->findBySlug($slug); + $this->logger->info( + 'OrganizationSyncService: Found existing entity by slug, updating UUID to match object', + [ + 'app' => 'stackiq', + 'organisatieId' => $run['organisationId'], + 'oldEntityUuid' => $organisationEntity->getUuid(), + 'slug' => $slug, + ] + ); - // Send registration email for new organization (skip for backfill). - if (empty($sendEmails) === false) { - $this->logger->info( - '[FLOW] Sending organization registration email', - ['organisatieId' => $organisationId] - ); - $emailSent = $this->sendOrganizationRegistrationEmail(organizationData: $objectData); - if (empty($emailSent) === false) { - $this->logger->info( - '📧 Organization registration email sent successfully', - ['organisatieId' => $organisationId] - ); - } + // Update the entity's UUID to match the object UUID so future lookups work. + $organisationEntity->setUuid($run['organisationId']); + $this->organisationMapper->save($organisationEntity); + $run['stats']['entitiesUpdated']++; - if (empty($emailSent) === true) { - $this->logger->info( - '📧 Organization registration email not sent (disabled or not configured)', - ['organisatieId' => $organisationId] - ); - } - }//end if + // Update organisatie object owner to this entity. + $this->makeEntityOwner(organisationEntity: $organisationEntity, run: $run); - // Update organisatie object owner to organisation entity UUID. - $this->updateOrganisationObjectOwner( - organisationObject: $organisationObject, - organisationEntity: $organisationEntity, - register: $register, - organizationSchema: $organizationSchema - ); - }//end if + return $organisationEntity; + } catch (\OCP\AppFramework\Db\DoesNotExistException $slugEx) { + // Not found by slug either — proceed with creation. + return null; + }//end try + }//end adoptOrganisationEntityBySlug() - if (empty($organisationEntity) === true) { - $this->logger->error( - '❌ ORGANISATION ENTITY CREATION FAILED', - [ - 'app' => 'stackiq', - 'organisatieId' => $organisationId, - ] - ); - } + /** + * Create the organisation entity, mail the new organisation and make the entity the object's owner. + * + * @param array $run The ensure run (see ensureOrganisationEntity). + * + * @return object|null The new entity, or what creation returned when it failed + */ + private function createOrganisationEntity(array $run): ?object { + $organisationId = $run['organisationId']; + $this->logger->debug( + 'Creating new organisation entity', + [ + 'app' => 'stackiq', + 'organisatieId' => $organisationId, + 'name' => ($run['objectData']['name'] ?? 'Unknown'), + ] + ); - return $organisationEntity; - }//end try - } catch (\Exception $e) { + $organisationEntity = $this->organisationService->createOrganisationInOpenRegister($run['objectData']); + if (empty($organisationEntity) === true) { $this->logger->error( - '💥 ENSURE ORGANISATION ENTITY EXCEPTION', + '❌ ORGANISATION ENTITY CREATION FAILED', [ 'app' => 'stackiq', - 'organisatieId' => $organisationObject->getId(), - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), + 'organisatieId' => $organisationId, ] ); - return null; - }//end try + return $organisationEntity; + } - }//end ensureOrganisationEntity() + $run['stats']['entitiesCreated']++; + $this->logger->debug( + 'New organisation entity created', + [ + 'app' => 'stackiq', + 'organisatieId' => $organisationId, + 'entityId' => $organisationEntity->getId(), + 'active' => $organisationEntity->getActive(), + 'name' => $organisationEntity->getName(), + ] + ); + + // Send registration email for new organization (skip for backfill). + if (empty($run['sendEmails']) === false) { + $this->mailOrganisation(kind: 'registration', run: $run); + } + + // Update organisatie object owner to organisation entity UUID. + $this->makeEntityOwner(organisationEntity: $organisationEntity, run: $run); + + return $organisationEntity; + }//end createOrganisationEntity() + + /** + * Send an organisation its registration or activation e-mail and log whether it went out. + * + * @param string $kind 'registration' or 'activation'. + * @param array $run The ensure run (see ensureOrganisationEntity). + * + * @return void + */ + private function mailOrganisation(string $kind, array $run): void { + $context = ['organisatieId' => $run['organisationId']]; + $this->logger->info('[FLOW] Sending organization ' . $kind . ' email', $context); + if ($kind === 'activation') { + $emailSent = $this->sendOrganizationActivationEmail(organizationData: $run['objectData']); + } else { + $emailSent = $this->sendOrganizationRegistrationEmail(organizationData: $run['objectData']); + } + + if (empty($emailSent) === false) { + $this->logger->info('📧 Organization ' . $kind . ' email sent successfully', $context); + return; + } + + $this->logger->info('📧 Organization ' . $kind . ' email not sent (disabled or not configured)', $context); + }//end mailOrganisation() + + /** + * Make the organisation entity the owner and organisation of the run's organisatie object. + * + * @param object $organisationEntity The organisation entity. + * @param array $run The ensure run (see ensureOrganisationEntity). + * + * @return void + */ + private function makeEntityOwner(object $organisationEntity, array $run): void { + $this->updateOrganisationObjectOwner( + organisationObject: $run['organisationObject'], + organisationEntity: $organisationEntity, + register: $run['register'], + organizationSchema: $run['organizationSchema'] + ); + }//end makeEntityOwner() /** * Safely sends organization registration email with error handling @@ -1724,7 +1766,7 @@ public function processSpecificOrganization($organizationObject): array { ] ); - $organisationEntity = $this->ensureOrganisationEntity(organisationObject: $organizationObject, stats: $stats); + $organisationEntity = $this->ensureOrganisationEntity(organisationObject: $organizationObject, run: ['stats' => &$stats, 'sendEmails' => true]); if (empty($organisationEntity) === false) { $stats['organizationsProcessed']++; @@ -2611,7 +2653,7 @@ private function createBackupOrganisationEntity(string $organizationUuid): ?obje ]; return $this->ensureOrganisationEntity( organisationObject: $orgObject, - stats: $backupStats + run: ['stats' => &$backupStats, 'sendEmails' => true] ); } } catch (\Exception $backupEx) { @@ -2902,7 +2944,7 @@ public function processSpecificContactPerson($contactObject): array { ]; $organisationEntity = $this->ensureOrganisationEntity( organisationObject: $orgObject, - stats: $backupStats + run: ['stats' => &$backupStats, 'sendEmails' => true] ); } } catch (\Exception $backupEx) { From 01b0c420e29583481143db0eb01bd9dac942ff2f Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 15:14:20 +0100 Subject: [PATCH 5/7] test(stackiq): golden for the organisation sync with OpenRegister, as it behaves today Creating an organisation escapes with an Error on both paths: the signed-in path calls OrganisationMapper::createWithUuid, which OpenRegister does not have, and the anonymous path saves and then reads the absent user. Assisted-by: ClaudeCode:claude-opus-5-5 --- ...ackiqServiceOrganisationSyncGoldenTest.php | 393 ++++++ .../stackiqServiceOrganisationSync.json | 1243 +++++++++++++++++ 2 files changed, 1636 insertions(+) create mode 100644 tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php create mode 100644 tests/Unit/Service/golden/stackiqServiceOrganisationSync.json diff --git a/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php b/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php new file mode 100644 index 000000000..1f49ea4d8 --- /dev/null +++ b/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php @@ -0,0 +1,393 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/specs/softwarecatalogue-orchestration/spec.md + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Service; + +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\OpenRegister\Db\Organisation; +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\OpenRegister\Service\OrganisationService; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Service\Stackiq\ContactPersonHandler; +use OCA\Stackiq\Service\Stackiq\GroupHandler; +use OCA\Stackiq\Service\Stackiq\HierarchyHandler; +use OCA\Stackiq\Service\Stackiq\OrganizationHandler; +use OCA\Stackiq\Service\StackiqService; +use OCA\Stackiq\Service\SymfonyEmailService; +use OCP\App\IAppManager; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IGroupManager; +use OCP\IUser; +use OCP\IUserManager; +use OCP\IUserSession; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Psr\Log\AbstractLogger; + +/** + * Golden cases for the organisation sync with OpenRegister. + * + * @covers \OCA\Stackiq\Service\StackiqService + * + * @spec openspec/specs/softwarecatalogue-orchestration/spec.md + */ +class StackiqServiceOrganisationSyncGoldenTest extends TestCase { + + /** + * Everything the doubles saw, in order. + * + * @var array + */ + private array $trace = []; + + /** + * Per-case behaviour of the doubles. + * + * @var array + */ + private array $plan = []; + + /** + * The golden file. + * + * @return string + */ + private static function golden(): string { + return __DIR__ . '/golden/stackiqServiceOrganisationSync.json'; + + }//end golden() + + /** + * The cases: entry point, organisation data and the doubles' behaviour. + * + * @return array}> + */ + public static function cases(): array { + $org = [ + 'id' => 'org-1', + 'name' => 'Gemeente A', + 'website' => 'https://a.nl', + 'beoordeling' => 'actief', + 'contactpersonen' => [['email' => 'a@a.nl'], ['naam' => 'zonder mail'], 'c-2'], + ]; + return [ + 'sync, OpenRegister disabled' => ['sync', $org, ['disabled' => true]], + 'sync, existing organisation' => ['sync', $org, ['exists' => true]], + 'sync, existing, save fails' => ['sync', $org, ['exists' => true, 'saveThrows' => true]], + 'sync, new, signed in' => ['sync', $org, []], + 'sync, new, signed in with schema' => ['sync', $org, ['schema' => true, 'contacts' => [['email' => 'b@a.nl', 'organization' => 'org-1'], ['email' => 'c@a.nl', 'organization' => 'org-9']]]], + 'sync, new, anonymous' => ['sync', $org, ['anonymous' => true]], + 'create, no id' => ['create', ['name' => 'X'], []], + 'create, signed in' => ['create', $org, []], + 'create, organisation service down' => ['create', $org, ['serviceThrows' => true]], + 'create, anonymous, no mapper' => ['create', $org, ['anonymous' => true, 'noMapper' => true]], + ]; + + }//end cases() + + /** + * Each case matches its recording. + * + * @param string $entry Which entry point runs. + * @param array $data The organisation object data. + * @param array $plan The behaviour of the doubles. + * + * @return void + * + * @dataProvider cases + */ + public function testSyncMatchesTheRecording(string $entry, array $data, array $plan): void { + $this->plan = $plan; + $this->trace = []; + $service = $this->service(); + try { + if ($entry === 'sync') { + $result = $service->syncOrganizationWithOpenRegister($this->stored(data: $data, id: 17)); + } else { + $result = $service->createOrganisationInOpenRegister($data); + if (is_object($result) === true) { + $result = 'entity:' . $result->getUuid(); + } + } + } catch (\Throwable $e) { + $result = ['escaped' => get_class($e), 'message' => preg_replace('/Mock\w+/', 'Mock', $e->getMessage())]; + } + + $this->check(key: $entry . ': ' . $this->dataName(), value: ['result' => $result, 'calls' => $this->trace]); + + }//end testSyncMatchesTheRecording() + + /** + * The service with recording doubles. + * + * @return StackiqService + */ + private function service(): StackiqService { + $appManager = $this->createMock(IAppManager::class); + $appManager->method('isEnabledForUser')->willReturn(isset($this->plan['disabled']) === false); + + $mapper = $this->mapper(); + $objects = $this->objectService(); + $settings = $this->createMock(SettingsService::class); + $settings->method('getVoorzieningenConfig')->willReturnCallback( + function (): array { + $this->trace[] = ['settings.getVoorzieningenConfig']; + return isset($this->plan['schema']) === true ? ['contactpersoon_schema' => '9'] : []; + } + ); + $organisations = $this->createMock(OrganisationService::class); + + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->willReturnCallback( + function (string $id) use ($mapper, $objects, $settings, $organisations) { + $this->trace[] = ['container.get', $id]; + if ($id === 'OCA\OpenRegister\Service\OrganisationService' && isset($this->plan['serviceThrows']) === true) { + throw new \RuntimeException('organisation service down'); + } + + if ($id === 'OCA\OpenRegister\Db\OrganisationMapper' && isset($this->plan['noMapper']) === true) { + throw new \RuntimeException('no mapper'); + } + + $answers = [ + 'OCA\OpenRegister\Db\OrganisationMapper' => $mapper, + 'OCA\OpenRegister\Service\ObjectService' => $objects, + 'OCA\Stackiq\Service\SettingsService' => $settings, + 'OCA\OpenRegister\Service\OrganisationService' => $organisations, + ]; + return $answers[$id] ?? null; + } + ); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('signed-in'); + $userSession = $this->createMock(IUserSession::class); + $userSession->method('getUser')->willReturn(isset($this->plan['anonymous']) === true ? null : $user); + + return new StackiqService( + _organizationHandler: $this->createMock(OrganizationHandler::class), + _contactPersonHandler: $this->createMock(ContactPersonHandler::class), + _groupHandler: $this->createMock(GroupHandler::class), + _hierarchyHandler: $this->createMock(HierarchyHandler::class), + _emailService: $this->createMock(SymfonyEmailService::class), + _logger: $this->logger(), + _container: $container, + _appManager: $appManager, + _userSession: $userSession, + _userManager: $this->createMock(IUserManager::class), + _groupManager: $this->createMock(IGroupManager::class) + ); + + }//end service() + + /** + * An organisation mapper that records lookups and saves. + * + * @return OrganisationMapper + */ + private function mapper(): OrganisationMapper { + $mapper = $this->createMock(OrganisationMapper::class); + $mapper->method('findByUuid')->willReturnCallback( + function (string $uuid) { + $this->trace[] = ['mapper.findByUuid', $uuid]; + if (isset($this->plan['exists']) === false) { + throw new DoesNotExistException('missing'); + } + + return $this->organisation(uuid: $uuid); + } + ); + $mapper->method('save')->willReturnCallback( + function ($organisation) { + $this->trace[] = ['mapper.save', $organisation->getUuid()]; + if (isset($this->plan['saveThrows']) === true) { + throw new \RuntimeException('save failed'); + } + + return $organisation; + } + ); + + return $mapper; + + }//end mapper() + + /** + * An organisation entity double that records its writes. + * + * @param string $uuid The uuid. + * + * @return Organisation + */ + private function organisation(string $uuid): Organisation { + $trace = &$this->trace; + $organisation = new class($trace) extends Organisation { + /** + * @param array $trace The shared trace. + */ + public function __construct(private array &$trace) { + } + + /** + * @param string $name The method. + * @param array $args The arguments. + * + * @return mixed + */ + public function __call(string $name, array $args) { + $this->trace[] = ['organisation.' . $name, $args]; + return null; + } + }; + $organisation->setUuid($uuid); + return $organisation; + + }//end organisation() + + /** + * An object service that answers the contact person lookups. + * + * @return ObjectServiceInterface + */ + private function objectService(): ObjectServiceInterface { + $objects = $this->createMock(ObjectServiceInterface::class); + $objects->method('findAll')->willReturnCallback( + function (array $config) { + $this->trace[] = ['objects.findAll', $config]; + $found = []; + foreach ($this->plan['contacts'] ?? [] as $i => $data) { + $found[] = $this->stored(data: $data, id: 100 + $i); + } + + return $found; + } + ); + + return $objects; + + }//end objectService() + + /** + * A stored object double with an id and its data. + * + * @param array $data The object data. + * @param int $id The object id. + * + * @return object + */ + private function stored(array $data, int $id): object { + return new class($data, $id) { + /** + * @param array $data The object data. + * @param int $id The object id. + */ + public function __construct(private array $data, private int $id) { + } + + /** + * @return int + */ + public function getId(): int { + return $this->id; + } + + /** + * @return array + */ + public function getObject(): array { + return $this->data; + } + }; + + }//end stored() + + /** + * A logger that records every line, without stack traces. + * + * @return AbstractLogger + */ + private function logger(): AbstractLogger { + $trace = &$this->trace; + return new class($trace) extends AbstractLogger { + /** + * @param array $trace The shared trace. + */ + public function __construct(private array &$trace) { + } + + /** + * @param mixed $level The level. + * @param string|\Stringable $message The message. + * @param array $context The context. + * + * @return void + */ + public function log($level, string|\Stringable $message, array $context = []): void { + foreach (['trace', 'file', 'line'] as $volatile) { + if (array_key_exists($volatile, $context) === true) { + $context[$volatile] = '(' . $volatile . ')'; + } + } + + array_walk_recursive( + $context, + function (&$value) { + if (is_string($value) === true) { + $value = preg_replace('/Mock\w+/', 'Mock', $value); + } + } + ); + $this->trace[] = ['log.' . $level, preg_replace('/Mock\w+/', 'Mock', (string) $message), $context]; + } + }; + + }//end logger() + + /** + * Compare a case with the golden file, or record it. + * + * @param string $key The case. + * @param array $value What the case observed. + * + * @return void + */ + private function check(string $key, array $value): void { + $json = json_decode(json_encode($value, JSON_PARTIAL_OUTPUT_ON_ERROR), true); + $file = self::golden(); + $saved = []; + if (file_exists($file) === true) { + $saved = json_decode((string) file_get_contents($file), true); + } + + if (getenv('RECORD_GOLDEN') !== false) { + $saved[$key] = $json; + file_put_contents($file, json_encode($saved, (JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)) . "\n"); + $this->addToAssertionCount(count: 1); + return; + } + + $this->assertArrayHasKey(key: $key, array: $saved, message: 'No golden for ' . $key); + $this->assertSame(expected: $saved[$key], actual: $json); + + }//end check() + +}//end class diff --git a/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json b/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json new file mode 100644 index 000000000..9d2ec8795 --- /dev/null +++ b/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json @@ -0,0 +1,1243 @@ +{ + "sync: sync, OpenRegister disabled": { + "result": false, + "calls": [ + [ + "log.info", + "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", + { + "objectId": 17, + "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_2 - Extracted organization data", + { + "organizationUuid": "org-1", + "objectDataKeys": [ + "id", + "name", + "website", + "beoordeling", + "contactpersonen" + ], + "hasId": true + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - Getting OrganisationService", + [] + ], + [ + "log.error", + "StackiqService: SYNC_STEP_3 - OpenRegister OrganisationService not available", + [] + ] + ] + }, + "sync: sync, existing organisation": { + "result": true, + "calls": [ + [ + "log.info", + "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", + { + "objectId": 17, + "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_2 - Extracted organization data", + { + "organizationUuid": "org-1", + "objectDataKeys": [ + "id", + "name", + "website", + "beoordeling", + "contactpersonen" + ], + "hasId": true + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - Getting OrganisationService", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - OrganisationService retrieved", + { + "serviceClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_4 - OpenRegister configuration", + { + "organizationUuid": "org-1", + "organizationName": "Gemeente A" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5 - Checking if organization exists in OpenRegister", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5A - Getting OrganisationMapper for lookup", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5B - Calling findByUuid", + { + "uuid": "org-1" + } + ], + [ + "mapper.findByUuid", + "org-1" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_6 - Organization exists in OpenRegister, updating", + { + "organizationId": "org-1", + "existingOrganisationClass": "OCA\\OpenRegister\\Db\\Organisation@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:243$c0" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_7 - Mapping organization data", + [] + ], + [ + "log.info", + "StackiqService: Updating organization in OpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A" + } + ], + [ + "organisation.setName", + [ + "Gemeente A" + ] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "mapper.save", + "org-1" + ], + [ + "log.info", + "StackiqService: Successfully updated organization in OpenRegister", + { + "organizationUuid": "org-1", + "openRegisterId": "org-1" + } + ], + [ + "log.info", + "StackiqService: Successfully updated organization in OpenRegister", + { + "organizationId": "org-1", + "openRegisterId": "org-1" + } + ] + ] + }, + "sync: sync, existing, save fails": { + "result": false, + "calls": [ + [ + "log.info", + "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", + { + "objectId": 17, + "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_2 - Extracted organization data", + { + "organizationUuid": "org-1", + "objectDataKeys": [ + "id", + "name", + "website", + "beoordeling", + "contactpersonen" + ], + "hasId": true + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - Getting OrganisationService", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - OrganisationService retrieved", + { + "serviceClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_4 - OpenRegister configuration", + { + "organizationUuid": "org-1", + "organizationName": "Gemeente A" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5 - Checking if organization exists in OpenRegister", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5A - Getting OrganisationMapper for lookup", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5B - Calling findByUuid", + { + "uuid": "org-1" + } + ], + [ + "mapper.findByUuid", + "org-1" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_6 - Organization exists in OpenRegister, updating", + { + "organizationId": "org-1", + "existingOrganisationClass": "OCA\\OpenRegister\\Db\\Organisation@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:243$c0" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_7 - Mapping organization data", + [] + ], + [ + "log.info", + "StackiqService: Updating organization in OpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A" + } + ], + [ + "organisation.setName", + [ + "Gemeente A" + ] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "mapper.save", + "org-1" + ], + [ + "log.error", + "StackiqService: Failed to sync organization with OpenRegister: save failed", + { + "objectId": 17, + "exception": "save failed", + "file": "(file)", + "line": "(line)", + "trace": "(trace)" + } + ] + ] + }, + "sync: sync, new, signed in": { + "result": { + "escaped": "Error", + "message": "Call to undefined method Mock::createWithUuid()" + }, + "calls": [ + [ + "log.info", + "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", + { + "objectId": 17, + "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_2 - Extracted organization data", + { + "organizationUuid": "org-1", + "objectDataKeys": [ + "id", + "name", + "website", + "beoordeling", + "contactpersonen" + ], + "hasId": true + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - Getting OrganisationService", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - OrganisationService retrieved", + { + "serviceClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_4 - OpenRegister configuration", + { + "organizationUuid": "org-1", + "organizationName": "Gemeente A" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5 - Checking if organization exists in OpenRegister", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5A - Getting OrganisationMapper for lookup", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5B - Calling findByUuid", + { + "uuid": "org-1" + } + ], + [ + "mapper.findByUuid", + "org-1" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_8 - Organization not found in OpenRegister, creating", + { + "organizationId": "org-1", + "exception": "missing" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_9 - Mapping organization data for creation", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_10 - Calling createOrganisationInOpenRegister", + [] + ], + [ + "log.info", + "StackiqService: STEP 1 - Starting createOrganisationInOpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A", + "mappedDataKeys": [ + "name", + "type", + "website", + "active", + "contactpersonen", + "participants" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 2 - Checking user context", + { + "hasUserSession": true, + "currentUser": "signed-in", + "isAnonymous": false + } + ], + [ + "log.info", + "StackiqService: STEP 4A - Auth path: User logged in, creating org via mapper", + { + "organizationUuid": "org-1", + "currentUser": "signed-in" + } + ], + [ + "log.info", + "StackiqService: STEP 4B - Using original UUID format for OpenRegister", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 4C - Getting OrganisationMapper from container", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: STEP 4D - OrganisationMapper retrieved", + { + "mapperClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: Processing nested contact persons", + { + "organizationUuid": "org-1", + "nestedContactPersonCount": 0 + } + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\ObjectService" + ], + [ + "container.get", + "OCA\\Stackiq\\Service\\SettingsService" + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.warning", + "StackiqService: Missing contactpersoon schema config for username extraction", + [] + ], + [ + "log.info", + "StackiqService: STEP 4E - Debug - UUID before createWithUuid", + { + "organizationUuid": "org-1", + "uuidLength": 5, + "uuidIsEmpty": false, + "name": "Gemeente A", + "description": "https://a.nl", + "owner": "signed-in", + "users": [ + "signed-in" + ], + "contactPersonUsernames": [] + } + ], + [ + "log.info", + "StackiqService: STEP 4F - Calling organisationMapper->createWithUuid()", + [] + ], + [ + "log.info", + "StackiqService: STEP 4F_DEBUG - Parameters for createWithUuid", + { + "name": "Gemeente A", + "description": "https://a.nl", + "uuid": "org-1", + "owner": "signed-in", + "users": [ + "signed-in" + ], + "isDefault": false, + "uuidLength": 5, + "uuidIsEmpty": false + } + ] + ] + }, + "sync: sync, new, signed in with schema": { + "result": { + "escaped": "Error", + "message": "Call to undefined method Mock::createWithUuid()" + }, + "calls": [ + [ + "log.info", + "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", + { + "objectId": 17, + "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_2 - Extracted organization data", + { + "organizationUuid": "org-1", + "objectDataKeys": [ + "id", + "name", + "website", + "beoordeling", + "contactpersonen" + ], + "hasId": true + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - Getting OrganisationService", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - OrganisationService retrieved", + { + "serviceClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_4 - OpenRegister configuration", + { + "organizationUuid": "org-1", + "organizationName": "Gemeente A" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5 - Checking if organization exists in OpenRegister", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5A - Getting OrganisationMapper for lookup", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5B - Calling findByUuid", + { + "uuid": "org-1" + } + ], + [ + "mapper.findByUuid", + "org-1" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_8 - Organization not found in OpenRegister, creating", + { + "organizationId": "org-1", + "exception": "missing" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_9 - Mapping organization data for creation", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_10 - Calling createOrganisationInOpenRegister", + [] + ], + [ + "log.info", + "StackiqService: STEP 1 - Starting createOrganisationInOpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A", + "mappedDataKeys": [ + "name", + "type", + "website", + "active", + "contactpersonen", + "participants" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 2 - Checking user context", + { + "hasUserSession": true, + "currentUser": "signed-in", + "isAnonymous": false + } + ], + [ + "log.info", + "StackiqService: STEP 4A - Auth path: User logged in, creating org via mapper", + { + "organizationUuid": "org-1", + "currentUser": "signed-in" + } + ], + [ + "log.info", + "StackiqService: STEP 4B - Using original UUID format for OpenRegister", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 4C - Getting OrganisationMapper from container", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: STEP 4D - OrganisationMapper retrieved", + { + "mapperClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: Processing nested contact persons", + { + "organizationUuid": "org-1", + "nestedContactPersonCount": 0 + } + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\ObjectService" + ], + [ + "container.get", + "OCA\\Stackiq\\Service\\SettingsService" + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "objects.findAll", + { + "filters": { + "register": "6", + "schema": "9", + "organization": "org-1" + } + } + ], + [ + "log.info", + "StackiqService: Found existing contact persons for organization", + { + "organizationUuid": "org-1", + "contactPersonCount": 2, + "contactPersonIds": [ + 100, + 101 + ] + } + ], + [ + "log.info", + "StackiqService: Added existing contact person username", + { + "username": "b@a.nl", + "contactPersonId": 100 + } + ], + [ + "log.info", + "StackiqService: Added existing contact person username", + { + "username": "c@a.nl", + "contactPersonId": 101 + } + ], + [ + "log.info", + "StackiqService: Collected contact person usernames", + { + "organizationUuid": "org-1", + "totalUsernames": 2, + "usernames": [ + "b@a.nl", + "c@a.nl" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 4E - Debug - UUID before createWithUuid", + { + "organizationUuid": "org-1", + "uuidLength": 5, + "uuidIsEmpty": false, + "name": "Gemeente A", + "description": "https://a.nl", + "owner": "signed-in", + "users": [ + "signed-in", + "b@a.nl", + "c@a.nl" + ], + "contactPersonUsernames": [ + "b@a.nl", + "c@a.nl" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 4F - Calling organisationMapper->createWithUuid()", + [] + ], + [ + "log.info", + "StackiqService: STEP 4F_DEBUG - Parameters for createWithUuid", + { + "name": "Gemeente A", + "description": "https://a.nl", + "uuid": "org-1", + "owner": "signed-in", + "users": [ + "signed-in", + "b@a.nl", + "c@a.nl" + ], + "isDefault": false, + "uuidLength": 5, + "uuidIsEmpty": false + } + ] + ] + }, + "sync: sync, new, anonymous": { + "result": { + "escaped": "Error", + "message": "Call to undefined method OCA\\OpenRegister\\Db\\Organisation::setName()" + }, + "calls": [ + [ + "log.info", + "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", + { + "objectId": 17, + "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_2 - Extracted organization data", + { + "organizationUuid": "org-1", + "objectDataKeys": [ + "id", + "name", + "website", + "beoordeling", + "contactpersonen" + ], + "hasId": true + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - Getting OrganisationService", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_3 - OrganisationService retrieved", + { + "serviceClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_4 - OpenRegister configuration", + { + "organizationUuid": "org-1", + "organizationName": "Gemeente A" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5 - Checking if organization exists in OpenRegister", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5A - Getting OrganisationMapper for lookup", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_5B - Calling findByUuid", + { + "uuid": "org-1" + } + ], + [ + "mapper.findByUuid", + "org-1" + ], + [ + "log.info", + "StackiqService: SYNC_STEP_8 - Organization not found in OpenRegister, creating", + { + "organizationId": "org-1", + "exception": "missing" + } + ], + [ + "log.info", + "StackiqService: SYNC_STEP_9 - Mapping organization data for creation", + [] + ], + [ + "log.info", + "StackiqService: SYNC_STEP_10 - Calling createOrganisationInOpenRegister", + [] + ], + [ + "log.info", + "StackiqService: STEP 1 - Starting createOrganisationInOpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A", + "mappedDataKeys": [ + "name", + "type", + "website", + "active", + "contactpersonen", + "participants" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 2 - Checking user context", + { + "hasUserSession": true, + "currentUser": "null", + "isAnonymous": true + } + ], + [ + "log.info", + "StackiqService: STEP 3A - Anonymous path: No user, creating org directly via mapper", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 3B - Using original UUID format for OpenRegister (anonymous)", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 3C - Getting OrganisationMapper from container", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: STEP 3D - OrganisationMapper retrieved", + { + "mapperClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: STEP 3E - Creating new Organisation entity", + [] + ], + [ + "log.info", + "StackiqService: STEP 3F - Setting organisation properties", + { + "name": "Gemeente A", + "description": "https://a.nl", + "uuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: Processing nested contact persons", + { + "organizationUuid": "org-1", + "nestedContactPersonCount": 0 + } + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\ObjectService" + ], + [ + "container.get", + "OCA\\Stackiq\\Service\\SettingsService" + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.warning", + "StackiqService: Missing contactpersoon schema config for username extraction", + [] + ], + [ + "log.info", + "StackiqService: STEP 3F_2 - Collected usernames for organization", + { + "organizationUuid": "org-1", + "totalUsernames": 1, + "usernames": [ + "admin" + ] + } + ] + ] + }, + "create: create, no id": { + "result": null, + "calls": [ + [ + "log.error", + "StackiqService: No organization UUID provided for creation", + [] + ] + ] + }, + "create: create, signed in": { + "result": { + "escaped": "Error", + "message": "Call to undefined method Mock::createWithUuid()" + }, + "calls": [ + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: STEP 1 - Starting createOrganisationInOpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A", + "mappedDataKeys": [ + "name", + "type", + "website", + "active", + "contactpersonen", + "participants" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 2 - Checking user context", + { + "hasUserSession": true, + "currentUser": "signed-in", + "isAnonymous": false + } + ], + [ + "log.info", + "StackiqService: STEP 4A - Auth path: User logged in, creating org via mapper", + { + "organizationUuid": "org-1", + "currentUser": "signed-in" + } + ], + [ + "log.info", + "StackiqService: STEP 4B - Using original UUID format for OpenRegister", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 4C - Getting OrganisationMapper from container", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.info", + "StackiqService: STEP 4D - OrganisationMapper retrieved", + { + "mapperClass": "Mock" + } + ], + [ + "log.info", + "StackiqService: Processing nested contact persons", + { + "organizationUuid": "org-1", + "nestedContactPersonCount": 3 + } + ], + [ + "log.info", + "StackiqService: Added nested contact person username", + { + "username": "a@a.nl", + "contactPersonData": { + "email": "a@a.nl" + } + } + ], + [ + "container.get", + "OCA\\OpenRegister\\Service\\ObjectService" + ], + [ + "container.get", + "OCA\\Stackiq\\Service\\SettingsService" + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "log.warning", + "StackiqService: Missing contactpersoon schema config for username extraction", + [] + ], + [ + "log.info", + "StackiqService: STEP 4E - Debug - UUID before createWithUuid", + { + "organizationUuid": "org-1", + "uuidLength": 5, + "uuidIsEmpty": false, + "name": "Gemeente A", + "description": "https://a.nl", + "owner": "signed-in", + "users": [ + "signed-in", + "a@a.nl" + ], + "contactPersonUsernames": [ + "a@a.nl" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 4F - Calling organisationMapper->createWithUuid()", + [] + ], + [ + "log.info", + "StackiqService: STEP 4F_DEBUG - Parameters for createWithUuid", + { + "name": "Gemeente A", + "description": "https://a.nl", + "uuid": "org-1", + "owner": "signed-in", + "users": [ + "signed-in", + "a@a.nl" + ], + "isDefault": false, + "uuidLength": 5, + "uuidIsEmpty": false + } + ] + ] + }, + "create: create, organisation service down": { + "result": null, + "calls": [ + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.error", + "StackiqService: Error in public createOrganisationInOpenRegister", + { + "error": "organisation service down", + "objectData": { + "id": "org-1", + "name": "Gemeente A", + "website": "https://a.nl", + "beoordeling": "actief", + "contactpersonen": [ + { + "email": "a@a.nl" + }, + { + "naam": "zonder mail" + }, + "c-2" + ] + } + } + ] + ] + }, + "create: create, anonymous, no mapper": { + "result": null, + "calls": [ + [ + "container.get", + "OCA\\OpenRegister\\Service\\OrganisationService" + ], + [ + "log.info", + "StackiqService: STEP 1 - Starting createOrganisationInOpenRegister", + { + "organizationUuid": "org-1", + "name": "Gemeente A", + "mappedDataKeys": [ + "name", + "type", + "website", + "active", + "contactpersonen", + "participants" + ] + } + ], + [ + "log.info", + "StackiqService: STEP 2 - Checking user context", + { + "hasUserSession": true, + "currentUser": "null", + "isAnonymous": true + } + ], + [ + "log.info", + "StackiqService: STEP 3A - Anonymous path: No user, creating org directly via mapper", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 3B - Using original UUID format for OpenRegister (anonymous)", + { + "organizationUuid": "org-1" + } + ], + [ + "log.info", + "StackiqService: STEP 3C - Getting OrganisationMapper from container", + [] + ], + [ + "container.get", + "OCA\\OpenRegister\\Db\\OrganisationMapper" + ], + [ + "log.error", + "Failed to get OrganisationMapper: no mapper", + [] + ], + [ + "log.error", + "StackiqService: Error in public createOrganisationInOpenRegister", + { + "error": "OpenRegister OrganisationMapper is not available", + "objectData": { + "id": "org-1", + "name": "Gemeente A", + "website": "https://a.nl", + "beoordeling": "actief", + "contactpersonen": [ + { + "email": "a@a.nl" + }, + { + "naam": "zonder mail" + }, + "c-2" + ] + } + } + ] + ] + } +} From e5ec6e3a81ca9ab15860769fc3ee7189acab16c5 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 15:36:39 +0100 Subject: [PATCH 6/7] refactor(stackiq): drop the unused organisation service parameter from the OpenRegister create and update steps Neither createOrganisationInOpenRegisterInternal nor updateOrganisationInOpenRegister reads the OrganisationService it was handed. The public create wrapper still fetches it first, so a missing OpenRegister fails at the same point. The golden masks mock and anonymous class names, which depend on test order; it was re-recorded on the code before this commit. Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/StackiqService.php | 16 +++------------- .../StackiqServiceOrganisationSyncGoldenTest.php | 5 +++-- .../golden/stackiqServiceOrganisationSync.json | 16 ++++++++-------- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/lib/Service/StackiqService.php b/lib/Service/StackiqService.php index 7fa7bcc41..c918112dd 100644 --- a/lib/Service/StackiqService.php +++ b/lib/Service/StackiqService.php @@ -1364,7 +1364,6 @@ public function syncOrganizationWithOpenRegister(object $organizationObject): bo // Update the organization using OrganisationService. $updatedOrganisation = $this->updateOrganisationInOpenRegister( - organisationService: $organisationService, existingOrganisation: $existingOrganisation, mappedData: $mappedData ); @@ -1395,7 +1394,6 @@ public function syncOrganizationWithOpenRegister(object $organizationObject): bo // Create the organization using OrganisationService. $this->_logger->info('StackiqService: SYNC_STEP_10 - Calling createOrganisationInOpenRegister'); $createdOrganisation = $this->createOrganisationInOpenRegisterInternal( - organisationService: $organisationService, mappedData: $mappedData, organizationUuid: $organizationUuid ); @@ -1452,11 +1450,10 @@ public function createOrganisationInOpenRegister(array $objectData): ?object { 'participants' => $objectData['participants'] ?? [], ]; - // Get organisation service. - $organisationService = $this->_container->get('OCA\OpenRegister\Service\OrganisationService'); + // Fails early when OpenRegister's organisation service is not available. + $this->_container->get('OCA\OpenRegister\Service\OrganisationService'); return $this->createOrganisationInOpenRegisterInternal( - organisationService: $organisationService, mappedData: $mappedData, organizationUuid: $organizationUuid ); @@ -1494,17 +1491,12 @@ private function mapStatus(string $status): bool { /** * Creates an organization in OpenRegister using OrganisationService * - * @param \OCA\OpenRegister\Service\OrganisationService $organisationService The OpenRegister organisation service * @param array $mappedData The mapped organization data * @param string $organizationUuid The organization UUID to use * * @return \OCA\OpenRegister\Db\Organisation The created organization */ - private function createOrganisationInOpenRegisterInternal( - \OCA\OpenRegister\Service\OrganisationService $organisationService, - array $mappedData, - string $organizationUuid, - ): \OCA\OpenRegister\Db\Organisation { + private function createOrganisationInOpenRegisterInternal(array $mappedData, string $organizationUuid): \OCA\OpenRegister\Db\Organisation { $this->_logger->info( 'StackiqService: STEP 1 - Starting createOrganisationInOpenRegister', [ @@ -1778,14 +1770,12 @@ private function createOrganisationInOpenRegisterInternal( /** * Updates an organization in OpenRegister using OrganisationService * - * @param \OCA\OpenRegister\Service\OrganisationService $organisationService The OpenRegister organisation service * @param \OCA\OpenRegister\Db\Organisation $existingOrganisation The existing organization * @param array $mappedData The mapped organization data * * @return \OCA\OpenRegister\Db\Organisation The updated organization */ private function updateOrganisationInOpenRegister( - \OCA\OpenRegister\Service\OrganisationService $organisationService, \OCA\OpenRegister\Db\Organisation $existingOrganisation, array $mappedData, ): \OCA\OpenRegister\Db\Organisation { diff --git a/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php b/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php index 1f49ea4d8..f94dd2cd1 100644 --- a/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php +++ b/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php @@ -6,7 +6,8 @@ * Records, per case, every container, mapper, organisation entity, object * service, settings and logger call that syncOrganizationWithOpenRegister and * createOrganisationInOpenRegister make, plus their result or the error that - * escapes. Recorded with RECORD_GOLDEN=1. + * escapes. Mock and anonymous class names are masked, as they depend on the + * order tests run in. Recorded with RECORD_GOLDEN=1. * * @category Test * @package OCA\Stackiq\Tests\Unit\Service @@ -352,7 +353,7 @@ public function log($level, string|\Stringable $message, array $context = []): v $context, function (&$value) { if (is_string($value) === true) { - $value = preg_replace('/Mock\w+/', 'Mock', $value); + $value = preg_replace(['/Mock\w+/', '/@anonymous.*/s'], ['Mock', '@anonymous'], $value); } } ); diff --git a/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json b/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json index 9d2ec8795..e7f29d151 100644 --- a/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json +++ b/tests/Unit/Service/golden/stackiqServiceOrganisationSync.json @@ -7,7 +7,7 @@ "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", { "objectId": 17, - "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + "objectClass": "class@anonymous" } ], [ @@ -45,7 +45,7 @@ "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", { "objectId": 17, - "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + "objectClass": "class@anonymous" } ], [ @@ -117,7 +117,7 @@ "StackiqService: SYNC_STEP_6 - Organization exists in OpenRegister, updating", { "organizationId": "org-1", - "existingOrganisationClass": "OCA\\OpenRegister\\Db\\Organisation@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:243$c0" + "existingOrganisationClass": "OCA\\OpenRegister\\Db\\Organisation@anonymous" } ], [ @@ -173,7 +173,7 @@ "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", { "objectId": 17, - "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + "objectClass": "class@anonymous" } ], [ @@ -245,7 +245,7 @@ "StackiqService: SYNC_STEP_6 - Organization exists in OpenRegister, updating", { "organizationId": "org-1", - "existingOrganisationClass": "OCA\\OpenRegister\\Db\\Organisation@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:243$c0" + "existingOrganisationClass": "OCA\\OpenRegister\\Db\\Organisation@anonymous" } ], [ @@ -299,7 +299,7 @@ "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", { "objectId": 17, - "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + "objectClass": "class@anonymous" } ], [ @@ -514,7 +514,7 @@ "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", { "objectId": 17, - "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + "objectClass": "class@anonymous" } ], [ @@ -781,7 +781,7 @@ "StackiqService: SYNC_STEP_1 - Starting syncOrganizationWithOpenRegister", { "objectId": 17, - "objectClass": "class@anonymous\u0000/home/rubenlinde/sig-lanes/sq3/stackiq/tests/Unit/Service/StackiqServiceOrganisationSyncGoldenTest.php:298$c1" + "objectClass": "class@anonymous" } ], [ From 96c21899ccc123f5c20afac0c138aa2a042fd0e9 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 20:33:06 +0100 Subject: [PATCH 7/7] fix(organisations): never take over an existing organisation on a name 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 ` 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 --- appinfo/info.xml | 4 + lib/Command/OrganisationLinkCommand.php | 111 ++++ lib/Service/OrganisatieService.php | 13 + lib/Service/OrganisationSlug.php | 56 ++ lib/Service/OrganizationSyncService.php | 193 ++++-- lib/Service/Stackiq/ContactPersonHandler.php | 50 +- tests/Stubs/Db/OrganisationMapper.php | 9 + .../OrganisationSlugConflictException.php | 35 ++ tests/Stubs/Service/OrganisationService.php | 17 + .../Command/OrganisationLinkCommandTest.php | 81 +++ .../OrganisatieServiceSlugConflictTest.php | 89 +++ ...rganizationSyncContactPersonGoldenTest.php | 1 + .../OrganizationSyncEntityGoldenTest.php | 84 +++ ...anizationSyncRelatedContactsGoldenTest.php | 1 + .../ContactPersonHandlerSlugConflictTest.php | 316 ++++++++++ .../golden/organizationSyncEntity.json | 554 +++++++++++++++--- 16 files changed, 1483 insertions(+), 131 deletions(-) create mode 100644 lib/Command/OrganisationLinkCommand.php create mode 100644 lib/Service/OrganisationSlug.php create mode 100644 tests/Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php create mode 100644 tests/Unit/Command/OrganisationLinkCommandTest.php create mode 100644 tests/Unit/Service/OrganisatieServiceSlugConflictTest.php create mode 100644 tests/Unit/Service/Stackiq/ContactPersonHandlerSlugConflictTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index a1b6820d6..bb0cc6f63 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -344,6 +344,10 @@ Vrij en open source onder de EUPL-licentie. shared `contractNumber`: a wrong cross-app link is worse than an empty one. --> OCA\Stackiq\Command\ReferencesAuditCommand + + OCA\Stackiq\Command\OrganisationLinkCommand diff --git a/lib/Command/OrganisationLinkCommand.php b/lib/Command/OrganisationLinkCommand.php new file mode 100644 index 000000000..bf97111da --- /dev/null +++ b/lib/Command/OrganisationLinkCommand.php @@ -0,0 +1,111 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Command; + +use OCA\Stackiq\Service\OrganizationSyncService; +use Psr\Container\ContainerInterface; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Throwable; + +/** + * `occ stackiq:organisation:link `. + * + * The organisation sync never takes over an OpenRegister organisation whose + * name gives the same slug: it leaves the organisatie unlinked and logs it. + * This command is how an administrator links the two when they are the same. + * + * @spec openspec/specs/organisatie-service/spec.md + * + * phpcs:disable CustomSniffs.Functions.NamedParameters + */ +class OrganisationLinkCommand extends Command { + + /** + * What each outcome means, for the console. + */ + private const MESSAGES = [ + 'linked' => 'Linked. The organisation now carries the organisatie uuid and owns the organisatie.', + 'no-organisatie' => 'No organisatie has this uuid.', + 'already-linked' => 'This organisatie already has an OpenRegister organisation.', + 'no-organisation' => 'No OpenRegister organisation has this uuid.', + 'organisation-in-use' => 'This OpenRegister organisation belongs to another organisatie.', + ]; + + /** + * Build the command. + * + * @param ContainerInterface $container Container, for the lazy sync service resolve. + */ + public function __construct( + private readonly ContainerInterface $container, + ) { + parent::__construct(); + }//end __construct() + + /** + * Declare the command. + * + * @return void + * + * @spec openspec/specs/organisatie-service/spec.md + */ + protected function configure(): void { + $this->setName('stackiq:organisation:link') + ->setDescription('Link an organisatie to an existing OpenRegister organisation.') + ->addArgument('organisatie', InputArgument::REQUIRED, 'The organisatie object uuid.') + ->addArgument('organisation', InputArgument::REQUIRED, 'The OpenRegister organisation uuid.'); + }//end configure() + + /** + * Make the link, or say why not. + * + * @param InputInterface $input Console input. + * @param OutputInterface $output Console output. + * + * @return int 0 when linked, 1 otherwise. + * + * @spec openspec/specs/organisatie-service/spec.md + */ + protected function execute(InputInterface $input, OutputInterface $output): int { + try { + $outcome = $this->container->get(OrganizationSyncService::class)->linkOrganisationEntity( + organisatieUuid: (string) $input->getArgument('organisatie'), + entityUuid: (string) $input->getArgument('organisation') + ); + } catch (Throwable $e) { + $output->writeln(messages: 'Could not link: ' . $e->getMessage() . ''); + return 1; + } + + $output->writeln(messages: self::MESSAGES[$outcome] ?? $outcome); + if ($outcome === 'linked') { + return 0; + } + + return 1; + }//end execute() +}//end class diff --git a/lib/Service/OrganisatieService.php b/lib/Service/OrganisatieService.php index 7c77a9a23..b76a4a36a 100644 --- a/lib/Service/OrganisatieService.php +++ b/lib/Service/OrganisatieService.php @@ -42,6 +42,12 @@ * @SuppressWarnings(PHPMD.UnusedPrivateMethod) */ class OrganisatieService { + /** + * OpenRegister's exception for a taken slug, by name (OrganisationSlug::CONFLICT_EXCEPTION; + * not imported, to keep this class's coupling down). + */ + private const SLUG_CONFLICT = 'OCA\\OpenRegister\\Exception\\OrganisationSlugConflictException'; + /** * OrganisatieService constructor. * @@ -71,6 +77,8 @@ public function __construct( * * @return object|null The created organisation entity or null on failure * + * OpenRegister's OrganisationSlugConflictException (another organisation has the name's slug) is passed on. + * * @spec openspec/specs/organisatie-service/spec.md */ public function createOrganisationInOpenRegister(array $objectData): ?object { @@ -118,6 +126,11 @@ public function createOrganisationInOpenRegister(array $objectData): ?object { return $organisationEntity; } catch (\Exception $e) { + // The caller decides what a taken slug means; every other failure ends here. + if (is_a($e, self::SLUG_CONFLICT) === true) { + throw $e; + } + $this->logger->error( 'OrganisatieService: Error creating organization entity', [ diff --git a/lib/Service/OrganisationSlug.php b/lib/Service/OrganisationSlug.php new file mode 100644 index 000000000..52cd6ee2e --- /dev/null +++ b/lib/Service/OrganisationSlug.php @@ -0,0 +1,56 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Service; + +/** + * Mirrors OpenRegister's OrganisationService::generateSlug(): lower case, runs of + * other characters as one hyphen, no hyphen at either end, at most 100 characters. + * It also names OpenRegister's exception for a taken slug. + * + * @spec openspec/specs/organisatie-service/spec.md + */ +class OrganisationSlug { + + /** + * OpenRegister's exception for a new organisation whose slug is taken (openregister#5462). + * Named as a string: older OpenRegister releases do not have the class. + */ + public const CONFLICT_EXCEPTION = 'OCA\\OpenRegister\\Exception\\OrganisationSlugConflictException'; + + /** + * OpenRegister keeps the first 100 characters of a slug. + */ + private const MAX_LENGTH = 100; + + /** + * The slug of an organisation name. + * + * @param string $name The organisation name. + * + * @return string The slug. + * + * @spec openspec/specs/organisatie-service/spec.md + */ + public function fromName(string $name): string { + $slug = trim((string) preg_replace('/[^a-z0-9]+/', '-', strtolower($name)), '-'); + return substr($slug, 0, self::MAX_LENGTH); + }//end fromName() +}//end class diff --git a/lib/Service/OrganizationSyncService.php b/lib/Service/OrganizationSyncService.php index 7cc7e4d0a..7a3a6332c 100644 --- a/lib/Service/OrganizationSyncService.php +++ b/lib/Service/OrganizationSyncService.php @@ -992,7 +992,11 @@ private function ensureOrganisationEntity(object $organisationObject, array $run $organisationEntity = $this->organisationMapper->findByUuid($organisationId); return $this->refreshOrganisationEntity(organisationEntity: $organisationEntity, run: $run); } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - return $this->adoptOrganisationEntityBySlug(run: $run) ?? $this->createOrganisationEntity(run: $run); + if ($this->slugIsTaken(run: $run) === true) { + return null; + } + + return $this->createOrganisationEntity(run: $run); } } catch (\Exception $e) { $this->logger->error( @@ -1100,48 +1104,159 @@ private function refreshOrganisationEntity(object $organisationEntity, array $ru }//end refreshOrganisationEntity() /** - * Adopt an entity that has the organisation's slug but another uuid: it takes the object's uuid. + * Link an organisatie to an existing OpenRegister organisation, as an administrator decides. * - * @param array $run The ensure run (see ensureOrganisationEntity). + * The organisation takes the organisatie's uuid, which is what links the two, + * and becomes the organisatie object's owner. Refused when the organisatie is + * unknown or already has an organisation, when the organisation is unknown, or + * when the organisation is already another organisatie's. * - * @return object|null The adopted entity, or null when the object has no name or no entity has its slug + * @param string $organisatieUuid The organisatie object's uuid. + * @param string $entityUuid The OpenRegister organisation's uuid. + * + * @return string linked, no-organisatie, already-linked, no-organisation or organisation-in-use. + * + * @spec openspec/specs/organisatie-service/spec.md */ - private function adoptOrganisationEntityBySlug(array $run): ?object { - // Entity not found by UUID — try finding by slug before creating. - // This handles the case where the entity exists but with a different UUID (e.g. from slug collision). - $orgName = ($run['objectData']['name'] ?? ''); - if (empty($orgName) === true) { - return null; + public function linkOrganisationEntity(string $organisatieUuid, string $entityUuid): string { + $config = $this->settingsService->getVoorzieningenConfig(); + $run = [ + 'register' => (string) ($config['register'] ?? ''), + 'organizationSchema' => (string) ($config['organisatie_schema'] ?? ''), + ]; + $run['organisationObject'] = $this->findOrganisatie(run: $run, uuid: $organisatieUuid); + if ($run['organisationObject'] === null) { + return 'no-organisatie'; + } + + $outcome = $this->linkRefusal(run: $run, uuids: [$organisatieUuid, $entityUuid]); + if ($outcome !== null) { + return $outcome; + } + + $entity = $this->organisationMapper->findByUuid($entityUuid); + $entity->setUuid($organisatieUuid); + $this->organisationMapper->save($entity); + $this->makeEntityOwner(organisationEntity: $entity, run: $run); + $this->logger->info( + 'OrganizationSyncService: an administrator linked an organisatie to an OpenRegister organisation', + ['app' => 'stackiq', 'organisatieId' => $organisatieUuid, 'previousOrganisationUuid' => $entityUuid] + ); + + return 'linked'; + }//end linkOrganisationEntity() + + /** + * Why a link may not be made, or null when it may. + * + * @param array $run The register and organisatie schema. + * @param array{0: string, 1: string} $uuids The organisatie uuid and the organisation uuid. + * + * @return string|null already-linked, no-organisation, organisation-in-use, or null. + */ + private function linkRefusal(array $run, array $uuids): ?string { + [$organisatieUuid, $entityUuid] = $uuids; + if ($this->organisationExists(uuid: $organisatieUuid) === true) { + return 'already-linked'; + } + + if ($this->organisationExists(uuid: $entityUuid) === false) { + return 'no-organisation'; } - $slug = strtolower(preg_replace('/[^a-z0-9]+/', '-', strtolower($orgName))); - $slug = trim($slug, '-'); + if ($this->findOrganisatie(run: $run, uuid: $entityUuid) !== null) { + return 'organisation-in-use'; + } + + return null; + }//end linkRefusal() + + /** + * Whether an OpenRegister organisation has this uuid. + * + * @param string $uuid The uuid. + * + * @return bool + */ + private function organisationExists(string $uuid): bool { try { - $organisationEntity = $this->organisationMapper->findBySlug($slug); - $this->logger->info( - 'OrganizationSyncService: Found existing entity by slug, updating UUID to match object', - [ - 'app' => 'stackiq', - 'organisatieId' => $run['organisationId'], - 'oldEntityUuid' => $organisationEntity->getUuid(), - 'slug' => $slug, - ] + $this->organisationMapper->findByUuid($uuid); + return true; + } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { + return false; + } + }//end organisationExists() + + /** + * The organisatie object with this uuid, or null. + * + * @param array $run The register and organisatie schema. + * @param string $uuid The uuid. + * + * @return object|null + */ + private function findOrganisatie(array $run, string $uuid): ?object { + try { + return $this->objectService->find( + id: $uuid, + register: $run['register'], + schema: $run['organizationSchema'], + _rbac: false, + _multitenancy: false ); + } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { + return null; + } + }//end findOrganisatie() - // Update the entity's UUID to match the object UUID so future lookups work. - $organisationEntity->setUuid($run['organisationId']); - $this->organisationMapper->save($organisationEntity); - $run['stats']['entitiesUpdated']++; + /** + * Whether another OpenRegister organisation already has the slug this organisatie's name gives. + * + * Such an organisation is never taken over on a name match: the organisatie is + * left without an entity, logged and counted, for an administrator to link + * explicitly (occ stackiq:organisation:link). A missing or "Unknown" name gets a + * uuid-based name when the entity is created, so it is not checked. + * + * @param array $run The ensure run (see ensureOrganisationEntity). + * + * @return bool True when the slug is taken and nothing may be created. + */ + private function slugIsTaken(array $run): bool { + $orgName = (string) ($run['objectData']['name'] ?? ''); + if ($orgName === '' || $orgName === 'Unknown') { + return false; + } - // Update organisatie object owner to this entity. - $this->makeEntityOwner(organisationEntity: $organisationEntity, run: $run); + $slug = (new OrganisationSlug())->fromName(name: $orgName); + try { + $existing = $this->organisationMapper->findBySlug($slug); + } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { + return false; + } - return $organisationEntity; - } catch (\OCP\AppFramework\Db\DoesNotExistException $slugEx) { - // Not found by slug either — proceed with creation. - return null; - }//end try - }//end adoptOrganisationEntityBySlug() + $this->refuseSlugConflict(run: $run, existingUuid: $existing->getUuid()); + return true; + }//end slugIsTaken() + + /** + * Log and count an organisatie left unlinked because its slug is taken. + * + * @param array $run The ensure run (see ensureOrganisationEntity). + * @param string|null $existingUuid The uuid of the organisation holding the slug, when known. + * + * @return void + */ + private function refuseSlugConflict(array $run, ?string $existingUuid): void { + $this->logger->warning( + 'OrganizationSyncService: an organisation with this name already exists; the organisatie is left unlinked for an administrator', + [ + 'app' => 'stackiq', + 'organisatieId' => $run['organisationId'], + 'existingOrganisationUuid' => $existingUuid, + ] + ); + $run['stats']['slugConflicts'][] = $run['organisationId']; + }//end refuseSlugConflict() /** * Create the organisation entity, mail the new organisation and make the entity the object's owner. @@ -1161,7 +1276,17 @@ private function createOrganisationEntity(array $run): ?object { ] ); - $organisationEntity = $this->organisationService->createOrganisationInOpenRegister($run['objectData']); + try { + $organisationEntity = $this->organisationService->createOrganisationInOpenRegister($run['objectData']); + } catch (\RuntimeException $e) { + if (is_a($e, OrganisationSlug::CONFLICT_EXCEPTION) === false) { + throw $e; + } + + $this->refuseSlugConflict(run: $run, existingUuid: null); + return null; + } + if (empty($organisationEntity) === true) { $this->logger->error( '❌ ORGANISATION ENTITY CREATION FAILED', diff --git a/lib/Service/Stackiq/ContactPersonHandler.php b/lib/Service/Stackiq/ContactPersonHandler.php index 13d312fad..ca4717a72 100644 --- a/lib/Service/Stackiq/ContactPersonHandler.php +++ b/lib/Service/Stackiq/ContactPersonHandler.php @@ -18,6 +18,7 @@ namespace OCA\Stackiq\Service\Stackiq; +use OCA\Stackiq\Service\OrganisationSlug; use OCA\Stackiq\Service\SymfonyEmailService; use OCP\App\IAppManager; use OCP\IAppConfig; @@ -2396,9 +2397,10 @@ public function addUserToOrganizationEntity( $organisation = $this->ensureOrganizationEntity(organizationUuid: $organizationUuid); + // The reason there is none is logged by ensureOrganizationEntity(). if ($organisation === null) { - $this->_logger->error( - 'ContactPersonHandler: Failed to create organization entity', + $this->_logger->info( + 'ContactPersonHandler: No organization entity, user not added', [ 'organizationUuid' => $organizationUuid, ] @@ -2522,23 +2524,14 @@ private function ensureOrganizationEntity(string $organizationUuid): ?\OCA\OpenR $description = $organizationData['description'] ?? $organizationData['longDescription'] ?? null; $organizationDescription = $description ?? $organizationData['description'] ?? ''; - // Check if an entity with the same slug already exists (prevents unique constraint violation). + // Another organisation with this name's slug is never taken over: the + // organisatie stays without an entity until an administrator links it. try { - $slug = strtolower(preg_replace('/[^a-z0-9]+/', '-', strtolower($organizationName))); - $slug = trim($slug, '-'); - $existingBySlug = $organisationMapper->findBySlug($slug); - $this->_logger->info( - 'ContactPersonHandler: Organization entity already exists (found by slug)', - [ - 'organizationUuid' => $organizationUuid, - 'slug' => $slug, - 'existingUuid' => $existingBySlug->getUuid(), - 'existingName' => $existingBySlug->getName(), - ] - ); - return $existingBySlug; + $existingBySlug = $organisationMapper->findBySlug((new OrganisationSlug())->fromName(name: $organizationName)); + $this->logSlugConflict(organizationUuid: $organizationUuid, existingUuid: $existingBySlug->getUuid()); + return null; } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - // No existing entity by slug — proceed with creation. + // No organisation has this slug: create one. } $this->_logger->info( @@ -2572,6 +2565,11 @@ private function ensureOrganizationEntity(string $organizationUuid): ?\OCA\OpenR return $organisation; } catch (\Exception $e) { + if (is_a($e, OrganisationSlug::CONFLICT_EXCEPTION) === true) { + $this->logSlugConflict(organizationUuid: $organizationUuid, existingUuid: null); + return null; + } + $this->_logger->error( 'ContactPersonHandler: Failed to ensure organization entity', [ @@ -2586,6 +2584,24 @@ private function ensureOrganizationEntity(string $organizationUuid): ?\OCA\OpenR }//end try }//end ensureOrganizationEntity() + /** + * Log an organisatie left without an entity because its name's slug is taken. + * + * @param string $organizationUuid The organisatie uuid. + * @param string|null $existingUuid The uuid of the organisation holding the slug, when known. + * + * @return void + */ + private function logSlugConflict(string $organizationUuid, ?string $existingUuid): void { + $this->_logger->warning( + 'ContactPersonHandler: an organisation with this name already exists; the organisatie is left unlinked for an administrator', + [ + 'organizationUuid' => $organizationUuid, + 'existingOrganisationUuid' => $existingUuid, + ] + ); + }//end logSlugConflict() + /** * Strips spaces and non-alphanumeric characters from voornaam/achternaam * and lowercases the result so they can be safely concatenated into a diff --git a/tests/Stubs/Db/OrganisationMapper.php b/tests/Stubs/Db/OrganisationMapper.php index e76182296..6adac68d8 100644 --- a/tests/Stubs/Db/OrganisationMapper.php +++ b/tests/Stubs/Db/OrganisationMapper.php @@ -49,4 +49,13 @@ abstract public function findBySlug(string $slug): Organisation; */ abstract public function save(Organisation $entity): Organisation; + /** + * Update an organisation entity. + * + * @param Organisation $entity The entity to update. + * + * @return Organisation + */ + abstract public function update(Organisation $entity): Organisation; + }//end class diff --git a/tests/Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php b/tests/Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php new file mode 100644 index 000000000..b969b65d9 --- /dev/null +++ b/tests/Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php @@ -0,0 +1,35 @@ + + * SPDX-License-Identifier: EUPL-1.2 + * + * @category Test + * @package OCA\Stackiq\Tests\Stubs + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Exception; + +use RuntimeException; + +/** + * A new organisation's slug is already taken. + */ +class OrganisationSlugConflictException extends RuntimeException { + + /** + * @param string $slug The taken slug. + * @param \Throwable|null $previous The cause. + */ + public function __construct(public readonly string $slug, ?\Throwable $previous = null) { + parent::__construct(message: 'An organisation with this name already exists.', code: 409, previous: $previous); + }//end __construct() +}//end class diff --git a/tests/Stubs/Service/OrganisationService.php b/tests/Stubs/Service/OrganisationService.php index d535023cf..bb1695b52 100644 --- a/tests/Stubs/Service/OrganisationService.php +++ b/tests/Stubs/Service/OrganisationService.php @@ -79,4 +79,21 @@ abstract public function joinOrganisation(string $organisationUuid, ?string $tar */ abstract public function leaveOrganisation(string $organisationUuid, ?string $targetUserId = null): bool; + /** + * Create an organisation; with addCurrentUser the signed-in user becomes its owner and a member. + * + * @param string $name The name. + * @param string $description The description. + * @param bool $addCurrentUser Whether the signed-in user owns and joins it. + * @param string $uuid The uuid to use. + * + * @return Organisation + */ + abstract public function createOrganisation( + string $name, + string $description = '', + bool $addCurrentUser = true, + string $uuid = '', + ): Organisation; + }//end class diff --git a/tests/Unit/Command/OrganisationLinkCommandTest.php b/tests/Unit/Command/OrganisationLinkCommandTest.php new file mode 100644 index 000000000..42fe7601e --- /dev/null +++ b/tests/Unit/Command/OrganisationLinkCommandTest.php @@ -0,0 +1,81 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/specs/organisatie-service/spec.md + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Command; + +use OCA\Stackiq\Command\OrganisationLinkCommand; +use OCA\Stackiq\Service\OrganizationSyncService; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use RuntimeException; +use Symfony\Component\Console\Tester\CommandTester; + +/** + * The command passes both uuids on and reports the outcome. + * + * @covers \OCA\Stackiq\Command\OrganisationLinkCommand + * + * @spec openspec/specs/organisatie-service/spec.md + */ +class OrganisationLinkCommandTest extends TestCase { + + /** + * Each outcome, its exit code and what the console says. + * + * @return array + */ + public static function outcomes(): array { + return [ + 'linked' => ['linked', 0, 'Linked.'], + 'no organisatie' => ['no-organisatie', 1, 'No organisatie has this uuid.'], + 'already linked' => ['already-linked', 1, 'already has an OpenRegister organisation'], + 'no organisation' => ['no-organisation', 1, 'No OpenRegister organisation has this uuid.'], + 'organisation in use' => ['organisation-in-use', 1, 'belongs to another organisatie'], + 'failure' => [new RuntimeException('db down'), 1, 'Could not link: db down'], + ]; + }//end outcomes() + + /** + * The outcome is reported with its exit code, for the uuids given. + * + * @param string|RuntimeException $outcome What the sync service answers. + * @param int $code The exit code. + * @param string $text Text the console shows. + * + * @return void + * + * @dataProvider outcomes + */ + public function testTheOutcomeIsReported(string|RuntimeException $outcome, int $code, string $text): void { + $sync = $this->createMock(OrganizationSyncService::class); + $call = $sync->expects(self::once())->method('linkOrganisationEntity')->with('org-uuid', 'or-uuid'); + if ($outcome instanceof RuntimeException) { + $call->willThrowException($outcome); + } else { + $call->willReturn($outcome); + } + + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->with(OrganizationSyncService::class)->willReturn($sync); + + $tester = new CommandTester(new OrganisationLinkCommand($container)); + $tester->execute(['organisatie' => 'org-uuid', 'organisation' => 'or-uuid']); + + self::assertSame($code, $tester->getStatusCode()); + self::assertStringContainsString($text, $tester->getDisplay()); + }//end testTheOutcomeIsReported() +}//end class diff --git a/tests/Unit/Service/OrganisatieServiceSlugConflictTest.php b/tests/Unit/Service/OrganisatieServiceSlugConflictTest.php new file mode 100644 index 000000000..f0b9eeb55 --- /dev/null +++ b/tests/Unit/Service/OrganisatieServiceSlugConflictTest.php @@ -0,0 +1,89 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/specs/organisatie-service/spec.md + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Service; + +use OCA\OpenRegister\Service\OrganisationService; +use OCA\Stackiq\Service\OrganisatieService; +use OCP\App\IAppManager; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Psr\Log\NullLogger; +use ReflectionClass; + +if (class_exists('OCA\\OpenRegister\\Exception\\OrganisationSlugConflictException') === false) { + require_once __DIR__ . '/../../Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php'; +} + +/** + * A taken slug reaches the caller; any other failure still ends as null. + * + * @covers \OCA\Stackiq\Service\OrganisatieService + * + * @spec openspec/specs/organisatie-service/spec.md + */ +class OrganisatieServiceSlugConflictTest extends TestCase { + + /** + * The refusal is passed on, not swallowed. + * + * @return void + */ + public function testATakenSlugIsPassedOn(): void { + $this->expectException(\OCA\OpenRegister\Exception\OrganisationSlugConflictException::class); + + $this->service(failure: new \OCA\OpenRegister\Exception\OrganisationSlugConflictException(slug: 'gemeente-a')) + ->createOrganisationInOpenRegister(['id' => 'org-1', 'name' => 'Gemeente A']); + }//end testATakenSlugIsPassedOn() + + /** + * Another failure still ends as null. + * + * @return void + */ + public function testAnotherFailureEndsAsNull(): void { + self::assertNull( + $this->service(failure: new \RuntimeException('db down'))->createOrganisationInOpenRegister(['id' => 'org-1', 'name' => 'Gemeente A']) + ); + }//end testAnotherFailureEndsAsNull() + + /** + * The service over an OpenRegister whose createOrganisation() throws. + * + * @param \Throwable $failure What createOrganisation() throws. + * + * @return OrganisatieService + */ + private function service(\Throwable $failure): OrganisatieService { + $organisations = $this->createMock(OrganisationService::class); + $organisations->method('getActiveOrganisation')->willReturn(null); + $organisations->method('createOrganisation')->willThrowException($failure); + + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->willReturn($organisations); + $apps = $this->createMock(IAppManager::class); + $apps->method('isEnabledForUser')->willReturn(true); + + $reflection = new ReflectionClass(OrganisatieService::class); + $service = $reflection->newInstanceWithoutConstructor(); + foreach (['container' => $container, 'logger' => new NullLogger(), 'appManager' => $apps] as $name => $value) { + $reflection->getProperty($name)->setValue($service, $value); + } + + return $service; + }//end service() +}//end class diff --git a/tests/Unit/Service/OrganizationSyncContactPersonGoldenTest.php b/tests/Unit/Service/OrganizationSyncContactPersonGoldenTest.php index 53f62cdd3..a3e40f167 100644 --- a/tests/Unit/Service/OrganizationSyncContactPersonGoldenTest.php +++ b/tests/Unit/Service/OrganizationSyncContactPersonGoldenTest.php @@ -40,6 +40,7 @@ * Golden cases for the contact person object sync. * * @covers \OCA\Stackiq\Service\OrganizationSyncService + * @uses \OCA\Stackiq\Service\OrganisationSlug * * @spec openspec/specs/organization-sync/spec.md */ diff --git a/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php b/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php index 4d9a86a69..c1f9336ff 100644 --- a/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php +++ b/tests/Unit/Service/OrganizationSyncEntityGoldenTest.php @@ -33,12 +33,17 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\IUser; use PHPUnit\Framework\TestCase; + +if (class_exists('OCA\\OpenRegister\\Exception\\OrganisationSlugConflictException') === false) { + require_once __DIR__ . '/../../Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php'; +} use Psr\Log\AbstractLogger; /** * Golden cases for the organisation entity sync. * * @covers \OCA\Stackiq\Service\OrganizationSyncService + * @uses \OCA\Stackiq\Service\OrganisationSlug * * @spec openspec/specs/organization-sync/spec.md */ @@ -294,6 +299,10 @@ function (...$args) { $mapper->method('findBySlug')->willReturnCallback( function (string $slug) { $this->trace[] = ['mapper.findBySlug', $slug]; + if (array_key_exists('slugIndex', $this->plan) === true) { + return $this->plan['slugIndex'][$slug] ?? throw new \OCP\AppFramework\Db\DoesNotExistException('no slug'); + } + $answer = array_shift($this->plan['slug']); if ($answer instanceof \Throwable) { throw $answer; @@ -385,6 +394,10 @@ private function organisationService(): OrganisatieService { $service->method('createOrganisationInOpenRegister')->willReturnCallback( function (array $data) { $this->trace[] = ['organisations.create', $data]; + if (($this->plan['created'] ?? null) instanceof \Throwable) { + throw $this->plan['created']; + } + return array_key_exists('created', $this->plan) === true ? $this->plan['created'] : $this->orgEntity(uuid: 'new-entity', active: true); } ); @@ -571,6 +584,11 @@ public static function entityCases(): array { 'mapper fails' => [['name' => 'Gemeente K'], true, fn ($t) => ['org' => [new \RuntimeException('db down')]]], 'owner save fails' => [['name' => 'Gemeente L'], true, fn ($t) => ['org' => [$t->orgEntity('o-1', true)], 'save' => [new \RuntimeException('save failed')]]], 'settings fail' => [['name' => 'Gemeente M'], true, fn ($t) => ['config' => new \RuntimeException('settings down')]], + 'same name as another organisation' => [['name' => 'Gemeente A', 'status' => 'actief'], true, fn ($t) => ['org' => [$missing()], 'slugIndex' => ['gemeente-a' => $t->orgEntity('o-other', true)]]], + 'long name, slug cut at 100' => [['name' => 'Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'status' => 'actief'], true, fn ($t) => ['org' => [$missing()], 'slugIndex' => ['gemeente-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' => $t->orgEntity('o-other', true)]]], + 'long name, slug free' => [['name' => 'Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'status' => 'actief'], true, fn ($t) => ['org' => [$missing()], 'slugIndex' => []]], + 'OpenRegister refuses the slug' => [['name' => 'Gemeente P', 'status' => 'actief'], true, fn ($t) => ['org' => [$missing()], 'slugIndex' => [], 'created' => new \OCA\OpenRegister\Exception\OrganisationSlugConflictException(slug: 'gemeente-p')]], + 'named Unknown' => [['name' => 'Unknown', 'status' => 'actief'], true, fn ($t) => ['org' => [$missing()], 'slugIndex' => ['unknown' => $t->orgEntity('o-other', true)]]], ]; }//end entityCases() @@ -601,6 +619,72 @@ public function testEnsureEntity(array $object, bool $sendEmails, \Closure $plan }//end testEnsureEntity() + /** + * A registration named like an existing organisation neither takes over that + * organisation (its uuid, its owner link) nor creates a second one. + * + * @return void + */ + public function testASameNamedRegistrationDoesNotCaptureTheExistingOrganisation(): void { + foreach (['Gemeente A' => 'gemeente-a', 'Gemeente ' . str_repeat('a', 120) => substr('gemeente-' . str_repeat('a', 120), 0, 100)] as $name => $slug) { + $this->trace = []; + $this->plan = [ + 'find' => [null], + 'save' => [], + 'org' => [new \OCP\AppFramework\Db\DoesNotExistException('missing')], + 'slugIndex' => [$slug => $this->orgEntity(uuid: 'o-other', active: true)], + ]; + $stats = ['entitiesCreated' => 0, 'entitiesUpdated' => 0]; + $entity = $this->service()->ensureOrganisationEntityPublic($this->entity(object: ['name' => $name, 'status' => 'actief'], uuid: 'o-1'), $stats, true); + + $writes = array_values(array_filter($this->trace, fn (array $call) => in_array($call[0], ['entity.setUuid', 'mapper.save', 'organisations.create', 'objects.saveObject', 'entity.setOwner', 'entity.setOrganisation'], true))); + self::assertNull($entity, $name); + self::assertSame([], $writes, $name); + self::assertSame(['o-1'], $stats['slugConflicts'] ?? null, $name); + } + }//end testASameNamedRegistrationDoesNotCaptureTheExistingOrganisation() + + /** + * The link outcomes, and what a link writes. + * + * @return array}> + */ + public static function linkCases(): array { + $missing = fn () => new \OCP\AppFramework\Db\DoesNotExistException('missing'); + $object = fn ($t, string $uuid) => $t->entity(object: ['name' => 'Gemeente A'], uuid: $uuid); + return [ + 'linked' => ['linked', fn ($t) => ['find' => [$object($t, 'o-1'), null], 'org' => [$missing(), $t->orgEntity('o-old', true), $t->orgEntity('o-old', true)]], ['entity.setUuid', 'mapper.save', 'objects.saveObject']], + 'no organisatie' => ['no-organisatie', fn ($t) => ['find' => [null]], []], + 'already linked' => ['already-linked', fn ($t) => ['find' => [$object($t, 'o-1')], 'org' => [$t->orgEntity('o-1', true)]], []], + 'no organisation' => ['no-organisation', fn ($t) => ['find' => [$object($t, 'o-1')], 'org' => [$missing(), $missing()]], []], + 'organisation in use' => ['organisation-in-use', fn ($t) => ['find' => [$object($t, 'o-1'), $object($t, 'o-old')], 'org' => [$missing(), $t->orgEntity('o-old', true)]], []], + ]; + }//end linkCases() + + /** + * An administrator's link writes only when every check passes. + * + * @param string $outcome The expected outcome. + * @param \Closure $plan Builds the doubles' answers. + * @param array $writes The writes expected, in order. + * + * @return void + * + * @dataProvider linkCases + */ + public function testLinkOrganisationEntity(string $outcome, \Closure $plan, array $writes): void { + $this->trace = []; + $this->plan = array_merge(['find' => [], 'save' => [], 'org' => [], 'slug' => []], $plan($this)); + + self::assertSame($outcome, $this->service()->linkOrganisationEntity('o-1', 'o-old')); + + $made = array_values(array_filter($this->trace, fn (array $call) => in_array($call[0], ['entity.setUuid', 'mapper.save', 'objects.saveObject'], true))); + self::assertSame($writes, array_column($made, 0)); + if ($outcome === 'linked') { + self::assertContains(['entity.setUuid', 'o-old', 'o-1'], $this->trace); + } + }//end testLinkOrganisationEntity() + /** * Compare a case with the golden file, or record it. * diff --git a/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php b/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php index 9928614e1..ce265069e 100644 --- a/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php +++ b/tests/Unit/Service/OrganizationSyncRelatedContactsGoldenTest.php @@ -39,6 +39,7 @@ * Golden cases for the related contact person sync. * * @covers \OCA\Stackiq\Service\OrganizationSyncService + * @uses \OCA\Stackiq\Service\OrganisationSlug * * @spec openspec/specs/organization-sync/spec.md */ diff --git a/tests/Unit/Service/Stackiq/ContactPersonHandlerSlugConflictTest.php b/tests/Unit/Service/Stackiq/ContactPersonHandlerSlugConflictTest.php new file mode 100644 index 000000000..64da8c454 --- /dev/null +++ b/tests/Unit/Service/Stackiq/ContactPersonHandlerSlugConflictTest.php @@ -0,0 +1,316 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/specs/sc-handlers/spec.md + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Service\Stackiq; + +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\OpenRegister\Db\ObjectEntity; +use OCA\OpenRegister\Db\Organisation; +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Service\Stackiq\ContactPersonHandler; +use OCA\Stackiq\Service\SymfonyEmailService; +use OCP\App\IAppManager; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IAppConfig; +use OCP\IConfig; +use OCP\IGroupManager; +use OCP\IUserManager; +use OCP\Security\ISecureRandom; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Psr\Log\AbstractLogger; + +if (class_exists('OCA\\OpenRegister\\Exception\\OrganisationSlugConflictException') === false) { + require_once __DIR__ . '/../../../Stubs/OpenRegister/Exception/OrganisationSlugConflictException.php'; +} + +/** + * The contact person's user joins no organisation it does not belong to. + * + * @covers \OCA\Stackiq\Service\Stackiq\ContactPersonHandler + * @uses \OCA\Stackiq\Service\OrganisationSlug + * + * @spec openspec/specs/sc-handlers/spec.md + */ +class ContactPersonHandlerSlugConflictTest extends TestCase { + + /** + * The slugs findBySlug() was asked for. + * + * @var array + */ + private array $slugs = []; + + /** + * The organisations written. + * + * @var array + */ + private array $writes = []; + + /** + * The log levels and messages. + * + * @var array + */ + public array $logs = []; + + /** + * The organisations created. + * + * @var array + */ + private array $created = []; + + /** + * A same-named organisation is not joined, and no second one is created. + * + * @return void + */ + public function testASameNamedOrganisationIsNotJoined(): void { + $name = 'Gemeente ' . str_repeat('a', 120); + $this->handler(name: $name, slugTakenBy: 'o-other')->addUserToOrganizationEntity($this->contact(), 'new@gemeente.nl'); + + self::assertSame([substr('gemeente-' . str_repeat('a', 120), 0, 100)], $this->slugs); + self::assertSame([], $this->writes); + self::assertSame([], $this->created); + }//end testASameNamedOrganisationIsNotJoined() + + /** + * OpenRegister refusing the taken slug leaves the user out as well. + * + * @return void + */ + public function testARefusedSlugJoinsNothing(): void { + $this->handler(name: 'Gemeente B', slugTakenBy: null, refuse: true)->addUserToOrganizationEntity($this->contact(), 'new@gemeente.nl'); + + self::assertSame(['Gemeente B'], $this->created); + self::assertSame([], $this->writes); + self::assertContains('warning', array_column($this->logs, 0), 'a refused slug is a neutral warning, not an error'); + self::assertNotContains('error', array_column($this->logs, 0)); + }//end testARefusedSlugJoinsNothing() + + /** + * A free slug creates the organisation and adds the user to it. + * + * @return void + */ + public function testAFreeSlugCreatesTheOrganisationAndAddsTheUser(): void { + $this->handler(name: 'Gemeente C', slugTakenBy: null)->addUserToOrganizationEntity($this->contact(), 'new@gemeente.nl'); + + self::assertSame(['gemeente-c'], $this->slugs); + self::assertSame(['Gemeente C'], $this->created); + self::assertSame(['org-1'], $this->writes); + }//end testAFreeSlugCreatesTheOrganisationAndAddsTheUser() + + /** + * A contact person of organisatie org-1. + * + * @return object + */ + private function contact(): object { + return new class() { + /** + * @return string + */ + public function getId(): string { + return 'contact-1'; + } + + /** + * @return array + */ + public function getObject(): array { + return ['organisation' => 'org-1']; + } + }; + }//end contact() + + /** + * The handler over an organisatie org-1 that has no entity yet. + * + * @param string $name The organisatie's name. + * @param string|null $slugTakenBy The uuid of an organisation holding the slug, or null when free. + * @param bool $refuse Whether OpenRegister refuses the create as a taken slug. + * + * @return ContactPersonHandler + */ + private function handler(string $name, ?string $slugTakenBy, bool $refuse = false): ContactPersonHandler { + $mapper = $this->createMock(OrganisationMapper::class); + $mapper->method('findByUuid')->willThrowException(new DoesNotExistException('missing')); + $mapper->method('findBySlug')->willReturnCallback( + function (string $slug) use ($slugTakenBy) { + $this->slugs[] = $slug; + if ($slugTakenBy === null) { + throw new DoesNotExistException('no slug'); + } + + return $this->organisation(uuid: $slugTakenBy); + } + ); + foreach (['update', 'save'] as $write) { + $mapper->method($write)->willReturnCallback( + function ($organisation) { + $this->writes[] = $organisation->getUuid(); + return $organisation; + } + ); + } + + $organisatie = $this->createMock(ObjectEntity::class); + $organisatie->method('getObject')->willReturn(['name' => $name]); + $objects = $this->createMock(ObjectServiceInterface::class); + $objects->method('find')->willReturn($organisatie); + + $organisations = new class($this, $refuse) { + /** + * @param ContactPersonHandlerSlugConflictTest $test The test. + * @param bool $refuse Whether to refuse. + */ + public function __construct(private ContactPersonHandlerSlugConflictTest $test, private bool $refuse) { + } + + /** + * @param mixed ...$args The organisation, by name. + * + * @return Organisation + */ + public function createOrganisation(...$args): Organisation { + return $this->test->create(name: (string) ($args['name'] ?? $args[0]), refuse: $this->refuse); + } + }; + + $settings = $this->createMock(SettingsService::class); + $settings->method('getVoorzieningenConfig')->willReturn(['register' => '5', 'organisatie_schema' => '1']); + + $apps = $this->createMock(IAppManager::class); + $apps->method('getInstalledApps')->willReturn(['openregister']); + + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->willReturnCallback( + fn (string $id) => [ + 'OCA\\OpenRegister\\Db\\OrganisationMapper' => $mapper, + 'OCA\OpenRegister\Service\ObjectService' => $objects, + 'OCA\Stackiq\Service\SettingsService' => $settings, + 'OCA\\OpenRegister\\Service\\OrganisationService' => $organisations, + ][$id] ?? null + ); + + return new ContactPersonHandler( + $this->createMock(IUserManager::class), + $this->createMock(ISecureRandom::class), + $this->createMock(IGroupManager::class), + $this->createMock(IAppConfig::class), + $container, + $apps, + $this->logger(), + $this->createMock(SymfonyEmailService::class), + $this->createMock(IConfig::class) + ); + }//end handler() + + /** + * A logger that keeps each level and message. + * + * @return AbstractLogger + */ + private function logger(): AbstractLogger { + return new class($this) extends AbstractLogger { + /** + * @param ContactPersonHandlerSlugConflictTest $test The test. + */ + public function __construct(private ContactPersonHandlerSlugConflictTest $test) { + } + + /** + * @param mixed $level The level. + * @param string|\Stringable $message The message. + * @param array $context The context. + * + * @return void + */ + public function log($level, string|\Stringable $message, array $context = []): void { + $this->test->logs[] = [(string) $level, (string) $message]; + } + }; + }//end logger() + + /** + * Record a create and answer it, or refuse it as a taken slug. + * + * @param string $name The organisation name. + * @param bool $refuse Whether to refuse. + * + * @return Organisation The created organisation. + */ + public function create(string $name, bool $refuse): Organisation { + $this->created[] = $name; + if ($refuse === true) { + throw new \OCA\OpenRegister\Exception\OrganisationSlugConflictException(slug: 'taken'); + } + + return $this->organisation(uuid: 'org-1'); + }//end create() + + /** + * An organisation entity double with users. + * + * @param string $uuid The uuid. + * + * @return Organisation + */ + private function organisation(string $uuid): Organisation { + return new class($uuid) extends Organisation { + /** + * @var array + */ + private array $members = []; + + /** + * @param string $entityUuid The uuid. + */ + public function __construct(private string $entityUuid) { + } + + /** + * @return string + */ + public function getUuid(): string { + return $this->entityUuid; + } + + /** + * @param string $name The method. + * @param array $args The arguments. + * + * @return mixed + */ + public function __call(string $name, array $args) { + if ($name === 'getUsers') { + return $this->members; + } + + if ($name === 'setUsers') { + $this->members = $args[0]; + } + + return null; + } + }; + }//end organisation() +}//end class diff --git a/tests/Unit/Service/golden/organizationSyncEntity.json b/tests/Unit/Service/golden/organizationSyncEntity.json index 02b4560ab..c3900893b 100644 --- a/tests/Unit/Service/golden/organizationSyncEntity.json +++ b/tests/Unit/Service/golden/organizationSyncEntity.json @@ -899,13 +899,13 @@ ] }, "entity: adopted by slug": { - "returned": [ - "o-1", - true - ], + "returned": null, "stats": { "entitiesCreated": 0, - "entitiesUpdated": 1 + "entitiesUpdated": 0, + "slugConflicts": [ + "o-1" + ] }, "trace": [ [ @@ -940,84 +940,12 @@ "gemeente-g-zn" ], [ - "log.info", - "OrganizationSyncService: Found existing entity by slug, updating UUID to match object", + "log.warning", + "OrganizationSyncService: an organisation with this name already exists; the organisatie is left unlinked for an administrator", { "app": "stackiq", "organisatieId": "o-1", - "oldEntityUuid": "o-old", - "slug": "gemeente-g-zn" - } - ], - [ - "entity.setUuid", - "o-old", - "o-1" - ], - [ - "mapper.save", - "o-1", - true - ], - [ - "log.info", - "OrganizationSyncService: Updating organisatie object owner and organisation", - { - "organisatieId": "o-1", - "organisationEntityUuid": "o-1", - "register": "reg-v", - "schema": "org-schema" - } - ], - [ - "entity.setObject", - "o-1", - { - "name": "Gemeente G & Zn!", - "status": "actief", - "organisation": "o-1", - "@self": { - "owner": "o-1", - "organisation": "o-1" - } - } - ], - [ - "entity.setOwner", - "o-1", - "o-1" - ], - [ - "entity.setOrganisation", - "o-1", - "o-1" - ], - [ - "objects.saveObject", - { - "object": { - "name": "Gemeente G & Zn!", - "status": "actief", - "organisation": "o-1", - "@self": { - "organisation": "o-1" - } - }, - "register": "reg-entity", - "schema": "schema-entity", - "uuid": "o-1", - "silent": true, - "_validation": false - } - ], - [ - "log.info", - "OrganizationSyncService: Successfully updated organisatie object owner and organisation", - { - "organisatieId": "o-1", - "organisationEntityUuid": "o-1", - "ownerSet": "o-1", - "organisationSet": "o-1" + "existingOrganisationUuid": "o-old" } ] ] @@ -1848,5 +1776,471 @@ } ] ] + }, + "entity: same name as another organisation": { + "returned": null, + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0, + "slugConflicts": [ + "o-1" + ] + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente A", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-a" + ], + [ + "log.warning", + "OrganizationSyncService: an organisation with this name already exists; the organisatie is left unlinked for an administrator", + { + "app": "stackiq", + "organisatieId": "o-1", + "existingOrganisationUuid": "o-other" + } + ] + ] + }, + "entity: long name, slug cut at 100": { + "returned": null, + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0, + "slugConflicts": [ + "o-1" + ] + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + ], + [ + "log.warning", + "OrganizationSyncService: an organisation with this name already exists; the organisatie is left unlinked for an administrator", + { + "app": "stackiq", + "organisatieId": "o-1", + "existingOrganisationUuid": "o-other" + } + ] + ] + }, + "entity: long name, slug free": { + "returned": [ + "new-entity", + true + ], + "stats": { + "entitiesCreated": 1, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + } + ], + [ + "organisations.create", + { + "name": "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "status": "actief" + } + ], + [ + "log.debug", + "New organisation entity created", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "active": true, + "name": "Entity new-entity" + } + ], + [ + "log.info", + "[FLOW] Sending organization registration email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationRegistrationEmail", + "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + ], + [ + "log.info", + "📧 Organization registration email sent successfully", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "status": "actief", + "organisation": "new-entity", + "@self": { + "owner": "new-entity", + "organisation": "new-entity" + } + } + ], + [ + "entity.setOwner", + "o-1", + "new-entity" + ], + [ + "entity.setOrganisation", + "o-1", + "new-entity" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Gemeente aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "status": "actief", + "organisation": "new-entity", + "@self": { + "organisation": "new-entity" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "ownerSet": "new-entity", + "organisationSet": "new-entity" + } + ] + ] + }, + "entity: OpenRegister refuses the slug": { + "returned": null, + "stats": { + "entitiesCreated": 0, + "entitiesUpdated": 0, + "slugConflicts": [ + "o-1" + ] + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente P", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "mapper.findBySlug", + "gemeente-p" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Gemeente P" + } + ], + [ + "organisations.create", + { + "name": "Gemeente P", + "status": "actief" + } + ], + [ + "log.warning", + "OrganizationSyncService: an organisation with this name already exists; the organisatie is left unlinked for an administrator", + { + "app": "stackiq", + "organisatieId": "o-1", + "existingOrganisationUuid": null + } + ] + ] + }, + "entity: named Unknown": { + "returned": [ + "new-entity", + true + ], + "stats": { + "entitiesCreated": 1, + "entitiesUpdated": 0 + }, + "trace": [ + [ + "objects.find", + { + "id": "o-1", + "register": "reg-entity", + "schema": "schema-entity", + "_rbac": false, + "_multitenancy": false + } + ], + [ + "log.debug", + "Ensuring organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Unknown", + "status": "actief" + } + ], + [ + "settings.getVoorzieningenConfig" + ], + [ + "mapper.findByUuid", + "o-1" + ], + [ + "log.debug", + "Creating new organisation entity", + { + "app": "stackiq", + "organisatieId": "o-1", + "name": "Unknown" + } + ], + [ + "organisations.create", + { + "name": "Unknown", + "status": "actief" + } + ], + [ + "log.debug", + "New organisation entity created", + { + "app": "stackiq", + "organisatieId": "o-1", + "entityId": 41, + "active": true, + "name": "Entity new-entity" + } + ], + [ + "log.info", + "[FLOW] Sending organization registration email", + { + "organisatieId": "o-1" + } + ], + [ + "mail.sendOrganizationRegistrationEmail", + "Unknown" + ], + [ + "log.info", + "📧 Organization registration email sent successfully", + { + "organisatieId": "o-1" + } + ], + [ + "log.info", + "OrganizationSyncService: Updating organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "register": "reg-v", + "schema": "org-schema" + } + ], + [ + "entity.setObject", + "o-1", + { + "name": "Unknown", + "status": "actief", + "organisation": "new-entity", + "@self": { + "owner": "new-entity", + "organisation": "new-entity" + } + } + ], + [ + "entity.setOwner", + "o-1", + "new-entity" + ], + [ + "entity.setOrganisation", + "o-1", + "new-entity" + ], + [ + "objects.saveObject", + { + "object": { + "name": "Unknown", + "status": "actief", + "organisation": "new-entity", + "@self": { + "organisation": "new-entity" + } + }, + "register": "reg-entity", + "schema": "schema-entity", + "uuid": "o-1", + "silent": true, + "_validation": false + } + ], + [ + "log.info", + "OrganizationSyncService: Successfully updated organisatie object owner and organisation", + { + "organisatieId": "o-1", + "organisationEntityUuid": "new-entity", + "ownerSet": "new-entity", + "organisationSet": "new-entity" + } + ] + ] } }