From ab271d0d0ddd90c80f1529bb27c6ce7893e1bf08 Mon Sep 17 00:00:00 2001 From: Julio Montoya Date: Thu, 17 Jun 2021 16:05:17 +0200 Subject: [PATCH] System Announcements: Use entities + fix behat test --- public/main/admin/system_announcements.php | 6 +- public/main/inc/ajax/session.ajax.php | 2 +- .../main/inc/lib/system_announcements.lib.php | 69 ++++------ .../Repository/Node/UserRepository.php | 129 +++++++++--------- 4 files changed, 100 insertions(+), 106 deletions(-) diff --git a/public/main/admin/system_announcements.php b/public/main/admin/system_announcements.php index 2c98a06f1a..98429311e4 100644 --- a/public/main/admin/system_announcements.php +++ b/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' ) ); diff --git a/public/main/inc/ajax/session.ajax.php b/public/main/inc/ajax/session.ajax.php index 6e08c26014..5ce3fa2345 100644 --- a/public/main/inc/ajax/session.ajax.php +++ b/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'][] = [ diff --git a/public/main/inc/lib/system_announcements.lib.php b/public/main/inc/lib/system_announcements.lib.php index 662f375199..db2b001a3f 100644 --- a/public/main/inc/lib/system_announcements.lib.php +++ b/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 = ''; - $urlCondition = ''; - $user_table = Database::get_main_table(TABLE_MAIN_USER); - 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; - } + $repo = Container::getUserRepository(); + $qb = $repo->addRoleListQueryBuilder($announcement->getRoles()); + $repo->addAccessUrlQueryBuilder(api_get_current_access_url_id(), $qb); if (!empty($language)) { - //special condition because language was already treated for SQL insert before - $sql .= " AND language = '".Database::escape_string($language)."' "; + $qb + ->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') "; - - if ((empty($teacher) || '0' == $teacher) && (empty($student) || '0' == $student)) { - return true; - } + //$sql .= " AND (expiration_date = '' OR expiration_date IS NULL OR expiration_date > '$now') "; $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); - $sql .= " AND (u.user_id IN ('$userListToFilterToString') ) "; - } - - $result = Database::query($sql); - if (false === $result) { - return false; + //$userListToFilterToString = implode("', '", $userListToFilter); + $qb + ->andWhere('u.id IN (:users)') + ->setParameter('users', $userListToFilter) + ; + //$sql .= " AND (u.user_id IN ('$userListToFilterToString') ) "; } + $users = $qb->getQuery()->getResult(); $message_sent = false; - while ($row = Database::fetch_array($result, 'ASSOC')) { - MessageManager::send_message_simple($row['user_id'], $title, $content); + /** @var \Chamilo\CoreBundle\Entity\User $user */ + 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 } diff --git a/src/CoreBundle/Repository/Node/UserRepository.php b/src/CoreBundle/Repository/Node/UserRepository.php index af8a87a351..7aff488897 100644 --- a/src/CoreBundle/Repository/Node/UserRepository.php +++ b/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') - ->from($this->_entityName, 'u') - ->where('u.roles LIKE :roles') - ->setParameter('roles', '%"'.$role.'"%', Types::STRING) - ; + $this->addAccessUrlQueryBuilder($accessUrlId, $qb); + $this->addRoleQueryBuilder($role, $qb); + $this->addSearchByKeywordQueryBuilder($keyword, $qb); 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; - } }