System Announcements: Use entities + fix behat test

public/main/admin/system_announcements.php

@@ -346,7 +346,7 @@ if ($action_todo) {
                     $values['content'],
                     $values['range_start'],
                     $values['range_end'],
-                    $values['roles'],
+                    $values['roles'] ?? [],
                     $values['lang'],
                     $sendMail,
                     empty($values['add_to_calendar']) ? false : true,
@@ -389,7 +389,7 @@ if ($action_todo) {
                     $values['content'],
                     $values['range_start'],
                     $values['range_end'],
-                    $values['roles'],
+                    $values['roles'] ?? [],
                     $values['lang'],
                     $sendMail,
                     $sendMailTest
@@ -418,7 +418,7 @@ if ($action_todo) {
                     }
                     Display::addFlash(
                         Display::return_message(
-                            get_lang('AnnouncementUpdate successful'),
+                            get_lang('Announcement has been updated'),
                             'confirmation'
                         )
                     );

public/main/inc/ajax/session.ajax.php

@@ -186,7 +186,7 @@ switch ($action) {
         ];
 
         $usersRepo = UserManager::getRepository();
-        $users = $usersRepo->findByStatus($_GET['q'], COURSEMANAGER, api_get_current_access_url_id());
+        $users = $usersRepo->findByRole('ROLE_TEACHER', $_GET['q'], api_get_current_access_url_id());
         /** @var User $user */
         foreach ($users as $user) {
             $list['items'][] = [

public/main/inc/lib/system_announcements.lib.php

@@ -340,7 +340,8 @@ class SystemAnnouncementManager
      * @param string $date_start      Start date (YYYY-MM-DD HH:II: SS)
      * @param string $date_end        End date (YYYY-MM-DD HH:II: SS)
      * @param array  $visibility
-     * @param string $lang            The language for which the announvement should be shown. Leave null for all langages
+     * @param string $lang            The language for which the announvement should be shown. Leave null for all
+     *                                langages
      * @param int    $send_mail       Whether to send an e-mail to all users (1) or not (0)
      * @param bool   $add_to_calendar
      * @param bool   $sendEmailTest
@@ -765,40 +766,25 @@ class SystemAnnouncementManager
             return true;
         }
 
-        $urlJoin = '';
+        $repo = Container::getUserRepository();
-        $urlCondition = '';
+        $qb = $repo->addRoleListQueryBuilder($announcement->getRoles());
-        $user_table = Database::get_main_table(TABLE_MAIN_USER);
+        $repo->addAccessUrlQueryBuilder(api_get_current_access_url_id(), $qb);
-        if (api_is_multiple_url_enabled()) {
-            $current_access_url_id = api_get_current_access_url_id();
-            $url_rel_user = Database::get_main_table(TABLE_MAIN_ACCESS_URL_REL_USER);
-            $urlJoin = " INNER JOIN $url_rel_user uu ON uu.user_id = u.id ";
-            $urlCondition = " AND access_url_id = '".$current_access_url_id."' ";
-        }
-
-        $sql = "SELECT DISTINCT u.id as user_id FROM $user_table u $urlJoin
-                WHERE status = '1' $urlCondition ";
-
-        $announcement;
-        $sql .= " AND roles IN () ";
-
-        if (!isset($sql)) {
-            return false;
-        }
 
         if (!empty($language)) {
-            //special condition because language was already treated for SQL insert before
+            $qb
-            $sql .= " AND language = '".Database::escape_string($language)."' ";
+                ->andWhere('u.locale = :lang')
+                ->setParameter('lang', $language)
+            ;
         }
 
+        $repo->addActiveAndNotAnonUserQueryBuilder($qb);
+        $repo->addExpirationDateQueryBuilder($qb);
+
         // Sent to active users.
-        $sql .= " AND email <>'' AND active = 1 ";
+        //$sql .= " AND email <>'' AND active = 1 ";
 
         // Expiration date
-        $sql .= " AND (expiration_date = '' OR expiration_date IS NULL OR expiration_date > '$now') ";
+        //$sql .= " AND (expiration_date = '' OR expiration_date IS NULL OR expiration_date > '$now') ";
-
-        if ((empty($teacher) || '0' == $teacher) && (empty($student) || '0' == $student)) {
-            return true;
-        }
 
         $userListToFilter = [];
         // @todo check if other filters will apply for the career/promotion option.
@@ -814,14 +800,14 @@ class SystemAnnouncementManager
                 foreach ($promotionList as $promotion) {
                     $sessionList = SessionManager::get_all_sessions_by_promotion($promotion['id']);
                     foreach ($sessionList as $session) {
-                        if ($teacher) {
+                        if (in_array('ROLE_TEACHER', $announcement->getRoles(), true)) {
                             $users = SessionManager::get_users_by_session($session['id'], 2);
                             if (!empty($users)) {
                                 $userListToFilter = array_merge($users, $userListToFilter);
                             }
                         }
 
-                        if ($student) {
+                        if (in_array('ROLE_STUDENT', $announcement->getRoles(), true)) {
                             $users = SessionManager::get_users_by_session($session['id'], 0);
                             if (!empty($users)) {
                                 $userListToFilter = array_merge($users, $userListToFilter);
@@ -834,28 +820,29 @@ class SystemAnnouncementManager
 
         if (!empty($userListToFilter)) {
             $userListToFilter = array_column($userListToFilter, 'user_id');
-            $userListToFilterToString = implode("', '", $userListToFilter);
+            //$userListToFilterToString = implode("', '", $userListToFilter);
-            $sql .= " AND (u.user_id IN ('$userListToFilterToString') ) ";
+            $qb
-        }
+                ->andWhere('u.id IN (:users)')
-
+                ->setParameter('users', $userListToFilter)
-        $result = Database::query($sql);
+            ;
-        if (false === $result) {
+            //$sql .= " AND (u.user_id IN ('$userListToFilterToString') ) ";
-            return false;
         }
+        $users = $qb->getQuery()->getResult();
 
         $message_sent = false;
-        while ($row = Database::fetch_array($result, 'ASSOC')) {
+        /** @var \Chamilo\CoreBundle\Entity\User $user */
-            MessageManager::send_message_simple($row['user_id'], $title, $content);
+        foreach ($users as $user) {
+            MessageManager::send_message_simple($user->getId(), $title, $content);
             $message_sent = true;
         }
 
         // Minor validation to clean up the attachment files in the announcement
-        if (!empty($_FILES)) {
+        /*if (!empty($_FILES)) {
             $attachments = $_FILES;
             foreach ($attachments as $attachment) {
                 unlink($attachment['tmp_name']);
             }
-        }
+        }*/
 
         return $message_sent; //true if at least one e-mail was sent
     }

src/CoreBundle/Repository/Node/UserRepository.php

@@ -210,6 +210,19 @@ class UserRepository extends ResourceRepository implements PasswordUpgraderInter
         return $resourceNode;
     }
 
+    public function addRoleListQueryBuilder(array $roleList, QueryBuilder $qb = null): QueryBuilder
+    {
+        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
+        if (!empty($roleList)) {
+            $qb
+                ->andWhere('u.roles IN (:roles)')
+                ->setParameter('roles', $roleList, Types::ARRAY)
+            ;
+        }
+
+        return $qb;
+    }
+
     public function findByUsername(string $username): ?User
     {
         $user = $this->findOneBy([
@@ -224,17 +237,20 @@ class UserRepository extends ResourceRepository implements PasswordUpgraderInter
     }
 
     /**
+     * Get a filtered list of user by role and (optionally) access url.
+     *
+     * @param string $keyword     The query to filter
+     * @param int    $accessUrlId The access URL ID
+     *
      * @return User[]
      */
-    public function findByRole(string $role)
+    public function findByRole(string $role, string $keyword, int $accessUrlId = 0)
     {
         $qb = $this->createQueryBuilder('u');
 
-        $qb->select('u')
+        $this->addAccessUrlQueryBuilder($accessUrlId, $qb);
-            ->from($this->_entityName, 'u')
+        $this->addRoleQueryBuilder($role, $qb);
-            ->where('u.roles LIKE :roles')
+        $this->addSearchByKeywordQueryBuilder($keyword, $qb);
-            ->setParameter('roles', '%"'.$role.'"%', Types::STRING)
-        ;
 
         return $qb->getQuery()->getResult();
     }
@@ -312,26 +328,6 @@ class UserRepository extends ResourceRepository implements PasswordUpgraderInter
         return $query->execute();
     }*/
 
-    /**
-     * Get a filtered list of user by status and (optionally) access url.
-     *
-     * @param string $keyword     The query to filter
-     * @param int    $status      The status
-     * @param int    $accessUrlId The access URL ID
-     *
-     * @return User[]
-     */
-    public function findByStatus(string $keyword, int $status, int $accessUrlId = 0)
-    {
-        $qb = $this->createQueryBuilder('u');
-
-        $this->addAccessUrlQueryBuilder($accessUrlId, $qb);
-        $this->addStatusQueryBuilder($status, $qb);
-        $this->addSearchByKeywordQueryBuilder($keyword, $qb);
-
-        return $qb->getQuery()->getResult();
-    }
-
     /**
      * Get the coaches for a course within a session.
      *
@@ -1436,6 +1432,52 @@ class UserRepository extends ResourceRepository implements PasswordUpgraderInter
         ;
     }
 
+    public function addAccessUrlQueryBuilder(int $accessUrlId, QueryBuilder $qb = null): QueryBuilder
+    {
+        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
+        $qb
+            ->innerJoin('u.portals', 'p')
+            ->andWhere('p.url = :url')
+            ->setParameter('url', $accessUrlId, Types::INTEGER)
+        ;
+
+        return $qb;
+    }
+
+    public function addActiveAndNotAnonUserQueryBuilder(QueryBuilder $qb = null): QueryBuilder
+    {
+        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
+        $qb
+            ->andWhere('u.active = 1')
+            ->andWhere('u.status <> :status')
+            ->setParameter('status', User::ANONYMOUS, Types::INTEGER)
+        ;
+
+        return $qb;
+    }
+
+    public function addExpirationDateQueryBuilder(QueryBuilder $qb = null): QueryBuilder
+    {
+        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
+        $qb
+            ->andWhere('u.registrationDate IS NULL OR u.registrationDate > :now')
+            ->setParameter('now', new Datetime(), Types::DATETIME_MUTABLE)
+        ;
+
+        return $qb;
+    }
+
+    private function addRoleQueryBuilder(string $role, QueryBuilder $qb = null): QueryBuilder
+    {
+        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
+        $qb
+            ->andWhere('u.roles LIKE :roles')
+            ->setParameter('roles', '%"'.$role.'"%', Types::STRING)
+        ;
+
+        return $qb;
+    }
+
     private function addSearchByKeywordQueryBuilder($keyword, QueryBuilder $qb = null): QueryBuilder
     {
         $qb = $this->getOrCreateQueryBuilder($qb, 'u');
@@ -1482,18 +1524,6 @@ class UserRepository extends ResourceRepository implements PasswordUpgraderInter
         return $qb;
     }
 
-    private function addAccessUrlQueryBuilder(int $accessUrlId, QueryBuilder $qb = null): QueryBuilder
-    {
-        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
-        $qb
-            ->innerJoin('u.portals', 'p')
-            ->andWhere('p.url = :url')
-            ->setParameter('url', $accessUrlId, Types::INTEGER)
-        ;
-
-        return $qb;
-    }
-
     private function addNotCurrentUserQueryBuilder(int $userId, QueryBuilder $qb = null): QueryBuilder
     {
         $qb = $this->getOrCreateQueryBuilder($qb, 'u');
@@ -1504,27 +1534,4 @@ class UserRepository extends ResourceRepository implements PasswordUpgraderInter
 
         return $qb;
     }
-
-    private function addActiveAndNotAnonUserQueryBuilder(QueryBuilder $qb = null): QueryBuilder
-    {
-        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
-        $qb
-            ->andWhere('u.active = 1')
-            ->andWhere('u.status <> :status')
-            ->setParameter('status', User::ANONYMOUS, Types::INTEGER)
-        ;
-
-        return $qb;
-    }
-
-    private function addStatusQueryBuilder(int $status, QueryBuilder $qb = null): QueryBuilder
-    {
-        $qb = $this->getOrCreateQueryBuilder($qb, 'u');
-        $qb
-            ->andWhere('u.status = :status')
-            ->setParameter('status', $status, Types::INTEGER)
-        ;
-
-        return $qb;
-    }
 }