Miscellaneous fixes for Group Nestings (CFM-41)

arlen · Oct 9, 2022 · 6a5f12f · 6a5f12f
1 parent 6d60cc9
commit 6a5f12f
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 6 deletions.
diff --git a/app/src/Model/Table/GroupMembersTable.php b/app/src/Model/Table/GroupMembersTable.php
@@ -386,7 +386,10 @@ public function syncNestedMembership(int  $personId,
                           ->all();
 
     // We convert $groupNestings to an array to avoid any confusion with the
-    // nested foreach() loops
+    // nested foreach() loops. Note that (unlike v4) we do not need to check for
+    // suspended Group status here since Groups cannot be suspended if they are
+    // nested (AR-Group-2), and cannot be nested if they are suspended (AR-GroupNesting-1).
+    // (This prevents admins from inadvertantly messing things up.)
     foreach($groupNestings->toArray() as $groupNesting) {
       $shouldBe = false;      // Should $person be a member of $targetGroup?
       $negated = false;       // $person is ineligible for $targetGroup due to any negative membership

diff --git a/app/src/Model/Table/GroupNestingsTable.php b/app/src/Model/Table/GroupNestingsTable.php
@@ -103,13 +103,13 @@ public function initialize(array $config): void {
    * specified source.
    *
    * @since  COmanage Registry v5.0.0
-   * @param  int   $id Group ID (of source Group)
-   * @return array     Array of available target Groups, as returned by find('list')
+   * @param  int   $groupId Group ID (of source Group)
+   * @return array          Array of available target Groups, as returned by find('list')
    */
 
   public function availableGroups(int $groupId): array {
     // Find the CO for $id. This will throw an exception if not found.
-    $sourceGroup = $this->Groups->get($id);
+    $sourceGroup = $this->Groups->get($groupId);
 
     // We don't remove groups that are already nested from the list -- we'll
     // catch those in rule validation.
@@ -120,7 +120,7 @@ public function availableGroups(int $groupId): array {
                           // AR-Group-Nesting-1 Only Active groups may be nested
                           'Groups.status'     => SuspendableStatusEnum::Active,
                           // AR-Group-Nesting-2 A group may not nest into itself
-                          'Groups.id IS NOT'  => $id,
+                          'Groups.id IS NOT'  => $groupId,
                           // AR-Group-Nesting-3 Automatic groups cannot be targets
                           'OR' => [
                             'Groups.group_type NOT IN' => [GroupTypeEnum::ActiveMembers, GroupTypeEnum::AllMembers],

diff --git a/app/src/Model/Table/GroupsTable.php b/app/src/Model/Table/GroupsTable.php
@@ -252,7 +252,7 @@ public function buildRules(RulesChecker $rules): RulesChecker {
 
     // AR-Group-2 A Group cannot be set to Suspended if it is nested into a
     // Target Group or is a Target Group for a nesting. This and AR-Group-3
-    // are to avoid unexpected consequences from implicitly undoing anesting...
+    // are to avoid unexpected consequences from implicitly undoing a nesting...
     // the administrator must do that first.
     $rules->addUpdate([$this, 'ruleIsNested'],
                        'isNestedUpdate',