Implement a (passing) test for sync adding an external group to a user

silinternational · Sep 3, 2024 · 9ce751c · 9ce751c
1 parent ad29b9e
commit 9ce751c
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 4 deletions.
diff --git a/application/common/models/User.php b/application/common/models/User.php
@@ -670,6 +670,28 @@ public static function listExternalGroups($appPrefix): array
         return $responseData;
     }
 
+    public static function listUsersWithExternalGroupWith($appPrefix): array
+    {
+        $appPrefixWithHyphen = $appPrefix . '-';
+
+        /** @var User[] $users */
+        $users = User::find()->where(
+            ['like', 'groups_external', $appPrefixWithHyphen]
+        )->all();
+
+        $emailAddresses = [];
+        foreach ($users as $user) {
+            $externalGroups = explode(',', $user->groups_external);
+            foreach ($externalGroups as $externalGroup) {
+                if (str_starts_with($externalGroup, $appPrefixWithHyphen)) {
+                    $emailAddresses[] = $user->email;
+                    break;
+                }
+            }
+        }
+        return $emailAddresses;
+    }
+
     public function loadMfaData(string $rpOrigin = '')
     {
         $verifiedMfaOptions = $this->getVerifiedMfaOptions($rpOrigin);
@@ -1032,6 +1054,37 @@ public function isPromptForMfa(): bool
         return false;
     }
 
+    public static function syncExternalGroups(string $appPrefix, array $desiredExternalGroupsByUserEmail): array
+    {
+        $errors = [];
+        $emailAddressesOfCurrentMatches = self::listUsersWithExternalGroupWith($appPrefix);
+
+        // Indicate that users not in the "desired" list should not have any
+        // such external groups.
+        foreach ($emailAddressesOfCurrentMatches as $email) {
+            if (! array_key_exists($email, $desiredExternalGroupsByUserEmail)) {
+                $desiredExternalGroupsByUserEmail[$email] = [];
+            }
+        }
+
+        foreach ($desiredExternalGroupsByUserEmail as $email => $groupsForPrefix) {
+            $user = User::findByEmail($email);
+            if ($user === null) {
+                $errors[] = 'No user found for ' . $email;
+                continue;
+            }
+            $successful = $user->updateExternalGroups($appPrefix, $groupsForPrefix);
+            if (! $successful) {
+                $errors[] = sprintf(
+                    'Failed to update external groups for %s: %s',
+                    $email,
+                    $user->getFirstErrors()
+                );
+            }
+        }
+        return $errors;
+    }
+
     public function updateExternalGroups(string $appPrefix, string $csvAppExternalGroups): bool
     {
         if (empty($csvAppExternalGroups)) {

diff --git a/application/features/bootstrap/GroupsExternalSyncContext.php b/application/features/bootstrap/GroupsExternalSyncContext.php
@@ -22,6 +22,9 @@ class GroupsExternalSyncContext extends GroupsExternalContext
      */
     private array $externalGroupsLists = [];
 
+    /** @var string[] */
+    private array $syncErrors;
+
     /**
      * @Given the following users exist, with these external groups:
      */
@@ -64,24 +67,40 @@ public function theExternalGroupsListIsTheFollowing(string $appPrefix, TableNode
      */
     public function iSyncTheListOfExternalGroups($appPrefix)
     {
-        User::syncExternalGroups(
+        $this->syncErrors = User::syncExternalGroups(
             $appPrefix,
             $this->externalGroupsLists[$appPrefix]
         );
     }
 
+    /**
+     * @Then there should not have been any sync errors
+     */
+    public function thereShouldNotHaveBeenAnySyncErrors()
+    {
+        Assert::isEmpty($this->syncErrors);
+    }
+
     /**
      * @Then the following users should have the following external groups:
      */
     public function theFollowingUsersShouldHaveTheFollowingExternalGroups(TableNode $table)
     {
         foreach ($table as $row) {
             $emailAddress = $row['email'];
-            $expectedExternalGroups = $row['groups'];
+            $expectedExternalGroups = explode(',', $row['groups']);
 
             $user = User::findByEmail($emailAddress);
             Assert::notNull($emailAddress, 'User not found: ' . $emailAddress);
-            Assert::same($user->groups_external, $expectedExternalGroups);
+            $actualExternalGroups = explode(',', $user->groups_external);
+
+            sort($actualExternalGroups);
+            sort($expectedExternalGroups);
+
+            Assert::same(
+                json_encode($actualExternalGroups, JSON_PRETTY_PRINT),
+                json_encode($expectedExternalGroups, JSON_PRETTY_PRINT)
+            );
         }
     }
 }
diff --git a/application/features/groups-external-sync.feature b/application/features/groups-external-sync.feature
@@ -8,6 +8,7 @@ Feature: Syncing a specific app-prefix of external groups with an external list
         | email                  | groups            |
         | john_smith@example.org | wiki-one,wiki-two |
     When I sync the list of "wiki" external groups
-    Then the following users should have the following external groups:
+    Then there should not have been any sync errors
+      And the following users should have the following external groups:
         | email                  | groups                        |
         | john_smith@example.org | wiki-one,map-america,wiki-two |