Away: Fix broken user group notification feature (#1145)

* Add getUserListByGroupIds() to user mapper and use that function in the away module. * Require ilchCore 2.2.7 for the away module. * Add unit tests for getUserListByGroupIds() and getUserListByGroupId().
IlchCMS · Dec 19, 2024 · 6e6efbc · 6e6efbc
1 parent e4ec70d
commit 6e6efbc
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 12 deletions.
diff --git a/application/modules/away/config/config.php b/application/modules/away/config/config.php
@@ -26,8 +26,8 @@ class Config extends \Ilch\Config\Install
                 'description' => 'User can enter when they are away (e.g. on holidays). There is an overview of this and the entries can be mananged in the admincenter.',
             ],
         ],
-        'ilchCore' => '2.2.0',
-        'phpVersion' => '7.3'
+        'ilchCore' => '2.2.7',
+        'phpVersion' => '7.4'
     ];
 
     public function install()

diff --git a/application/modules/away/controllers/Index.php b/application/modules/away/controllers/Index.php
@@ -91,9 +91,7 @@ public function indexAction()
 
                     $notifications = [];
                     $userGroups = $awayGroupMapper->getGroups();
-                    $users = $userMapper->getUserListByGroupId($userGroups, 1);
-                    // Users might be in several groups. Remove duplicates so each user only gets one notification.
-                    $users = array_unique($users, SORT_REGULAR);
+                    $users = $userMapper->getUserListByGroupIds($userGroups, 1);
                     $currentUserId = $this->getUser()->getId();
 
                     foreach ($users as $user) {
@@ -155,9 +153,7 @@ public function updateAction()
 
                 $notifications = [];
                 $userGroups = $awayGroupMapper->getGroups();
-                $users = $userMapper->getUserListByGroupId($userGroups, 1);
-                // Users might be in several groups. Remove duplicates so each user only gets one notification.
-                $users = array_unique($users, SORT_REGULAR);
+                $users = $userMapper->getUserListByGroupIds($userGroups, 1);
                 $currentUserId = $this->getUser()->getId();
 
                 foreach ($users as $user) {

diff --git a/application/modules/away/controllers/admin/Index.php b/application/modules/away/controllers/admin/Index.php
@@ -79,9 +79,7 @@ public function updateAction()
 
                 $notifications = [];
                 $userGroups = $awayGroupMapper->getGroups();
-                $users = $userMapper->getUserListByGroupId($userGroups, 1);
-                // Users might be in several groups. Remove duplicates so each user only gets one notification.
-                $users = array_unique($users, SORT_REGULAR);
+                $users = $userMapper->getUserListByGroupIds($userGroups, 1);
                 $currentUserId = $this->getUser()->getId();
 
                 foreach ($users as $user) {

diff --git a/application/modules/user/mappers/User.php b/application/modules/user/mappers/User.php
@@ -112,12 +112,28 @@ public function getUserBySelector(string $selector): ?UserModel
      * @since 2.1.20
      */
     public function getUserListByGroupId(int $groupId, int $confirmed = 0, $pagination = null): array
+    {
+        return $this->getUserListByGroupIds([$groupId], $confirmed, $pagination);
+    }
+
+    /**
+     * Get users (not all fields) by group ids. The users have to be in at least one of the groups.
+     *
+     * @param int[] $groupIds array of group ids.
+     * @param int $confirmed
+     * @param $pagination
+     * @return array
+     * @since 2.2.7
+     */
+    public function getUserListByGroupIds(array $groupIds, int $confirmed = 0, $pagination = null): array
     {
         $select = $this->db()->select()
             ->fields(['u.id', 'u.name', 'u.opt_mail', 'u.date_created', 'u.date_last_activity', 'u.confirmed'])
             ->from(['u' => 'users'])
             ->join(['g' => 'users_groups'], 'u.id = g.user_id', 'LEFT', ['group_id' => 'g.group_id'])
-            ->where(['group_id' => $groupId, 'confirmed' => $confirmed]);
+            ->where(['group_id' => $groupIds], 'or')
+            ->andWhere(['confirmed' => $confirmed])
+            ->group(['u.id']);
         if ($pagination !== null) {
             $select->limit($pagination->getLimit())
                 ->useFoundRows();

diff --git a/tests/modules/user/mappers/UserTest.php b/tests/modules/user/mappers/UserTest.php
@@ -42,6 +42,46 @@ public function testGetAdministratorCount()
         self::assertEquals(1, $this->out->getAdministratorCount());
     }
 
+    /**
+     * Test getUserListByGroupId to see if the correct user will be returned.
+     *
+     * @return void
+     */
+    public function testGetUserListByGroupId()
+    {
+        $userMapper = new UserMapper();
+
+        /** @var \Modules\User\Models\User[] $users */
+        $users = $userMapper->getUserListByGroupId(1, 1);
+        self::assertNotEmpty($users);
+        self::assertEquals(1, $users[0]->getId());
+        self::assertEquals('Testuser1', $users[0]->getName());
+        self::assertEquals(1, $users[0]->getConfirmed());
+    }
+
+    /**
+     * Test getUserListByGroupIds to see if the correct users will be returned.
+     *
+     * @return void
+     */
+    public function testGetUserListByGroupIds()
+    {
+        $userMapper = new UserMapper();
+
+        /** @var \Modules\User\Models\User[] $users */
+        $users = $userMapper->getUserListByGroupIds([1, 3], 1);
+        self::assertNotEmpty($users);
+        self::assertCount(2, $users);
+
+        self::assertEquals(1, $users[0]->getId());
+        self::assertEquals('Testuser1', $users[0]->getName());
+        self::assertEquals(1, $users[0]->getConfirmed());
+
+        self::assertEquals(2, $users[1]->getId());
+        self::assertEquals('Testuser2', $users[1]->getName());
+        self::assertEquals(1, $users[1]->getConfirmed());
+    }
+
     /**
      * Returns database schema sql statements to initialize database
      *