From e4c309944435a03c71b60ac6cfe5417d6c49a321 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 15 Oct 2018 15:12:15 +0800 Subject: [PATCH 1/3] MDL-63548 core_message: deprecated mark_all_messages_as_read web service --- lib/db/services.php | 3 ++- message/externallib.php | 18 +++++++++++++++--- message/upgrade.txt | 2 ++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/db/services.php b/lib/db/services.php index 5cec94e4616..4265c1413fe 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -1059,7 +1059,8 @@ $functions = array( 'classname' => 'core_message_external', 'methodname' => 'mark_all_messages_as_read', 'classpath' => 'message/externallib.php', - 'description' => 'Mark all messages as read for a given user', + 'description' => '** DEPRECATED ** Please do not call this function any more. + Mark all messages as read for a given user', 'type' => 'write', 'ajax' => true, 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), diff --git a/message/externallib.php b/message/externallib.php index 89016ecb451..1dba7ed5b18 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -2414,6 +2414,7 @@ class core_message_external extends external_api { /** * Mark all messages as read parameters description. * + * @deprecated since 3.6 * @return external_function_parameters * @since 3.2 */ @@ -2429,14 +2430,15 @@ class core_message_external extends external_api { } /** - * Mark all notifications as read function. + * Mark all messages as read function. * - * @since 3.2 + * @deprecated since 3.6 * @throws invalid_parameter_exception * @throws moodle_exception * @param int $useridto the user id who received the message * @param int $useridfrom the user id who send the message. -10 or -20 for no-reply or support user * @return external_description + * @since 3.2 */ public static function mark_all_messages_as_read($useridto, $useridfrom) { global $USER, $CFG; @@ -2492,8 +2494,9 @@ class core_message_external extends external_api { } /** - * Mark all notifications as read return description. + * Mark all messages as read return description. * + * @deprecated since 3.6 * @return external_single_structure * @since 3.2 */ @@ -2501,6 +2504,15 @@ class core_message_external extends external_api { return new external_value(PARAM_BOOL, 'True if the messages were marked read, false otherwise'); } + /** + * Marking the method as deprecated. + * + * @return bool + */ + public static function mark_all_messages_as_read_is_deprecated() { + return true; + } + /** * Returns description of method parameters. * diff --git a/message/upgrade.txt b/message/upgrade.txt index b623af2ae8b..1a7ec62dcc8 100644 --- a/message/upgrade.txt +++ b/message/upgrade.txt @@ -36,6 +36,8 @@ information provided here is intended especially for developers. - core_message_external::unblock_contacts(), please use core_message_external::unblock_user() instead. - core_message_external::create_contacts(), please use core_message_external::create_contact_request() instead. - core_message_external::delete_conversation(), please use core_message_external::delete_conversations_by_id() instead. + - core_message_external::core_message_mark_all_messages_as_read(), please use + core_message_external::core_message_mark_all_conversation_messages_as_read() instead. * The following function has been added for getting the privacy messaging preference: - get_user_privacy_messaging_preference() From abf7a261a91762bcc1a42c84e9555368676604ad Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 15 Oct 2018 20:15:04 +0800 Subject: [PATCH 2/3] MDL-63548 core_message: added api::can_mark_all_messages_as_read --- message/classes/api.php | 28 ++++++++++++++++++++++++++++ message/tests/api_test.php | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/message/classes/api.php b/message/classes/api.php index d9b507edb40..90e28c94902 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -767,6 +767,34 @@ class api { return $DB->count_records_sql($sql, [$user->id, self::MESSAGE_ACTION_READ, $user->id]); } + /** + * Checks if a user can mark all messages as read. + * + * @param int $userid The user id of who we want to mark the messages for + * @param int $conversationid The id of the conversation + * @return bool true if user is permitted, false otherwise + * @since 3.6 + */ + public static function can_mark_all_messages_as_read(int $userid, int $conversationid) : bool { + global $USER; + + $systemcontext = \context_system::instance(); + + if (has_capability('moodle/site:readallmessages', $systemcontext)) { + return true; + } + + if (!self::is_user_in_conversation($userid, $conversationid)) { + return false; + } + + if ($USER->id == $userid) { + return true; + } + + return false; + } + /** * Marks all messages being sent to a user in a particular conversation. * diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 8be0f3f6070..907346e0b99 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -1136,6 +1136,43 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertFalse($profile->iscontact); } + /** + * Tests checking if a user can mark all messages as read. + */ + public function test_can_mark_all_messages_as_read() { + // Set as the admin. + $this->setAdminUser(); + + // Create some users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = 1; + $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1); + $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2); + $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3); + $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // The admin can do anything. + $this->assertTrue(\core_message\api::can_mark_all_messages_as_read($user1->id, $conversationid)); + + // Set as the user 1. + $this->setUser($user1); + + // The user can mark the messages as he is in the conversation. + $this->assertTrue(\core_message\api::can_mark_all_messages_as_read($user1->id, $conversationid)); + + // User 1 can not mark the messages read for user 2. + $this->assertFalse(\core_message\api::can_mark_all_messages_as_read($user2->id, $conversationid)); + + // This user is not a part of the conversation. + $this->assertFalse(\core_message\api::can_mark_all_messages_as_read($user3->id, $conversationid)); + } + /** * Tests checking if a user can delete a conversation. */ From 09ec50178161574928d89f3074e53a5740a052d5 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 15 Oct 2018 17:28:52 +0800 Subject: [PATCH 3/3] MDL-63548 core_message: added mark_all_conversation_messages_as_read WS --- lib/db/services.php | 9 +++ message/externallib.php | 60 ++++++++++++++ message/tests/externallib_test.php | 122 +++++++++++++++++++++++++++++ version.php | 2 +- 4 files changed, 192 insertions(+), 1 deletion(-) diff --git a/lib/db/services.php b/lib/db/services.php index 4265c1413fe..3b40f3efa28 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -1065,6 +1065,15 @@ $functions = array( 'ajax' => true, 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), ), + 'core_message_mark_all_conversation_messages_as_read' => array( + 'classname' => 'core_message_external', + 'methodname' => 'mark_all_conversation_messages_as_read', + 'classpath' => 'message/externallib.php', + 'description' => 'Mark all conversation messages as read for a given user', + 'type' => 'write', + 'ajax' => true, + 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), + ), 'core_message_mark_message_read' => array( 'classname' => 'core_message_external', 'methodname' => 'mark_message_read', diff --git a/message/externallib.php b/message/externallib.php index 1dba7ed5b18..f201e836faf 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -2513,6 +2513,66 @@ class core_message_external extends external_api { return true; } + /** + * Mark all conversation messages as read parameters description. + * + * @return external_function_parameters + * @since 3.6 + */ + public static function mark_all_conversation_messages_as_read_parameters() { + return new external_function_parameters( + array( + 'userid' => new external_value(PARAM_INT, 'The user id who who we are marking the messages as read for'), + 'conversationid' => + new external_value(PARAM_INT, 'The conversation id who who we are marking the messages as read for') + ) + ); + } + + /** + * Mark all conversation messages as read function. + * + * @param int $userid The user id of who we want to delete the conversation for + * @param int $conversationid The id of the conversations + * @since 3.6 + */ + public static function mark_all_conversation_messages_as_read(int $userid, int $conversationid) { + global $CFG; + + // Check if messaging is enabled. + if (empty($CFG->messaging)) { + throw new moodle_exception('disabled', 'message'); + } + + $params = array( + 'userid' => $userid, + 'conversationid' => $conversationid, + ); + $params = self::validate_parameters(self::mark_all_conversation_messages_as_read_parameters(), $params); + + $context = context_system::instance(); + self::validate_context($context); + + $user = core_user::get_user($params['userid'], '*', MUST_EXIST); + core_user::require_active_user($user); + + if (\core_message\api::can_mark_all_messages_as_read($userid, $conversationid)) { + \core_message\api::mark_all_messages_as_read($userid, $conversationid); + } else { + throw new moodle_exception('accessdenied', 'admin'); + } + } + + /** + * Mark all conversation messages as read return description. + * + * @return external_warnings + * @since 3.6 + */ + public static function mark_all_conversation_messages_as_read_returns() { + return new external_warnings(); + } + /** * Returns description of method parameters. * diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index aa7f61f8d52..505d7553230 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -3245,6 +3245,128 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(6, $DB->count_records('message_user_actions')); } + /** + * Test marking all conversation messages as read with an invalid user. + */ + public function test_mark_all_conversation_messages_as_read_invalid_user_exception() { + $this->resetAfterTest(true); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = time(); + $this->send_message($user1, $user2, 'Yo!', 0, $time); + $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + $this->expectException('moodle_exception'); + core_message_external::mark_all_conversation_messages_as_read(-2132131, $conversationid); + } + + /** + * Test marking all conversation messages as read without proper access. + */ + public function test_mark_all_conversation_messages_as_read_access_denied_exception() { + $this->resetAfterTest(true); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = time(); + $this->send_message($user1, $user2, 'Yo!', 0, $time); + $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // User 3 is not in the conversation. + $this->expectException('moodle_exception'); + core_message_external::mark_all_conversation_messages_as_read($user3->id, $conversationid); + } + + /** + * Test marking all conversation messages as read for another user. + */ + public function test_mark_all_conversation_messages_as_read_wrong_user() { + $this->resetAfterTest(true); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = time(); + $this->send_message($user1, $user2, 'Yo!', 0, $time); + $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // Can't mark the messages as read for user 2. + $this->setUser($user1); + $this->expectException('moodle_exception'); + core_message_external::mark_all_conversation_messages_as_read($user2->id, $conversationid); + } + + /** + * Test marking all conversation messages as admin. + */ + public function test_mark_all_conversation_messages_as_admin() { + global $DB; + + $this->resetAfterTest(true); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = time(); + $this->send_message($user1, $user2, 'Yo!', 0, $time); + $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // Admin can do anything. + $this->setAdminUser(); + core_message_external::mark_all_conversation_messages_as_read($user2->id, $conversationid); + $this->assertEquals(2, $DB->count_records('message_user_actions')); + } + + /** + * Test marking all conversation messages. + */ + public function test_mark_all_conversation_messages_as_read() { + global $DB; + + $this->resetAfterTest(true); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = time(); + $this->send_message($user1, $user2, 'Yo!', 0, $time); + $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // We are the user we want to mark the messages for and we are in the conversation, all good. + $this->setUser($user1); + core_message_external::mark_all_conversation_messages_as_read($user1->id, $conversationid); + $this->assertEquals(2, $DB->count_records('message_user_actions')); + } + /** * Test getting unread conversation count. */ diff --git a/version.php b/version.php index d10fd0eef92..82f0af37444 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018101900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2018101900.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.