From 22d94c01d4bbfe7c0f9c3a507f30bd3b2b4bc995 Mon Sep 17 00:00:00 2001 From: Benn Oshrin Date: Tue, 17 Mar 2026 06:59:24 -0400 Subject: [PATCH] Fix handling of COUs and Groups during provisioning (CFM-26) --- .../src/Model/Table/SqlProvisionersTable.php | 11 +- app/config/schema/schema.json | 2 +- app/resources/locales/en_US/error.po | 5 +- app/src/Model/Table/CousTable.php | 130 ++++++++++++++++-- app/src/Model/Table/GroupsTable.php | 17 ++- 5 files changed, 149 insertions(+), 16 deletions(-) diff --git a/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php b/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php index 58d2c10b0..12fae5525 100644 --- a/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php +++ b/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php @@ -213,6 +213,15 @@ class SqlProvisionersTable extends Table { 'related' => [] ] /* XXX not yet implemented + +Note that all currently supported reference data (Cous, Types, and Groups, +which are treated like reference data) are also provisionable, meaning we +don't need to jump through special hoops to detect when they have changed. +If we add additional reference models that are not provisionable, then we +will need to implement something like v4's Reference Data Event Listener, +which would call syncReferenceData() on model.afterSave for the appropriate +reference data mode. + [ 'table' => 'co_terms_and_conditions', // Ordinarily we'd call this SpCoTermsAndConditions, but it's not worth @@ -477,7 +486,7 @@ protected function syncEntity( } // Pull the current target record -// XXX similar code in syncReferenceData, refactor? +// XXX similar code in syncReferenceData and status, refactor? $options = [ 'table' => $SqlProvisioner->table_prefix . $mconfig['table'], 'alias' => $mconfig['name'] . $SqlProvisioner->id, diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index 22ca32140..8b85e92ba 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -325,7 +325,7 @@ "groups_i2": { "columns": [ "co_id", "name" ] }, "groups_i3": { "columns": [ "co_id", "group_type" ] }, "groups_i4": { "columns": [ "cou_id", "group_type" ] }, - "groups_i5": { "needed": false, "columns": [ "cou_id" ]}, + "groups_i5": { "columns": [ "cou_id" ]}, "groups_i6": { "needed": false, "columns": [ "owners_group_id" ]} }, "clonable": true, diff --git a/app/resources/locales/en_US/error.po b/app/resources/locales/en_US/error.po index b78c8eae1..2547f18e0 100644 --- a/app/resources/locales/en_US/error.po +++ b/app/resources/locales/en_US/error.po @@ -128,7 +128,10 @@ msgid "Cos.active" msgstr "Requested CO {0} is not active" msgid "Cous.children" -msgstr "Cous has {0} child(ren) and cannot be deleted" +msgstr "Cou has {0} child(ren) and cannot be deleted" + +msgid "Cous.members" +msgstr "Cou has {0} member(s) and cannot be deleted" msgid "EmailAddresses.mail.delivery" msgstr "No verified Email Address is available for Person {0}" diff --git a/app/src/Model/Table/CousTable.php b/app/src/Model/Table/CousTable.php index 098a89f2f..3d9185e60 100644 --- a/app/src/Model/Table/CousTable.php +++ b/app/src/Model/Table/CousTable.php @@ -49,9 +49,12 @@ class CousTable extends Table { use \App\Lib\Traits\ChangelogBehaviorTrait; use \App\Lib\Traits\ClonableTrait; use \App\Lib\Traits\CoLinkTrait; + use \App\Lib\Traits\LabeledLogTrait; use \App\Lib\Traits\PermissionsTrait; use \App\Lib\Traits\PrimaryLinkTrait; - use \App\Lib\Traits\ProvisionableTrait; + use \App\Lib\Traits\ProvisionableTrait{ + requestProvisioning as traitRequestProvisioning; + } use \App\Lib\Traits\SearchFilterTrait; use \App\Lib\Traits\TableMetaTrait; use \App\Lib\Traits\TreeTrait; @@ -83,8 +86,8 @@ public function initialize(array $config): void { // _id suffix to match Cake's default pattern. ->setProperty('parent'); - // AR-COU-6 If a COU is deleted, the special groups associated with the COU will also be deleted. $this->hasMany('EnrollmentFlows'); + // AR-COU-6 If a COU is deleted, the special groups associated with the COU will also be deleted. $this->hasMany('Groups') ->setDependent(true) ->setCascadeCallbacks(true); @@ -149,6 +152,11 @@ public function initialize(array $config): void { */ public function buildRules(RulesChecker $rules): RulesChecker { + // AR-COU-1 A COU may not be deleted if it has any members. + $rules->addDelete([$this, 'ruleHasMembers'], + 'hasMembersDelete', + ['errorField' => 'status']); + // AR-COU-2 A COU may not be deleted if it has any children. $rules->addDelete([$this, 'ruleHasChildren'], 'hasChildrenDelete', @@ -226,12 +234,6 @@ public function localAfterSave(\Cake\Event\EventInterface $event, \Cake\Datasour } } - if($entity->isNew() && !empty($entity->id)) { - // Run setup for new COU - - $this->setup(id: $entity->id, coId: $entity->co_id); - } - return true; } @@ -291,7 +293,95 @@ public function postClone( } /** - * Application Rule to determine if the group has children. + * Request provisioning. + * + * @since COmanage Registry v5.2.0 + * @param int $id This table's entity ID to provision + * @param ProvisioningContextEnum $context Context in which provisioning is being requested + * @param int $provisioningTargetId If set, the Provisioning Target ID to request provisioning for (otherwise all) + * @param Job $job If called from a Job, the current Job entity + * @throws InvalidArgumentException + */ + + public function requestProvisioning( + int $id, + string $context, + ?int $provisioningTargetId=null, + ?Job $job=null, + ) { + // We need to handle the special COU Groups manually, depending on whether this is + // a delete operation or an add. This is going to result in some duplicate work, + // but that's the tradeoff to work within the existing set of callbacks. + + $couData = $this->marshalProvisioningData($id); + + if($couData['eligibility'] == ProvisioningEligibilityEnum::Deleted) { + // This is a delete operation. Per AR-COU-6, the special groups associated with the + // COU also need to be deleted. This has already happened via Cake's dependency + // deletion, ie + // + // (1) StandardController::delete() deletes the COU entity + // (2) Cake cascades that delete to the Groups with a matching cou_id + // (3) StandardController::delete() calls (de)provisioning on the COU, but nothing + // calls (de)provisioning on the Groups. + // + // Our workaround is to find the deleted groups and then request provisioning + // for them. Once that's done, we'll use the standard trait behavior to handle + // the COU deletion. + + // (This is really a general problem for deleting cascaded provisionable models, + // but it only manifests here currently, so we haven't implemented a general solution.) + + + $groups = $this->Groups->find('all', archived: true)->where(['cou_id' => $id])->all(); + + foreach($groups as $g) { + $this->llog('trace', "Forcing reprovisioning of deleted Group " . $g->id . " following deletion of COU " . $id); + + $this->Groups->requestProvisioning( + $g->id, + $context, + $provisioningTargetId, + $job + ); + } + + $this->traitRequestProvisioning($id, $context, $provisioningTargetId, $job); + } elseif($couData['eligibility'] == ProvisioningEligibilityEnum::Eligible) { + // We generally want the standard functionality. In addition, when a new COU is created, + // we also create default Groups, and GroupsTable::addDefault will attempt to provision + // then. This is fine for updates, but for new COUs the sequence of calls is + // + // (1) StandardController::add() saves new COU + // (2) CousTable::localAfterSave() calls setup + // (3) GroupsTable::addDefaults() creates the new Groups and tries to provision them, + // but the COU hasn't been provisoned yet, so this may or may not work (depending + // on the Provisioner) + // (4) StandardController::add() runs provisioning on the new COU + // + // Our workaround is to pull all COU related Groups and reprovision them here. + // We do this on both adds and updates because we don't haev the context anymore + // for whether $id is new. + + $this->traitRequestProvisioning($id, $context, $provisioningTargetId, $job); + + $groups = $this->Groups->find()->where(['cou_id' => $id])->all(); + + foreach($groups as $g) { + $this->llog('trace', "Forcing reprovisioning of Group " . $g->id . " following provisioning of COU " . $id); + + $this->Groups->requestProvisioning( + $g->id, + $context, + $provisioningTargetId, + $job + ); + } + } + } + + /** + * Application Rule to determine if the COU has children. * * @since COmanage Registry v5.2.0 * @param Entity $entity Entity to be validated @@ -310,6 +400,28 @@ public function ruleHasChildren($entity, $options) { return true; } + + /** + * Application Rule to determine if the COU has members. + * + * @since COmanage Registry v5.2.0 + * @param Entity $entity Entity to be validated + * @param array $options Application rule options + * @return boolean true if the Rule check passes, false otherwise + */ + + public function ruleHasMembers($entity, $options) { + $count = $this->PersonRoles + ->find('all') + ->where(['cou_id' => $entity->id]) + ->count(); + + if($count > 0) { + return __d('error', 'Cous.members', [$count]); + } + + return true; + } /** * Perform initial setup for a COU. diff --git a/app/src/Model/Table/GroupsTable.php b/app/src/Model/Table/GroupsTable.php index 3014f15c6..f5e0df886 100644 --- a/app/src/Model/Table/GroupsTable.php +++ b/app/src/Model/Table/GroupsTable.php @@ -40,6 +40,7 @@ use \App\Lib\Util\TableUtilities; use \App\Lib\Enum\ActionEnum; use \App\Lib\Enum\GroupTypeEnum; +use \App\Lib\Enum\ProvisioningContextEnum; use \App\Lib\Enum\ProvisioningEligibilityEnum; use \App\Lib\Enum\StatusEnum; use \App\Lib\Enum\SuspendableStatusEnum; @@ -376,11 +377,11 @@ public function addDefaults( if(!$grp) { // No existing group, create a new one - $entity = $this->newEntity($attrs); - $entity->co_id = $coId; - $entity->name = $gname; + $grp = $this->newEntity($attrs); + $grp->co_id = $coId; + $grp->name = $gname; - if(!$this->save($entity, options: ['autoOnly' => $autoOnly])) { + if(!$this->save($grp, options: ['autoOnly' => $autoOnly])) { throw new \RuntimeException(__d('error', 'save', ['GroupsTable::addDefaults'])); } } elseif($rename) { @@ -392,6 +393,14 @@ public function addDefaults( throw new \RuntimeException(__d('error', 'save', ['GroupsTable::addDefaults'])); } } + + if($couId || $rename) { + // If we're adding COU Group or renaming any Groups that call provisioning in case + // there are any Provisioning Targets that need to be updated. + + $this->llog('trace', "Requesting provisioning for default Group " . $grp->name); + $this->requestProvisioning(id: $grp->id, context: ProvisioningContextEnum::Automatic); + } } return true;