From 5cdebf2d7e180b77ea8f939bff6eb2dfe9c9adc6 Mon Sep 17 00:00:00 2001 From: Ryan Wyllie Date: Thu, 29 Sep 2016 01:12:01 +0000 Subject: [PATCH] MDL-56292 message: simplify get_popup_notifications function --- .../src/notification_popover_controller.js | 3 - message/classes/api.php | 94 +++------- message/classes/output/popup_notification.php | 45 +---- message/externallib.php | 68 ++----- message/tests/api_test.php | 176 +----------------- message/tests/externallib_test.php | 52 ++---- 6 files changed, 61 insertions(+), 377 deletions(-) diff --git a/message/amd/src/notification_popover_controller.js b/message/amd/src/notification_popover_controller.js index d428723f0ba..8cda2df193e 100644 --- a/message/amd/src/notification_popover_controller.js +++ b/message/amd/src/notification_popover_controller.js @@ -285,9 +285,6 @@ define(['jquery', 'theme_bootstrapbase/bootstrap', 'core/ajax', 'core/templates' limit: this.limit, offset: this.getOffset(), useridto: this.userId, - markasread: false, - embeduserto: false, - embeduserfrom: false, }; var container = this.getContent(); diff --git a/message/classes/api.php b/message/classes/api.php index a713405bfe4..c7d1ced4514 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -539,15 +539,9 @@ class api { * @throws \moodle_exception * @since 3.2 */ - public static function get_popup_notifications($useridto = 0, $status = '', $embeduserto = false, $embeduserfrom = false, - $sort = 'DESC', $limit = 0, $offset = 0) { + public static function get_popup_notifications($useridto = 0, $sort = 'DESC', $limit = 0, $offset = 0) { global $DB, $USER; - if (!empty($status) && $status != MESSAGE_READ && $status != MESSAGE_UNREAD) { - throw new \moodle_exception(sprintf('invalid parameter: status: must be "%s" or "%s"', - MESSAGE_READ, MESSAGE_UNREAD)); - } - $sort = strtoupper($sort); if ($sort != 'DESC' && $sort != 'ASC') { throw new \moodle_exception('invalid parameter: sort: must be "DESC" or "ASC"'); @@ -557,68 +551,32 @@ class api { $useridto = $USER->id; } - $params = array(); + $params = [ + 'useridto1' => $useridto, + 'useridto2' => $useridto, + ]; - $buildtablesql = function($table, $prefix, $additionalfields, $messagestatus) - use ($status, $useridto, $embeduserto, $embeduserfrom) { - - $joinsql = ''; - $fields = "concat('$prefix', $prefix.id) as uniqueid, $prefix.id, $prefix.useridfrom, $prefix.useridto, - $prefix.subject, $prefix.fullmessage, $prefix.fullmessageformat, - $prefix.fullmessagehtml, $prefix.smallmessage, $prefix.notification, $prefix.contexturl, - $prefix.contexturlname, $prefix.timecreated, $prefix.timeuserfromdeleted, $prefix.timeusertodeleted, - $prefix.component, $prefix.eventtype, $additionalfields"; - $where = " AND $prefix.useridto = :{$prefix}useridto"; - $params = ["{$prefix}useridto" => $useridto]; - - if ($embeduserto) { - $embedprefix = "{$prefix}ut"; - $fields .= ", " . get_all_user_name_fields(true, $embedprefix, '', 'userto'); - $joinsql .= " LEFT JOIN {user} $embedprefix ON $embedprefix.id = $prefix.useridto"; - } - - if ($embeduserfrom) { - $embedprefix = "{$prefix}uf"; - $fields .= ", " . get_all_user_name_fields(true, $embedprefix, '', 'userfrom'); - $joinsql .= " LEFT JOIN {user} $embedprefix ON $embedprefix.id = $prefix.useridfrom"; - } - - if ($messagestatus == MESSAGE_READ) { - $isread = '1'; - } else { - $isread = '0'; - } - - return array( - sprintf( - "SELECT %s - FROM %s %s %s - WHERE %s.notification = 1 - AND %s.id IN (SELECT messageid FROM {message_popup} WHERE isread = %s) - %s", - $fields, $table, $prefix, $joinsql, $prefix, $prefix, $isread, $where - ), - $params - ); - }; - - switch ($status) { - case MESSAGE_READ: - list($sql, $readparams) = $buildtablesql('{message_read}', 'r', 'r.timeread', MESSAGE_READ); - $params = array_merge($params, $readparams); - break; - case MESSAGE_UNREAD: - list($sql, $unreadparams) = $buildtablesql('{message}', 'u', '0 as timeread', MESSAGE_UNREAD); - $params = array_merge($params, $unreadparams); - break; - default: - list($readsql, $readparams) = $buildtablesql('{message_read}', 'r', 'r.timeread', MESSAGE_READ); - list($unreadsql, $unreadparams) = $buildtablesql('{message}', 'u', '0 as timeread', MESSAGE_UNREAD); - $sql = sprintf("SELECT * FROM (%s UNION %s) f", $readsql, $unreadsql); - $params = array_merge($params, $readparams, $unreadparams); - } - - $sql .= " ORDER BY timecreated $sort, timeread $sort, id $sort"; + $sql = "SELECT * FROM ( + SELECT concat('r', r.id) as uniqueid, r.id, r.useridfrom, r.useridto, + r.subject, r.fullmessage, r.fullmessageformat, + r.fullmessagehtml, r.smallmessage, r.notification, r.contexturl, + r.contexturlname, r.timecreated, r.timeuserfromdeleted, r.timeusertodeleted, + r.component, r.eventtype, r.timeread + FROM {message_read} r + WHERE r.notification = 1 + AND r.id IN (SELECT messageid FROM {message_popup} WHERE isread = 1) + AND r.useridto = :useridto1 + UNION + SELECT concat('u', u.id) as uniqueid, u.id, u.useridfrom, u.useridto, + u.subject, u.fullmessage, u.fullmessageformat, + u.fullmessagehtml, u.smallmessage, u.notification, u.contexturl, + u.contexturlname, u.timecreated, u.timeuserfromdeleted, u.timeusertodeleted, + u.component, u.eventtype, 0 as timeread + FROM {message} u + WHERE u.notification = 1 + AND u.id IN (SELECT messageid FROM {message_popup} WHERE isread = 0) + AND u.useridto = :useridto2 + ) f ORDER BY timecreated $sort, timeread $sort, id $sort"; return array_values($DB->get_records_sql($sql, $params, $offset, $limit)); } diff --git a/message/classes/output/popup_notification.php b/message/classes/output/popup_notification.php index cc3a9d5a682..59df2e08ebb 100644 --- a/message/classes/output/popup_notification.php +++ b/message/classes/output/popup_notification.php @@ -47,21 +47,6 @@ class popup_notification implements templatable, renderable { */ protected $notification; - /** - * @var \stdClass Indicates if the receiver of the notification should have their details embedded in the output. - */ - protected $embeduserto; - - /** - * @var \stdClass Indicates if the sender of the notification should have their details embedded in the output. - */ - protected $embeduserfrom; - - /** - * @var string A cache for the receiver's full name, if it's already known, so that a DB lookup isn't required. - */ - protected $usertofullname; - /** * Constructor. * @@ -70,11 +55,8 @@ class popup_notification implements templatable, renderable { * @param \stdClass $embeduserfrom * @param string $usertofullname */ - public function __construct($notification, $embeduserto, $embeduserfrom, $usertofullname = '') { + public function __construct($notification) { $this->notification = $notification; - $this->embeduserto = $embeduserto; - $this->embeduserfrom = $embeduserfrom; - $this->usertofullname = $usertofullname; } public function export_for_template(\renderer_base $output) { @@ -88,31 +70,6 @@ class popup_notification implements templatable, renderable { $context->deleted = false; } - // We need to get the user from the query. - if ($this->embeduserfrom) { - // Check for non-reply and support users. - if (core_user::is_real_user($context->useridfrom)) { - $user = new \stdClass(); - $user = username_load_fields_from_object($user, $context, 'userfrom'); - $profileurl = new moodle_url('/user/profile.php', array('id' => $context->useridfrom)); - $context->userfromfullname = fullname($user); - $context->userfromprofileurl = $profileurl->out(); - } else { - $context->userfromfullname = get_string('coresystem'); - } - } - - // We need to get the user from the query. - if ($this->embeduserto) { - if (empty($this->usertofullname)) { - $user = new \stdClass(); - $user = username_load_fields_from_object($user, $context, 'userto'); - $context->usertofullname = fullname($user); - } else { - $context->usertofullname = $this->usertofullname; - } - } - $context->timecreatedpretty = get_string('ago', 'message', format_time(time() - $context->timecreated)); $context->text = message_format_message_text($context); $context->read = $context->timeread ? true : false; diff --git a/message/externallib.php b/message/externallib.php index 07731c4ff08..e45e348f6bc 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -1496,22 +1496,10 @@ class core_message_external extends external_api { public static function get_popup_notifications_parameters() { return new external_function_parameters( array( - 'useridto' => new external_value(PARAM_INT, 'the user id who received the message, 0 for any user', VALUE_REQUIRED), - 'status' => new external_value( - PARAM_ALPHA, 'filter the results to just "read" or "unread" notifications', - VALUE_DEFAULT, ''), - 'embeduserto' => new external_value( - PARAM_BOOL, 'true for returning user details for the recipient in each notification', - VALUE_DEFAULT, false), - 'embeduserfrom' => new external_value( - PARAM_BOOL, 'true for returning user details for the sender in each notification', - VALUE_DEFAULT, false), + 'useridto' => new external_value(PARAM_INT, 'the user id who received the message, 0 for current user', VALUE_REQUIRED), 'newestfirst' => new external_value( PARAM_BOOL, 'true for ordering by newest first, false for oldest first', VALUE_DEFAULT, true), - 'markasread' => new external_value( - PARAM_BOOL, 'mark notifications as read when they are returned by this function', - VALUE_DEFAULT, false), 'limit' => new external_value(PARAM_INT, 'the number of results to return', VALUE_DEFAULT, 0), 'offset' => new external_value(PARAM_INT, 'offset the result set by a given amount', VALUE_DEFAULT, 0) ) @@ -1525,28 +1513,19 @@ class core_message_external extends external_api { * @throws invalid_parameter_exception * @throws moodle_exception * @param int $useridto the user id who received the message - * @param string $status filter the results to only read or unread notifications - * @param bool $embeduserto true to embed the recipient user details in the record for each notification - * @param bool $embeduserfrom true to embed the send user details in the record for each notification * @param bool $newestfirst true for ordering by newest first, false for oldest first - * @param bool $markasread mark notifications as read when they are returned by this function * @param int $limit the number of results to return * @param int $offset offset the result set by a given amount * @return external_description */ - public static function get_popup_notifications($useridto, $status, $embeduserto, - $embeduserfrom, $newestfirst, $markasread, $limit, $offset) { + public static function get_popup_notifications($useridto, $newestfirst, $limit, $offset) { global $USER, $PAGE; $params = self::validate_parameters( self::get_popup_notifications_parameters(), array( 'useridto' => $useridto, - 'status' => $status, - 'embeduserto' => $embeduserto, - 'embeduserfrom' => $embeduserfrom, 'newestfirst' => $newestfirst, - 'markasread' => $markasread, 'limit' => $limit, 'offset' => $offset, ) @@ -1556,24 +1535,14 @@ class core_message_external extends external_api { self::validate_context($context); $useridto = $params['useridto']; - $status = $params['status']; - $embeduserto = $params['embeduserto']; - $embeduserfrom = $params['embeduserfrom']; $newestfirst = $params['newestfirst']; - $markasread = $params['markasread']; $limit = $params['limit']; $offset = $params['offset']; $issuperuser = has_capability('moodle/site:readallmessages', $context); $renderer = $PAGE->get_renderer('core_message'); - if (!empty($useridto)) { - if (core_user::is_real_user($useridto)) { - if ($embeduserto) { - $userto = core_user::get_user($useridto, '*', MUST_EXIST); - } - } else { - throw new moodle_exception('invaliduser'); - } + if (empty($useridto)) { + $useridto = $USER->id; } // Check if the current user is the sender/receiver or just a privileged user. @@ -1581,31 +1550,23 @@ class core_message_external extends external_api { throw new moodle_exception('accessdenied', 'admin'); } + if (!empty($useridto)) { + if (!core_user::is_real_user($useridto)) { + throw new moodle_exception('invaliduser'); + } + } + $sort = $newestfirst ? 'DESC' : 'ASC'; - $notifications = \core_message\api::get_popup_notifications($useridto, $status, $embeduserto, - $embeduserfrom, $sort, $limit, $offset); + $notifications = \core_message\api::get_popup_notifications($useridto, $sort, $limit, $offset); $notificationcontexts = []; if ($notifications) { - // In some cases, we don't need to get the to user objects from the sql query. - $usertofullname = ''; - - // In this case, the useridto field is not empty, so we can get the user destinatary fullname from there. - if (!empty($useridto) && $embeduserto) { - $usertofullname = fullname($userto); - } - foreach ($notifications as $notification) { - $notificationoutput = new \core_message\output\popup_notification($notification, $embeduserto, - $embeduserfrom, $usertofullname); + $notificationoutput = new \core_message\output\popup_notification($notification); $notificationcontext = $notificationoutput->export_for_template($renderer); $notificationcontexts[] = $notificationcontext; - - if ($markasread && !$notificationcontext->read) { - message_mark_message_read($notification, time()); - } } } @@ -1644,9 +1605,6 @@ class core_message_external extends external_api { 'timecreated' => new external_value(PARAM_INT, 'Time created'), 'timecreatedpretty' => new external_value(PARAM_TEXT, 'Time created in a pretty format'), 'timeread' => new external_value(PARAM_INT, 'Time read'), - 'usertofullname' => new external_value(PARAM_TEXT, 'User to full name', VALUE_OPTIONAL), - 'userfromfullname' => new external_value(PARAM_TEXT, 'User from full name', VALUE_OPTIONAL), - 'userfromprofileurl' => new external_value(PARAM_URL, 'User from profile url', VALUE_OPTIONAL), 'read' => new external_value(PARAM_BOOL, 'notification read status'), 'deleted' => new external_value(PARAM_BOOL, 'notification deletion status'), 'iconurl' => new external_value(PARAM_URL, 'URL for notification icon'), @@ -1656,7 +1614,7 @@ class core_message_external extends external_api { ), 'message' ) ), - 'unreadcount' => new external_value(PARAM_INT, 'the user whose blocked users we want to retrieve'), + 'unreadcount' => new external_value(PARAM_INT, 'the number of unread message for the given user'), ) ); } diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 5b7b0d8e192..616ac078149 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -95,56 +95,9 @@ class core_message_api_testcase extends core_message_messagelib_testcase { } /** - * Test that the get_popup_notifications function will return only read notifications if requested. + * Test that the get_popup_notifications function will return the correct notifications. */ - public function test_message_get_popup_notifications_read_only() { - $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); - $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); - - $this->send_fake_read_popup_notification($sender, $recipient, 'Message 1', 2); - $this->send_fake_read_popup_notification($sender, $recipient, 'Message 2', 4); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, MESSAGE_READ); - - $this->assertEquals($notifications[0]->fullmessage, 'Message 2'); - $this->assertEquals($notifications[1]->fullmessage, 'Message 1'); - - // Check if we request read and unread but there are only read messages, it should - // still return those correctly. - $notifications = \core_message\api::get_popup_notifications($recipient->id, ''); - - $this->assertEquals($notifications[0]->fullmessage, 'Message 2'); - $this->assertEquals($notifications[1]->fullmessage, 'Message 1'); - } - - /** - * Test that the get_popup_notifications function will return only unread notifications if requested. - */ - public function test_message_get_popup_notifications_unread_only() { - $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); - $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); - - $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 1', 2); - $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 2', 4); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, MESSAGE_UNREAD); - - $this->assertEquals($notifications[0]->fullmessage, 'Message 2'); - $this->assertEquals($notifications[1]->fullmessage, 'Message 1'); - - // Check if we request read and unread but there are only read messages, it should - // still return those correctly. - $notifications = \core_message\api::get_popup_notifications($recipient->id, ''); - - $this->assertEquals($notifications[0]->fullmessage, 'Message 2'); - $this->assertEquals($notifications[1]->fullmessage, 'Message 1'); - } - - /** - * Test that the get_popup_notifications function will return the correct notifications when both - * read and unread notifications are included. - */ - public function test_message_get_popup_notifications_mixed() { + public function test_message_get_popup_notifications() { $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); @@ -161,24 +114,13 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals($notifications[2]->fullmessage, 'Message 3'); $this->assertEquals($notifications[3]->fullmessage, 'Message 2'); $this->assertEquals($notifications[4]->fullmessage, 'Message 1'); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, MESSAGE_READ); - - $this->assertEquals($notifications[0]->fullmessage, 'Message 4'); - $this->assertEquals($notifications[1]->fullmessage, 'Message 3'); - $this->assertEquals($notifications[2]->fullmessage, 'Message 1'); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, MESSAGE_UNREAD); - - $this->assertEquals($notifications[0]->fullmessage, 'Message 5'); - $this->assertEquals($notifications[1]->fullmessage, 'Message 2'); } /** * Test that the get_popup_notifications function works correctly with limiting and offsetting * the result set if requested. */ - public function test_message_get_popup_notifications_all_with_limit_and_offset() { + public function test_message_get_popup_notifications_all_limit_and_offset() { $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); @@ -189,127 +131,23 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 5', 4); $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 6', 5); - $notifications = \core_message\api::get_popup_notifications($recipient->id, '', false, false, 'DESC', 2, 0); + $notifications = \core_message\api::get_popup_notifications($recipient->id, 'DESC', 2, 0); $this->assertEquals($notifications[0]->fullmessage, 'Message 6'); $this->assertEquals($notifications[1]->fullmessage, 'Message 5'); - $notifications = \core_message\api::get_popup_notifications($recipient->id, '', false, false, 'DESC', 2, 2); + $notifications = \core_message\api::get_popup_notifications($recipient->id, 'DESC', 2, 2); $this->assertEquals($notifications[0]->fullmessage, 'Message 4'); $this->assertEquals($notifications[1]->fullmessage, 'Message 3'); - $notifications = \core_message\api::get_popup_notifications($recipient->id, '', false, false, 'DESC', 0, 3); + $notifications = \core_message\api::get_popup_notifications($recipient->id, 'DESC', 0, 3); $this->assertEquals($notifications[0]->fullmessage, 'Message 3'); $this->assertEquals($notifications[1]->fullmessage, 'Message 2'); $this->assertEquals($notifications[2]->fullmessage, 'Message 1'); } - /** - * Test that the get_popup_notifications function returns embedded user details for the - * sender if requested. - */ - public function test_message_get_popup_notifications_embed_sender() { - $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); - $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); - - $this->send_fake_read_popup_notification($sender, $recipient, 'Message 1', 1); - $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 2', 2); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, '', false, true, 'DESC'); - - $func = function($type) { - return function($notification) use ($type) { - $user = new stdClass(); - $user = username_load_fields_from_object($user, $notification, $type); - return $user; - }; - }; - $senders = array_map($func('userfrom'), $notifications); - $recipients = array_map($func('userto'), $notifications); - - $this->assertEquals($senders[0]->firstname, 'Test1'); - $this->assertEquals($senders[0]->lastname, 'User1'); - $this->assertEquals($senders[1]->firstname, 'Test1'); - $this->assertEquals($senders[1]->lastname, 'User1'); - - // Make sure we didn't get recipient details when they weren't requested. - $this->assertEmpty($recipients[0]->firstname); - $this->assertEmpty($recipients[0]->lastname); - $this->assertEmpty($recipients[1]->firstname); - $this->assertEmpty($recipients[1]->lastname); - } - - /** - * Test that the get_popup_notifications function returns embedded user details for the - * recipient if requested. - */ - public function test_message_get_popup_notifications_embed_recipient() { - $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); - $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); - - $this->send_fake_read_popup_notification($sender, $recipient, 'Message 1', 1); - $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 2', 2); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, '', true, false, 'DESC'); - - $func = function($type) { - return function($notification) use ($type) { - $user = new stdClass(); - $user = username_load_fields_from_object($user, $notification, $type); - return $user; - }; - }; - $senders = array_map($func('userfrom'), $notifications); - $recipients = array_map($func('userto'), $notifications); - - $this->assertEquals($recipients[0]->firstname, 'Test2'); - $this->assertEquals($recipients[0]->lastname, 'User2'); - $this->assertEquals($recipients[1]->firstname, 'Test2'); - $this->assertEquals($recipients[1]->lastname, 'User2'); - - // Make sure we didn't get sender details when they weren't requested. - $this->assertEmpty($senders[0]->firstname); - $this->assertEmpty($senders[0]->lastname); - $this->assertEmpty($senders[1]->firstname); - $this->assertEmpty($senders[1]->lastname); - } - - /** - * Test that the get_popup_notifications function returns embedded all user details. - */ - public function test_message_get_popup_notifications_embed_both() { - $sender = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'User1')); - $recipient = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'User2')); - - $this->send_fake_read_popup_notification($sender, $recipient, 'Message 1', 1); - $this->send_fake_unread_popup_notification($sender, $recipient, 'Message 2', 2); - - $notifications = \core_message\api::get_popup_notifications($recipient->id, '', true, true, 'DESC'); - - $func = function($type) { - return function($notification) use ($type) { - $user = new stdClass(); - $user = username_load_fields_from_object($user, $notification, $type); - return $user; - }; - }; - $senders = array_map($func('userfrom'), $notifications); - $recipients = array_map($func('userto'), $notifications); - - $this->assertEquals($recipients[0]->firstname, 'Test2'); - $this->assertEquals($recipients[0]->lastname, 'User2'); - $this->assertEquals($recipients[1]->firstname, 'Test2'); - $this->assertEquals($recipients[1]->lastname, 'User2'); - - // Make sure we didn't get sender details when they weren't requested. - $this->assertEquals($senders[0]->firstname, 'Test1'); - $this->assertEquals($senders[0]->lastname, 'User1'); - $this->assertEquals($senders[1]->firstname, 'Test1'); - $this->assertEquals($senders[1]->lastname, 'User1'); - } - /** * Test count_unread_popup_notifications. */ @@ -353,4 +191,4 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(0, \core_message\api::count_blocked_users($user2)); $this->assertEquals(1, \core_message\api::count_blocked_users()); } -} \ No newline at end of file +} diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 8beba1799f9..cff71185208 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -900,7 +900,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->resetAfterTest(true); $this->setExpectedException('moodle_exception'); - $result = core_message_external::get_popup_notifications(-2132131, '', false, false, false, true, false, 0, 0); + $result = core_message_external::get_popup_notifications(-2132131, false, 0, 0); } public function test_get_popup_notifications_access_denied_exception() { @@ -911,7 +911,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->setUser($user); $this->setExpectedException('moodle_exception'); - $result = core_message_external::get_popup_notifications($sender->id, '', false, false, false, true, false, 0, 0); + $result = core_message_external::get_popup_notifications($sender->id, false, 0, 0); } public function test_get_popup_notifications_as_recipient() { @@ -929,35 +929,11 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { // Confirm that admin has super powers to retrieve any notifications. $this->setAdminUser(); - $result = core_message_external::get_popup_notifications($recipient->id, '', false, false, true, false, 0, 0); + $result = core_message_external::get_popup_notifications($recipient->id, false, 0, 0); $this->assertCount(4, $result['notifications']); $this->setUser($recipient); - $result = core_message_external::get_popup_notifications($recipient->id, '', false, false, true, false, 0, 0); - $this->assertCount(4, $result['notifications']); - - $result = core_message_external::get_popup_notifications($recipient->id, MESSAGE_UNREAD, false, true, true, false, 0, 0); - $this->assertCount(2, $result['notifications']); - $this->assertObjectHasAttribute('userfromfullname', $result['notifications'][0]); - $this->assertObjectNotHasAttribute('usertofullname', $result['notifications'][0]); - $this->assertObjectHasAttribute('userfromfullname', $result['notifications'][1]); - $this->assertObjectNotHasAttribute('usertofullname', $result['notifications'][1]); - - $result = core_message_external::get_popup_notifications($recipient->id, MESSAGE_UNREAD, true, true, true, false, 0, 0); - $this->assertCount(2, $result['notifications']); - $this->assertObjectHasAttribute('userfromfullname', $result['notifications'][0]); - $this->assertObjectHasAttribute('usertofullname', $result['notifications'][0]); - $this->assertObjectHasAttribute('userfromfullname', $result['notifications'][1]); - $this->assertObjectHasAttribute('usertofullname', $result['notifications'][1]); - - $result = core_message_external::get_popup_notifications($recipient->id, MESSAGE_UNREAD, true, true, true, true, 0, 0); - $this->assertCount(2, $result['notifications']); - $this->assertEquals(0, $result['unreadcount']); - - $result = core_message_external::get_popup_notifications($recipient->id, MESSAGE_UNREAD, true, true, true, true, 0, 0); - $this->assertCount(0, $result['notifications']); - - $result = core_message_external::get_popup_notifications($recipient->id, MESSAGE_READ, true, true, true, true, 0, 0); + $result = core_message_external::get_popup_notifications($recipient->id, false, 0, 0); $this->assertCount(4, $result['notifications']); } @@ -980,12 +956,12 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->send_fake_read_popup_notification($sender, $recipient, 'Notification 8', 8), ); - $result = core_message_external::get_popup_notifications($recipient->id, '', false, false, true, false, 2, 0); + $result = core_message_external::get_popup_notifications($recipient->id, true, 2, 0); $this->assertEquals($result['notifications'][0]->id, $notificationids[7]); $this->assertEquals($result['notifications'][1]->id, $notificationids[6]); - $result = core_message_external::get_popup_notifications($recipient->id, '', false, false, true, false, 2, 2); + $result = core_message_external::get_popup_notifications($recipient->id, true, 2, 2); $this->assertEquals($result['notifications'][0]->id, $notificationids[5]); $this->assertEquals($result['notifications'][1]->id, $notificationids[4]); @@ -1039,18 +1015,18 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { ); core_message_external::mark_all_notifications_as_read($recipient->id, $sender1->id); - $readresult = core_message_external::get_popup_notifications($recipient->id, 'read', false, false, true, false, 0, 0); - $unreadresult = core_message_external::get_popup_notifications($recipient->id, 'unread', false, false, true, false, 0, 0); + $result = core_message_external::get_popup_notifications($recipient->id, false, 0, 0); + $notifications = $result['notifications']; - $this->assertCount(2, $readresult['notifications']); - $this->assertCount(4, $unreadresult['notifications']); + $this->assertCount(2, array_filter($notifications, function($a) { return $a->read; })); + $this->assertCount(4, array_filter($notifications, function($a) { return !$a->read; })); core_message_external::mark_all_notifications_as_read($recipient->id, 0); - $readresult = core_message_external::get_popup_notifications($recipient->id, 'read', false, false, true, false, 0, 0); - $unreadresult = core_message_external::get_popup_notifications($recipient->id, 'unread', false, false, true, false, 0, 0); + $result = core_message_external::get_popup_notifications($recipient->id, false, 0, 0); + $notifications = $result['notifications']; - $this->assertCount(6, $readresult['notifications']); - $this->assertCount(0, $unreadresult['notifications']); + $this->assertCount(6, array_filter($notifications, function($a) { return $a->read; })); + $this->assertCount(0, array_filter($notifications, function($a) { return !$a->read; })); } public function test_get_unread_popup_notification_count_invalid_user_exception() {