MDL-36941 core_message: mark notification/messages takes stdClass

This avoid unnecessary DB calls.
This commit is contained in:
Mark Nelson 2018-02-26 19:10:16 +08:00
parent 883ce42127
commit 548936a6c9
10 changed files with 68 additions and 47 deletions

View File

@ -91,9 +91,10 @@ class manager {
// If they have deselected all processors and its a notification mark it read. The user doesn't want to be bothered.
// The same goes if the messaging is completely disabled.
if ($eventdata->notification) {
\core_message\api::mark_notification_as_read($eventdata->userto->id, $savemessage->id);
$savemessage->timeread = null;
\core_message\api::mark_notification_as_read($savemessage);
} else {
\core_message\api::mark_message_as_read($eventdata->userto->id, $savemessage->id);
\core_message\api::mark_message_as_read($eventdata->userto->id, $savemessage);
}
}
@ -158,15 +159,16 @@ class manager {
// or any processor that puts a row in message_working then the notification will remain forever
// unread. To prevent this mark the message read if messaging is disabled.
if (empty($CFG->messaging) && $eventdata->notification) {
\core_message\api::mark_notification_as_read($eventdata->userto->id, $savemessage->id);
\core_message\api::mark_notification_as_read($savemessage);
}
// If there is no more processors that want to process this we can mark the message as read.
if ($DB->count_records('message_working', array('unreadmessageid' => $savemessage->id)) == 0) {
if ($eventdata->notification) {
\core_message\api::mark_notification_as_read($eventdata->userto->id, $savemessage->id);
$savemessage->timeread = null;
\core_message\api::mark_notification_as_read($savemessage);
} else {
\core_message\api::mark_message_as_read($eventdata->userto->id, $savemessage->id);
\core_message\api::mark_message_as_read($eventdata->userto->id, $savemessage);
}
}

View File

@ -6719,9 +6719,9 @@ function message_mark_message_read($message, $timeread, $messageworkingempty=fal
global $DB;
if (!empty($message->notification)) {
\core_message\api::mark_notification_as_read($message->useridto, $message->id, $timeread);
\core_message\api::mark_notification_as_read($message, $timeread);
} else {
\core_message\api::mark_message_as_read($message->useridto, $message->id, $timeread);
\core_message\api::mark_message_as_read($message->useridto, $message, $timeread);
}
// If any processors have pending actions abort them.

View File

@ -201,9 +201,9 @@ function message_send($eventdata) {
// Mark the message/notification as read.
if ($eventdata->notification) {
\core_message\api::mark_notification_as_read($eventdata->userto->id, $message->id);
\core_message\api::mark_notification_as_read($message);
} else {
\core_message\api::mark_message_as_read($eventdata->userto->id, $message->id);
\core_message\api::mark_message_as_read($eventdata->userto->id, $message);
}
// Unit tests need this detail.

View File

@ -758,11 +758,11 @@ class api {
}
foreach ($messages as $message) {
self::mark_message_as_read($touserid, $message->id);
self::mark_message_as_read($touserid, $message);
}
foreach ($notifications as $notification) {
self::mark_notification_as_read($touserid, $notification->id);
self::mark_notification_as_read($notification);
}
}
@ -1045,24 +1045,22 @@ class api {
* Mark a single message as read.
*
* @param int $userid The user id who marked the message as read
* @param int $messageid The message id
* @param \stdClass $message The message
* @param int|null $timeread The time the message was marked as read, if null will default to time()
*/
public static function mark_message_as_read($userid, $messageid, $timeread = null) {
public static function mark_message_as_read($userid, $message, $timeread = null) {
global $DB;
if (is_null($timeread)) {
$timeread = time();
}
$message = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
// Check if the user has already read this message.
if (!$DB->record_exists('message_user_actions', ['userid' => $userid,
'messageid' => $messageid, 'action' => self::MESSAGE_ACTION_READ])) {
'messageid' => $message->id, 'action' => self::MESSAGE_ACTION_READ])) {
$mua = new \stdClass();
$mua->userid = $userid;
$mua->messageid = $messageid;
$mua->messageid = $message->id;
$mua->action = self::MESSAGE_ACTION_READ;
$mua->timecreated = $timeread;
$mua->id = $DB->insert_record('message_user_actions', $mua);
@ -1081,7 +1079,7 @@ class api {
'context' => $context,
'relateduserid' => $message->useridfrom,
'other' => array(
'messageid' => $messageid
'messageid' => $message->id
)
));
$event->trigger();
@ -1091,20 +1089,16 @@ class api {
/**
* Mark a single notification as read.
*
* @param int $userid The user id who marked the notification as read
* @param int $notificationid The notification id
* @param \stdClass $notification The notification
* @param int|null $timeread The time the message was marked as read, if null will default to time()
*/
public static function mark_notification_as_read($userid, $notificationid, $timeread = null) {
public static function mark_notification_as_read($notification, $timeread = null) {
global $DB;
if (is_null($timeread)) {
$timeread = time();
}
// Make sure the notification is for the user.
$notification = $DB->get_record('notifications', ['id' => $notificationid, 'useridto' => $userid], '*', MUST_EXIST);
if (is_null($notification->timeread)) {
$updatenotification = new \stdClass();
$updatenotification->id = $notification->id;

View File

@ -1930,7 +1930,7 @@ class core_message_external extends external_api {
throw new invalid_parameter_exception('Invalid messageid, you don\'t have permissions to mark this message as read');
}
\core_message\api::mark_message_as_read($USER->id, $message->id, $timeread);
\core_message\api::mark_message_as_read($USER->id, $message, $timeread);
$results = array(
'messageid' => $message->id,

View File

@ -89,17 +89,17 @@ trait message_popup_test_helper {
$record->timecreated = $timecreated ? $timecreated : time();
$record->timeread = $timeread ? $timeread : time();
$id = $DB->insert_record('notifications', $record);
$record->id = $DB->insert_record('notifications', $record);
// Mark it as read.
\core_message\api::mark_notification_as_read($userto->id, $id);
\core_message\api::mark_notification_as_read($record);
$popup = new stdClass();
$popup->messageid = $id;
$popup->messageid = $record->id;
$popup->isread = 1;
$DB->insert_record('message_popup', $popup);
return $id;
return $record->id;
}
}

View File

@ -1638,6 +1638,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
* Test returning contacts with unread message count.
*/
public function test_get_contacts_with_unread_message_count() {
global $DB;
$user1 = self::getDataGenerator()->create_user();
$user2 = self::getDataGenerator()->create_user();
$user3 = self::getDataGenerator()->create_user();
@ -1677,8 +1679,10 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertEquals(5, $messageinfo2->messagecount);
// Mark some of the messages as read.
\core_message\api::mark_message_as_read($user2->id, $message4id);
\core_message\api::mark_message_as_read($user2->id, $message6id);
$m4 = $DB->get_record('messages', ['id' => $message4id]);
$m6 = $DB->get_record('messages', ['id' => $message6id]);
\core_message\api::mark_message_as_read($user2->id, $m4);
\core_message\api::mark_message_as_read($user2->id, $m6);
// Get the contacts and the unread message count.
$messages = \core_message\api::get_contacts_with_unread_message_count($user2->id);
@ -1713,7 +1717,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertEquals(3, $messageinfo1->messagecount);
// Mark the last message as read.
\core_message\api::mark_message_as_read($user1->id, $messageid);
$m = $DB->get_record('messages', ['id' => $messageid]);
\core_message\api::mark_message_as_read($user1->id, $m);
$messages = \core_message\api::get_contacts_with_unread_message_count($user1->id);
@ -1752,6 +1757,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
* Test returning non-contacts with unread message count.
*/
public function test_get_non_contacts_with_unread_message_count() {
global $DB;
$user1 = self::getDataGenerator()->create_user();
$user2 = self::getDataGenerator()->create_user();
$user3 = self::getDataGenerator()->create_user();
@ -1789,8 +1796,10 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertEquals(5, $messageinfo2->messagecount);
// Mark some of the messages as read.
\core_message\api::mark_message_as_read($user2->id, $message4id);
\core_message\api::mark_message_as_read($user2->id, $message6id);
$m4 = $DB->get_record('messages', ['id' => $message4id]);
$m6 = $DB->get_record('messages', ['id' => $message6id]);
\core_message\api::mark_message_as_read($user2->id, $m4);
\core_message\api::mark_message_as_read($user2->id, $m6);
// Get the non-contacts and the unread message count.
$messages = \core_message\api::get_non_contacts_with_unread_message_count($user2->id);
@ -1823,7 +1832,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertEquals(3, $messageinfo1->messagecount);
// Mark the last message as read.
\core_message\api::mark_message_as_read($user1->id, $messageid);
$m = $DB->get_record('messages', ['id' => $messageid]);
\core_message\api::mark_message_as_read($user1->id, $m);
// Get the non-contacts and the unread message count.
$messages = \core_message\api::get_non_contacts_with_unread_message_count($user1->id);
@ -1849,8 +1859,10 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->send_fake_message($user2, $user1);
$m4id = $this->send_fake_message($user2, $user1);
\core_message\api::mark_message_as_read($user2->id, $m2id, 11);
\core_message\api::mark_message_as_read($user1->id, $m4id, 12);
$m2 = $DB->get_record('messages', ['id' => $m2id]);
$m4 = $DB->get_record('messages', ['id' => $m4id]);
\core_message\api::mark_message_as_read($user2->id, $m2, 11);
\core_message\api::mark_message_as_read($user1->id, $m4, 12);
// Confirm there are two user actions.
$muas = $DB->get_records('message_user_actions', [], 'timecreated ASC');
@ -1887,8 +1899,11 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->send_fake_message($user2, $user1, 'Notification 3', 1);
$n4id = $this->send_fake_message($user2, $user1, 'Notification 4', 1);
\core_message\api::mark_notification_as_read($user2->id, $n2id, 11);
\core_message\api::mark_notification_as_read($user1->id, $n4id, 12);
$n2 = $DB->get_record('notifications', ['id' => $n2id]);
$n4 = $DB->get_record('notifications', ['id' => $n4id]);
\core_message\api::mark_notification_as_read($n2, 11);
\core_message\api::mark_notification_as_read($n4, 12);
// Retrieve the notifications.
$n2 = $DB->get_record('notifications', ['id' => $n2id]);

View File

@ -302,7 +302,8 @@ class core_message_events_testcase extends core_message_messagelib_testcase {
// Trigger and capture the event.
$sink = $this->redirectEvents();
\core_message\api::mark_message_as_read($user1->id, $messageid);
$message = $DB->get_record('messages', ['id' => $messageid]);
\core_message\api::mark_message_as_read($user1->id, $message);
$events = $sink->get_events();
$event = reset($events);
@ -352,7 +353,8 @@ class core_message_events_testcase extends core_message_messagelib_testcase {
// Create a read message.
$messageid = $this->send_fake_message($user1, $user2);
\core_message\api::mark_message_as_read($user2->id, $messageid);
$m = $DB->get_record('messages', ['id' => $messageid]);
\core_message\api::mark_message_as_read($user2->id, $m);
// Trigger and capture the event.
$sink = $this->redirectEvents();
@ -400,10 +402,14 @@ class core_message_events_testcase extends core_message_messagelib_testcase {
$messages[] = $this->send_fake_message($user2, $user1, 'Ouch.', 0, $time + 8);
// Mark the last 4 messages as read.
\core_message\api::mark_message_as_read($user2->id, $messages[4]);
\core_message\api::mark_message_as_read($user1->id, $messages[5]);
\core_message\api::mark_message_as_read($user2->id, $messages[6]);
\core_message\api::mark_message_as_read($user1->id, $messages[7]);
$m5 = $DB->get_record('messages', ['id' => $messages[4]]);
$m6 = $DB->get_record('messages', ['id' => $messages[5]]);
$m7 = $DB->get_record('messages', ['id' => $messages[6]]);
$m8 = $DB->get_record('messages', ['id' => $messages[7]]);
\core_message\api::mark_message_as_read($user2->id, $m5);
\core_message\api::mark_message_as_read($user1->id, $m6);
\core_message\api::mark_message_as_read($user2->id, $m7);
\core_message\api::mark_message_as_read($user1->id, $m8);
// Trigger and capture the event.
$sink = $this->redirectEvents();

View File

@ -832,7 +832,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
'action' => \core_message\api::MESSAGE_ACTION_DELETED)));
// Delete a message read.
\core_message\api::mark_message_as_read($user3->id, $m3to2, time());
$message = $DB->get_record('messages', ['id' => $m3to2]);
\core_message\api::mark_message_as_read($user3->id, $message, time());
$result = core_message_external::delete_message($m3to2, $user3->id);
$result = external_api::clean_returnvalue(core_message_external::delete_message_returns(), $result);
$this->assertTrue($result['status']);

View File

@ -223,6 +223,8 @@ class core_message_messagelib_testcase extends advanced_testcase {
* Test message_count_unread_messages with read messages.
*/
public function test_message_count_unread_messages_with_read_messages() {
global $DB;
// Create users to send and receive messages.
$userfrom1 = $this->getDataGenerator()->create_user();
$userfrom2 = $this->getDataGenerator()->create_user();
@ -235,7 +237,8 @@ class core_message_messagelib_testcase extends advanced_testcase {
$this->send_fake_message($userfrom2, $userto);
// Mark message as read.
\core_message\api::mark_message_as_read($userto->id, $messageid);
$message = $DB->get_record('messages', ['id' => $messageid]);
\core_message\api::mark_message_as_read($userto->id, $message);
// Should only count the messages that weren't read by the current user.
$this->assertEquals(1, message_count_unread_messages($userto));