diff --git a/app/src/Command/CloneCommand.php b/app/src/Command/CloneCommand.php index 8b3ca44de..03afbf1ce 100644 --- a/app/src/Command/CloneCommand.php +++ b/app/src/Command/CloneCommand.php @@ -791,7 +791,8 @@ protected function cloneEntityRelations( ->first(); if(!empty($srcent)) { - $this->cloneUpsert( + // In the event of an insert we'll need the record ID that was created + $targetEntId = $this->cloneUpsert( $SourceRelatedTable, $srcent, $TargetRelatedTable, @@ -806,7 +807,7 @@ protected function cloneEntityRelations( // Recurse on any subrelations, but only on upserts. // (Cake's dependency handling should deal with deletes.) - $this->cloneEntityRelations($srcent, $targetent->id, $related[$rt], $targetDataSource, $targetCoId); + $this->cloneEntityRelations($srcent, $targetEntId, $related[$rt], $targetDataSource, $targetCoId); } } elseif(!empty($targetent)) { // If there is no $srcent but there is a $targetent, delete $targetent. @@ -841,7 +842,8 @@ protected function cloneEntityRelations( // There should be at most one ->first(); - $foundEntities[] = $this->cloneUpsert( + // In the event of an insert we'll need the record ID that was created + $targetEntId = $this->cloneUpsert( $SourceRelatedTable, $srcent, $TargetRelatedTable, @@ -853,6 +855,8 @@ protected function cloneEntityRelations( true ); + $foundEntities[] = $targetEntId; + if(!empty($related[$rt])) { // Recurse on any subrelations. We have to recurse on _each_ source entity. // We only recurse on inserts and updates. We assume that on a delete @@ -860,7 +864,7 @@ protected function cloneEntityRelations( // If $original was Pipeline and $srcent was Flange, we're now calling ourselves // with Flange and its relations (its Plugin instantiations, eg PipelineToolkit.PersonRoleMappers) - $this->cloneEntityRelations($srcent, $targetent->id, $related[$rt], $targetDataSource, $targetCoId); + $this->cloneEntityRelations($srcent, $targetEntId, $related[$rt], $targetDataSource, $targetCoId); } } @@ -869,13 +873,13 @@ protected function cloneEntityRelations( $this->io->out("Reviewing " . $targetIterator->count() . " records in target for deletions"); - foreach($targetIterator as $targetent) { - if(!in_array($targetent->id, $foundEntities)) { + foreach($targetIterator as $te) { + if(!in_array($te->id, $foundEntities)) { // We didn't see this target entry in the source data, so remove it - $this->io->out("Deleting target record " . $targetent->id); + $this->io->out("Deleting target record " . $te->id); - $TargetRelatedTable->delete($targetent); + $TargetRelatedTable->delete($te); } } } diff --git a/app/src/Lib/Util/SearchUtilities.php b/app/src/Lib/Util/SearchUtilities.php index 155f3772a..346d88a89 100644 --- a/app/src/Lib/Util/SearchUtilities.php +++ b/app/src/Lib/Util/SearchUtilities.php @@ -36,16 +36,16 @@ class SearchUtilities { // To add a new clonable model, see https://spaces.at.internet2.edu/x/DIBuFQ // Because this list is used by CloneCommand, it should be sorted in dependency order. static protected $clonableModels = [ - 'ApiUsers', - 'Apis', + 'Servers', + 'Types', 'Cous', 'Groups', + 'ApiUsers', 'IdentifierAssignments', 'Pipelines', 'ExternalIdentitySources', 'ProvisioningTargets', - 'Servers', - 'Types' + 'Apis' ]; // To add a new backend to search: diff --git a/app/src/Model/Entity/Group.php b/app/src/Model/Entity/Group.php index 8c7a30bf6..3420a06c0 100644 --- a/app/src/Model/Entity/Group.php +++ b/app/src/Model/Entity/Group.php @@ -75,7 +75,7 @@ public function isAllMembers(): bool { */ public function isAutomatic(): bool { - // If this list is updated, CousTable::getCloneSuccessors will need to be updated as well + // If this list is updated, GroupsTable::findNonAutomaticGroups will need to be updated as well return in_array($this->group_type, [GroupTypeEnum::ActiveMembers, GroupTypeEnum::AllMembers]); } diff --git a/app/src/Model/Table/CousTable.php b/app/src/Model/Table/CousTable.php index 475e1f32c..589b459f7 100644 --- a/app/src/Model/Table/CousTable.php +++ b/app/src/Model/Table/CousTable.php @@ -182,16 +182,10 @@ public function getCloneSuccessors( ): array { // Pull the set of non-automatic COU Groups and return their UUIDs - $clonableGroups = $this->Groups->find() - ->where([ - 'cou_id' => $original->id, - 'group_type NOT IN' => [ - GroupTypeEnum::ActiveMembers, - GroupTypeEnum::AllMembers - ] - ]) - ->all(); - + $clonableGroups = $this->Groups + ->find('nonAutomaticGroups', ['cou_id' => $original->id]) + ->all(); + return $clonableGroups->extract('uuid')->toArray(); } diff --git a/app/src/Model/Table/GroupsTable.php b/app/src/Model/Table/GroupsTable.php index 05f5da4e4..14dc14260 100644 --- a/app/src/Model/Table/GroupsTable.php +++ b/app/src/Model/Table/GroupsTable.php @@ -643,6 +643,38 @@ public function findMfaExemptGroup(Query $query, array $options): Query { ]); } + /** + * Find non-Automatic Groups. + * + * @since COmanage Registry v5.2.0 + * @param \Cake\ORM\Query $query Query + * @param array $options Options: co_id, cou_id + * @return \Cake\ORM\Query Query + */ + + public function findNonAutomaticGroups(Query $query, array $options): Query { + $whereClause = [ + 'status' => SuspendableStatusEnum::Active, + 'group_type NOT IN' => [GroupTypeEnum::ActiveMembers, GroupTypeEnum::AllMembers] + ]; + + // We accept _either_ co_id or cou_id (but not both, since cou_id implies co_id). + // We also specifically allow co_id and a null cou_id (all groups in a CO without + // a cou_id, ie: the CO level non-automatic Groups). + + if(isset($options['co_id'])) { + $whereClause['co_id'] = $options['co_id']; + + if(isset($options['cou_id']) && is_null($options['cou_id'])) { + $whereClause['cou_id IS'] = null; + } + } elseif(isset($options['cou_id'])) { + $whereClause['cou_id'] = $options['cou_id']; + } + + return $query->where($whereClause); + } + /** * Get the Admin Group for a CO. * @@ -845,6 +877,11 @@ public function implementedEvents(): array { */ public function localAfterSave(EventInterface $event, EntityInterface $entity, \ArrayObject $options): bool { + // If we're cloning we don't want to record history or manage Owners Groups + if(isset($options['clone']) && $options['clone']) { + return true; + } + // We don't record history if autoOnly is set because we're in the middle of cloning // and aside from the datasources not lining up, it's not clear it makes sense to record // the history in that context