From 221e61bcd6659e5eead06a34150a5207a9d3421f Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Willms <tleilax+studip@gmail.com> Date: Thu, 9 Feb 2023 11:04:42 +0000 Subject: [PATCH] ensure user is actually a member of a course when sending notifications, fixes #2023 Closes #2023 Merge request studip/studip!1317 --- lib/classes/ModulesNotification.class.php | 51 +++++++++------ .../send_mail_notifications.class.php | 64 ++++++++++--------- lib/models/CourseMemberNotification.php | 2 + lib/models/User.class.php | 2 + 4 files changed, 69 insertions(+), 50 deletions(-) diff --git a/lib/classes/ModulesNotification.class.php b/lib/classes/ModulesNotification.class.php index 888052ab865..7a8b0ef269a 100644 --- a/lib/classes/ModulesNotification.class.php +++ b/lib/classes/ModulesNotification.class.php @@ -79,19 +79,24 @@ class ModulesNotification - function getAllNotifications ($user_id = NULL) + public function getAllNotifications ($user_id = null) { - - if (is_null($user_id)) { + if ($user_id === null) { $user_id = $GLOBALS['user']->id; } $my_sem = []; - $query = "SELECT s.Seminar_id, s.Name, s.chdate, s.start_time, IFNULL(visitdate, :threshold) AS visitdate " - . "FROM seminar_user_notifications su " - . "LEFT JOIN seminare s USING (Seminar_id) " - . "LEFT JOIN object_user_visits ouv ON (ouv.object_id = su.Seminar_id AND ouv.user_id = :user_id AND ouv.plugin_id = 0) " - . "WHERE su.user_id = :user_id"; + $query = "SELECT s.Seminar_id, s.Name, s.chdate, s.start_time, IFNULL(visitdate, :threshold) AS visitdate + FROM seminar_user_notifications su + JOIN seminar_user USING (user_id, seminar_id) + JOIN seminare s USING (Seminar_id) + LEFT JOIN object_user_visits ouv + ON ( + ouv.object_id = su.Seminar_id + AND ouv.user_id = :user_id + AND ouv.plugin_id = 0 + ) + WHERE su.user_id = :user_id"; $statement = DBManager::get()->prepare($query); $statement->bindValue(':user_id', $user_id); @@ -101,21 +106,23 @@ class ModulesNotification $seminar_id = $row['Seminar_id']; $tools = ToolActivation::findbyRange_id($seminar_id); $notification = CourseMemberNotification::find([$user_id, $seminar_id]); + + if (!$notification || count($notification->notification_data) === 0) { + continue; + } + $my_sem[$seminar_id] = [ - 'name' => $row['Name'], - 'chdate' => $row['chdate'], - 'start_time' => $row['start_time'], - 'tools' => new SimpleCollection($tools), - 'visitdate' => $row['visitdate'], - 'notification'=> $notification ? $notification->notification_data->getArrayCopy() : [] + 'name' => $row['Name'], + 'chdate' => $row['chdate'], + 'start_time' => $row['start_time'], + 'tools' => new SimpleCollection($tools), + 'visitdate' => $row['visitdate'], + 'notification' => $notification->notification_data->getArrayCopy(), ]; } $visit_data = get_objects_visits(array_keys($my_sem), 'sem', null, $user_id, array_keys($this->registered_notification_modules)); $news = []; foreach ($my_sem as $seminar_id => $s_data) { - if (!count($s_data['notification'])) { - continue; - } $navigation = MyRealmModel::getAdditionalNavigations($seminar_id, $s_data, null, $user_id, $visit_data[$seminar_id]); $n_data = []; foreach ($this->registered_notification_modules as $id => $m_data) { @@ -124,11 +131,11 @@ class ModulesNotification && $navigation[$id]->getImage() && $navigation[$id]->getImage()->getRole() === Icon::ROLE_ATTENTION ) { - $data = $this->getPluginText($navigation[$id], $seminar_id, $id); - if ($data) { - $n_data[] = $data; - } + $data = $this->getPluginText($navigation[$id], $seminar_id, $id); + if ($data) { + $n_data[] = $data; } + } } if (count($n_data)) { $news[$s_data['name']] = $n_data; @@ -149,6 +156,8 @@ class ModulesNotification $template_text->set_attribute('sso', $auth_plugin); return ['text' => $template_text->render(), 'html' => $template->render()]; } + + return null; } function getPluginText($nav, $seminar_id, $id) diff --git a/lib/cronjobs/send_mail_notifications.class.php b/lib/cronjobs/send_mail_notifications.class.php index b21d0febc47..9529d0bc8d9 100644 --- a/lib/cronjobs/send_mail_notifications.class.php +++ b/lib/cronjobs/send_mail_notifications.class.php @@ -94,40 +94,46 @@ class SendMailNotificationsJob extends CronJob */ public function execute($last_result, $parameters = []) { - global $user; + $notification = new ModulesNotification(); - $cli_user = $user; + $query = "SELECT DISTINCT user_id + FROM seminar_user_notifications + JOIN seminar_user USING (user_id, seminar_id)"; + DBManager::get()->fetchFirst( + $query, + [], + function ($user_id) use ($parameters, $notification) { + $user = User::find($user_id); + if ( + !$user + || $user->locked + || ($user->config->EXPIRATION_DATE > 0 && $user->config->EXPIRATION_DATE < time()) + ) { + return; + } - $notification = new ModulesNotification(); + $ok = false; + $mailmessage = $notification->getAllNotifications($user->id); - $query = "SELECT DISTINCT user_id FROM seminar_user_notifications"; + if ($mailmessage) { + setTempLanguage('', $user->preferred_language); - $rs = DBManager::get()->query($query); - while($r = $rs->fetch()){ - $user = new Seminar_User($r["user_id"]); - if ($user->locked || ($user->cfg->EXPIRATION_DATE > 0 && $user->cfg->EXPIRATION_DATE < time())) { - continue; - } - setTempLanguage('', $user->preferred_language); - $to = $user->email; - $title = "[" . Config::get()->UNI_NAME_CLEAN . "] " . _("Tägliche Benachrichtigung"); - $mailmessage = $notification->getAllNotifications($user->id); - $ok = false; - if ($mailmessage) { - if ($user->cfg->getValue('MAIL_AS_HTML')) { - $smail = new StudipMail(); - $ok = $smail->setSubject($title) - ->addRecipient($to) - ->setBodyHtml($mailmessage['html']) - ->setBodyText($mailmessage['text']) - ->send(); - } else { - $ok = StudipMail::sendMessage($to, $title, $mailmessage['text']); + $ok = StudipMail::sendMessage( + $user->email, + "[" . Config::get()->UNI_NAME_CLEAN . "] " . _('Tägliche Benachrichtigung'), + $mailmessage['text'], + $user->config->MAIL_AS_HTML ? $mailmessage['html'] : null + ); + } + + // Unset user configuration cache to preserve memory + UserConfig::set($user->id, null); + + // Log results + if ($ok !== false && $parameters['verbose']) { + echo $user->username . ':' . $ok . "\n"; } } - UserConfig::set($user->id, null); - if ($ok !== false && $parameters['verbose']) echo $user->username . ':' . $ok . "\n"; - } - $user = $cli_user; + ); } } diff --git a/lib/models/CourseMemberNotification.php b/lib/models/CourseMemberNotification.php index 8436f7d38d6..3a1f072f13f 100644 --- a/lib/models/CourseMemberNotification.php +++ b/lib/models/CourseMemberNotification.php @@ -20,6 +20,8 @@ * @property string chdate database column * @property User user belongs_to User * @property Course course belongs_to Course + * + * @property JSONArrayObject notification_data */ class CourseMemberNotification extends SimpleORMap implements PrivacyObject { diff --git a/lib/models/User.class.php b/lib/models/User.class.php index 9c6430f7c7b..14d3c9cd9c0 100644 --- a/lib/models/User.class.php +++ b/lib/models/User.class.php @@ -68,6 +68,8 @@ * @property UserInfo info has_one UserInfo * @property UserOnline online has_one UserOnline * @property Kategorie[]|SimpleORMapCollection $profile_categories has_many Kategorie + * + * @property UserConfig config */ class User extends AuthUserMd5 implements Range, PrivacyObject { -- GitLab