diff --git a/lib/classes/event/message_deleted.php b/lib/classes/event/message_deleted.php index b991853b309..ab38448b1db 100644 --- a/lib/classes/event/message_deleted.php +++ b/lib/classes/event/message_deleted.php @@ -33,8 +33,6 @@ defined('MOODLE_INTERNAL') || die(); * Extra information about event. * * - int messageid: the id of the message. - * - int useridfrom: the id of the user who received the message. - * - int useridto: the id of the user who sent the message. * } * * @package core @@ -47,32 +45,22 @@ class message_deleted extends base { /** * Create event using ids. * - * @param int $userfromid the user who the message was from. - * @param int $usertoid the user who the message was sent to. - * @param int $userdeleted the user who deleted it. + * @param int $userid the user who the we are deleting the message for. + * @param int $userdeleting the user who deleted it (it's possible that an admin may delete a message on someones behalf) * @param int $messageid the id of the message that was deleted. - * @param int $muaid The id in the message_user_actions table + * @param int $muaid The id in the message_user_actions table. * @return message_deleted */ - public static function create_from_ids($userfromid, $usertoid, $userdeleted, $messageid, $muaid) { - // Check who was deleting the message. - if ($userdeleted == $userfromid) { - $relateduserid = $usertoid; - } else { - $relateduserid = $userfromid; - } - + public static function create_from_ids(int $userid, int $userdeleting, int $messageid, int $muaid) : message_deleted { // We set the userid to the user who deleted the message, nothing to do // with whether or not they sent or received the message. $event = self::create(array( 'objectid' => $muaid, - 'userid' => $userdeleted, + 'userid' => $userdeleting, 'context' => \context_system::instance(), - 'relateduserid' => $relateduserid, + 'relateduserid' => $userid, 'other' => array( 'messageid' => $messageid, - 'useridfrom' => $userfromid, - 'useridto' => $usertoid ) )); @@ -103,14 +91,28 @@ class message_deleted extends base { * @return string */ public function get_description() { - // Check if the person who deleted the message received or sent it. - if ($this->userid == $this->other['useridto']) { - $str = 'from'; - } else { - $str = 'to'; + // This is for BC when the event used to take this value into account before group conversations. + // We still want the same message to display for older events. + if (isset($this->other['useridto'])) { + // Check if the person who deleted the message received or sent it. + if ($this->userid == $this->other['useridto']) { + $str = 'from'; + } else { + $str = 'to'; + } + + return "The user with id '$this->userid' deleted a message sent $str the user with id '$this->relateduserid'."; } - return "The user with id '$this->userid' deleted a message sent $str the user with id '$this->relateduserid'."; + $messageid = $this->other['messageid']; + + // Check if the user deleting the message was not the actual user we are deleting for. + $str = "The user with id '$this->userid' deleted a message with id '$messageid'"; + if ($this->userid != $this->relateduserid) { + $str .= " for the user with id '$this->relateduserid'"; + } + + return $str; } /** @@ -129,14 +131,6 @@ class message_deleted extends base { if (!isset($this->other['messageid'])) { throw new \coding_exception('The \'messageid\' value must be set in other.'); } - - if (!isset($this->other['useridfrom'])) { - throw new \coding_exception('The \'useridfrom\' value must be set in other.'); - } - - if (!isset($this->other['useridto'])) { - throw new \coding_exception('The \'useridto\' value must be set in other.'); - } } public static function get_objectid_mapping() { @@ -145,9 +139,9 @@ class message_deleted extends base { public static function get_other_mapping() { // Messages are not backed up, so no need to map them on restore. - $othermapped = array(); - $othermapped['useridfrom'] = array('db' => 'user', 'restore' => base::NOT_MAPPED); - $othermapped['useridto'] = array('db' => 'user', 'restore' => base::NOT_MAPPED); + $othermapped = []; + $othermapped['messageid'] = ['db' => 'messages', 'restore' => base::NOT_MAPPED]; + return $othermapped; } } diff --git a/lib/db/services.php b/lib/db/services.php index 10bd788f3bd..f187293dfdd 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -917,7 +917,18 @@ $functions = array( 'classname' => 'core_message_external', 'methodname' => 'delete_conversation', 'classpath' => 'message/externallib.php', - 'description' => 'Deletes a conversation.', + 'description' => '** DEPRECATED ** Please do not call this function any more. + Deletes a conversation.', + 'type' => 'write', + 'capabilities' => 'moodle/site:deleteownmessage', + 'ajax' => true, + 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), + ), + 'core_message_delete_conversations_by_id' => array( + 'classname' => 'core_message_external', + 'methodname' => 'delete_conversations_by_id', + 'classpath' => 'message/externallib.php', + 'description' => 'Deletes a list of conversations.', 'type' => 'write', 'capabilities' => 'moodle/site:deleteownmessage', 'ajax' => true, diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 288ed76e1bc..6561f6525cd 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -135,6 +135,9 @@ the groupid field. - message_contact_unblocked The reason for this is because you can now block/unblock users without them necessarily being a contact. These events have been replaced with message_user_blocked and message_user_unblocked respectively. +* The event message_deleted has been changed, it no longer records the value of the 'useridto' due to + the introduction of group messaging. Please, if you have any observers or are triggering this event + in your code you will have to make some changes! === 3.5 === diff --git a/message/classes/api.php b/message/classes/api.php index 8bc93a74288..092f47c2990 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -628,17 +628,30 @@ class api { * * @param int $userid The user id of who we want to delete the messages for (this may be done by the admin * but will still seem as if it was by the user) + * @param int $conversationid The id of the conversation * @return bool Returns true if a user can delete the conversation, false otherwise. */ - public static function can_delete_conversation($userid) { + public static function can_delete_conversation(int $userid, int $conversationid = null) : bool { global $USER; + if (is_null($conversationid)) { + debugging('\core_message\api::can_delete_conversation() now expects a \'conversationid\' to be passed.', + DEBUG_DEVELOPER); + return false; + } + $systemcontext = \context_system::instance(); - // Let's check if the user is allowed to delete this conversation. - if (has_capability('moodle/site:deleteanymessage', $systemcontext) || - ((has_capability('moodle/site:deleteownmessage', $systemcontext) && - $USER->id == $userid))) { + if (has_capability('moodle/site:deleteanymessage', $systemcontext)) { + return true; + } + + if (!self::is_user_in_conversation($userid, $conversationid)) { + return false; + } + + if (has_capability('moodle/site:deleteownmessage', $systemcontext) && + $USER->id == $userid) { return true; } @@ -650,13 +663,15 @@ class api { * * This function does not verify any permissions. * + * @deprecated since 3.6 * @param int $userid The user id of who we want to delete the messages for (this may be done by the admin * but will still seem as if it was by the user) * @param int $otheruserid The id of the other user in the conversation * @return bool */ public static function delete_conversation($userid, $otheruserid) { - global $DB, $USER; + debugging('\core_message\api::delete_conversation() is deprecated, please use ' . + '\core_message\api::delete_conversation_by_id() instead.', DEBUG_DEVELOPER); $conversationid = self::get_conversation_between_users([$userid, $otheruserid]); @@ -665,6 +680,23 @@ class api { return true; } + self::delete_conversation_by_id($userid, $conversationid); + + return true; + } + + /** + * Deletes a conversation for a specified user. + * + * This function does not verify any permissions. + * + * @param int $userid The user id of who we want to delete the messages for (this may be done by the admin + * but will still seem as if it was by the user) + * @param int $conversationid The id of the other user in the conversation + */ + public static function delete_conversation_by_id(int $userid, int $conversationid) { + global $DB, $USER; + // Get all messages belonging to this conversation that have not already been deleted by this user. $sql = "SELECT m.* FROM {messages} m @@ -686,16 +718,9 @@ class api { $mua->timecreated = time(); $mua->id = $DB->insert_record('message_user_actions', $mua); - if ($message->useridfrom == $userid) { - $useridto = $otheruserid; - } else { - $useridto = $userid; - } - \core\event\message_deleted::create_from_ids($message->useridfrom, $useridto, - $USER->id, $message->id, $mua->id)->trigger(); + \core\event\message_deleted::create_from_ids($userid, $USER->id, + $message->id, $mua->id)->trigger(); } - - return true; } /** @@ -1202,30 +1227,20 @@ class api { public static function can_delete_message($userid, $messageid) { global $DB, $USER; - $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto - FROM {messages} m - INNER JOIN {message_conversations} mc - ON m.conversationid = mc.id - INNER JOIN {message_conversation_members} mcm - ON mcm.conversationid = mc.id - WHERE mcm.userid != m.useridfrom - AND m.id = ?"; - $message = $DB->get_record_sql($sql, [$messageid], MUST_EXIST); + $systemcontext = \context_system::instance(); - if ($message->useridfrom == $userid) { - $userdeleting = 'useridfrom'; - } else if ($message->useridto == $userid) { - $userdeleting = 'useridto'; - } else { + $conversationid = $DB->get_field('messages', 'conversationid', ['id' => $messageid], MUST_EXIST); + + if (has_capability('moodle/site:deleteanymessage', $systemcontext)) { + return true; + } + + if (!self::is_user_in_conversation($userid, $conversationid)) { return false; } - $systemcontext = \context_system::instance(); - - // Let's check if the user is allowed to delete this message. - if (has_capability('moodle/site:deleteanymessage', $systemcontext) || - ((has_capability('moodle/site:deleteownmessage', $systemcontext) && - $USER->id == $message->$userdeleting))) { + if (has_capability('moodle/site:deleteownmessage', $systemcontext) && + $USER->id == $userid) { return true; } @@ -1243,17 +1258,11 @@ class api { * @return bool */ public static function delete_message($userid, $messageid) { - global $DB; + global $DB, $USER; - $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto - FROM {messages} m - INNER JOIN {message_conversations} mc - ON m.conversationid = mc.id - INNER JOIN {message_conversation_members} mcm - ON mcm.conversationid = mc.id - WHERE mcm.userid != m.useridfrom - AND m.id = ?"; - $message = $DB->get_record_sql($sql, [$messageid], MUST_EXIST); + if (!$DB->record_exists('messages', ['id' => $messageid])) { + return false; + } // Check if the user has already deleted this message. if (!$DB->record_exists('message_user_actions', ['userid' => $userid, @@ -1266,8 +1275,8 @@ class api { $mua->id = $DB->insert_record('message_user_actions', $mua); // Trigger event for deleting a message. - \core\event\message_deleted::create_from_ids($message->useridfrom, $message->useridto, - $userid, $message->id, $mua->id)->trigger(); + \core\event\message_deleted::create_from_ids($userid, $USER->id, + $messageid, $mua->id)->trigger(); return true; } @@ -1612,4 +1621,19 @@ class api { OR (mcr.userid = ? AND mcr.requesteduserid = ?)"; return $DB->record_exists_sql($sql, [$userid, $requesteduserid, $requesteduserid, $userid]); } + + /** + * Checks if a user is already in a conversation. + * + * @param int $userid The id of the user we want to check if they are in a group + * @param int $conversationid The id of the conversation + * @return bool Returns true if a contact request exists, false otherwise + */ + public static function is_user_in_conversation(int $userid, int $conversationid) : bool { + global $DB; + + return $DB->record_exists('message_conversation_members', ['conversationid' => $conversationid, + 'userid' => $userid]); + + } } diff --git a/message/externallib.php b/message/externallib.php index f678c2f2732..dec1cd573a2 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -2541,6 +2541,7 @@ class core_message_external extends external_api { /** * Returns description of method parameters. * + * @deprecated since 3.6 * @return external_function_parameters * @since 3.2 */ @@ -2556,6 +2557,7 @@ class core_message_external extends external_api { /** * Deletes a conversation. * + * @deprecated since 3.6 * @param int $userid The user id of who we want to delete the conversation for * @param int $otheruserid The user id of the other user in the conversation * @return array @@ -2587,8 +2589,13 @@ class core_message_external extends external_api { $user = core_user::get_user($params['userid'], '*', MUST_EXIST); core_user::require_active_user($user); - if (\core_message\api::can_delete_conversation($user->id)) { - $status = \core_message\api::delete_conversation($user->id, $otheruserid); + if (!$conversationid = \core_message\api::get_conversation_between_users([$userid, $otheruserid])) { + return []; + } + + if (\core_message\api::can_delete_conversation($user->id, $conversationid)) { + \core_message\api::delete_conversation_by_id($user->id, $conversationid); + $status = true; } else { throw new moodle_exception('You do not have permission to delete messages'); } @@ -2604,6 +2611,7 @@ class core_message_external extends external_api { /** * Returns description of method result value. * + * @deprecated since 3.6 * @return external_description * @since 3.2 */ @@ -2616,6 +2624,85 @@ class core_message_external extends external_api { ); } + /** + * Marking the method as deprecated. + * + * @return bool + */ + public static function delete_conversation_is_deprecated() { + return true; + } + + /** + * Returns description of method parameters. + * + * @return external_function_parameters + * @since 3.6 + */ + public static function delete_conversations_by_id_parameters() { + return new external_function_parameters( + array( + 'userid' => new external_value(PARAM_INT, 'The user id of who we want to delete the conversation for'), + 'conversationids' => new external_multiple_structure( + new external_value(PARAM_INT, 'The id of the conversation'), + 'List of conversation IDs' + ), + ) + ); + } + + /** + * Deletes a conversation. + * + * @param int $userid The user id of who we want to delete the conversation for + * @param int[] $conversationids The ids of the conversations + * @return array + * @throws moodle_exception + * @since 3.6 + */ + public static function delete_conversations_by_id($userid, array $conversationids) { + global $CFG; + + // Check if private messaging between users is allowed. + if (empty($CFG->messaging)) { + throw new moodle_exception('disabled', 'message'); + } + + // Validate params. + $params = [ + 'userid' => $userid, + 'conversationids' => $conversationids, + ]; + $params = self::validate_parameters(self::delete_conversations_by_id_parameters(), $params); + + // Validate context. + $context = context_system::instance(); + self::validate_context($context); + + $user = core_user::get_user($params['userid'], '*', MUST_EXIST); + core_user::require_active_user($user); + + foreach ($conversationids as $conversationid) { + if (\core_message\api::can_delete_conversation($user->id, $conversationid)) { + \core_message\api::delete_conversation_by_id($user->id, $conversationid); + } else { + throw new moodle_exception("You do not have permission to delete the conversation '$conversationid'"); + } + } + + return []; + } + + /** + * Returns description of method result value. + * + * @return external_description + * @since 3.6 + */ + public static function delete_conversations_by_id_returns() { + return new external_warnings(); + } + /** * Returns description of method parameters * @@ -3012,7 +3099,6 @@ class core_message_external extends external_api { ); } - /** * Returns description of method parameters * diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 0278d679352..f3253e6eb83 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -1146,17 +1146,26 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user1 = self::getDataGenerator()->create_user(); $user2 = 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_delete_conversation($user1->id)); + $this->assertTrue(\core_message\api::can_delete_conversation($user1->id, $conversationid)); // Set as the user 1. $this->setUser($user1); // They can delete their own messages. - $this->assertTrue(\core_message\api::can_delete_conversation($user1->id)); + $this->assertTrue(\core_message\api::can_delete_conversation($user1->id, $conversationid)); // They can't delete someone elses. - $this->assertFalse(\core_message\api::can_delete_conversation($user2->id)); + $this->assertFalse(\core_message\api::can_delete_conversation($user2->id, $conversationid)); } /** @@ -1181,6 +1190,58 @@ class core_message_api_testcase extends core_message_messagelib_testcase { // Delete the conversation as user 1. \core_message\api::delete_conversation($user1->id, $user2->id); + $this->assertDebuggingCalled(); + + $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); + $this->assertCount(4, $muas); + // Sort by id. + ksort($muas); + + $mua1 = array_shift($muas); + $mua2 = array_shift($muas); + $mua3 = array_shift($muas); + $mua4 = array_shift($muas); + + $this->assertEquals($user1->id, $mua1->userid); + $this->assertEquals($m1id, $mua1->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action); + + $this->assertEquals($user1->id, $mua2->userid); + $this->assertEquals($m2id, $mua2->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action); + + $this->assertEquals($user1->id, $mua3->userid); + $this->assertEquals($m3id, $mua3->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action); + + $this->assertEquals($user1->id, $mua4->userid); + $this->assertEquals($m4id, $mua4->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua4->action); + } + + /** + * Tests deleting a conversation by conversation id. + */ + public function test_delete_conversation_by_id() { + global $DB; + + // Create some users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // The person doing the search. + $this->setUser($user1); + + // Send some messages back and forth. + $time = 1; + $m1id = $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1); + $m2id = $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2); + $m3id = $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3); + $m4id = $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4); + + // Delete the conversation as user 1. + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + \core_message\api::delete_conversation_by_id($user1->id, $conversationid); $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); $this->assertCount(4, $muas); @@ -2226,6 +2287,31 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertTrue(\core_message\api::does_contact_request_exist($user2->id, $user1->id)); } + /** + * Test the user in conversation check. + */ + public function test_is_user_in_conversation() { + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]); + + $this->assertTrue(\core_message\api::is_user_in_conversation($user1->id, $conversationid)); + } + + /** + * Test the user in conversation check when they are not. + */ + public function test_is_user_in_conversation_when_not() { + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]); + + $this->assertFalse(\core_message\api::is_user_in_conversation($user3->id, $conversationid)); + } + /** * Comparison function for sorting contacts. * diff --git a/message/tests/events_test.php b/message/tests/events_test.php index 25660ce5054..714461b9fce 100644 --- a/message/tests/events_test.php +++ b/message/tests/events_test.php @@ -274,7 +274,9 @@ class core_message_events_testcase extends core_message_messagelib_testcase { * Test the message deleted event. */ public function test_message_deleted() { - global $DB; + global $DB, $USER; + + $this->setAdminUser(); // Create users to send messages between. $user1 = $this->getDataGenerator()->create_user(); @@ -294,12 +296,12 @@ class core_message_events_testcase extends core_message_messagelib_testcase { // Check that the event data is valid. $this->assertInstanceOf('\core\event\message_deleted', $event); - $this->assertEquals($user1->id, $event->userid); // The user who deleted it. - $this->assertEquals($user2->id, $event->relateduserid); + $this->assertEquals($USER->id, $event->userid); // The user who deleted it. + $this->assertEquals($user1->id, $event->relateduserid); $this->assertEquals($mua->id, $event->objectid); $this->assertEquals($messageid, $event->other['messageid']); - $this->assertEquals($user1->id, $event->other['useridfrom']); - $this->assertEquals($user2->id, $event->other['useridto']); + + $this->setUser($user1); // Create a read message. $messageid = $this->send_fake_message($user1, $user2); @@ -318,12 +320,10 @@ class core_message_events_testcase extends core_message_messagelib_testcase { // Check that the event data is valid. $this->assertInstanceOf('\core\event\message_deleted', $event); - $this->assertEquals($user2->id, $event->userid); - $this->assertEquals($user1->id, $event->relateduserid); + $this->assertEquals($user1->id, $event->userid); + $this->assertEquals($user2->id, $event->relateduserid); $this->assertEquals($mua->id, $event->objectid); $this->assertEquals($messageid, $event->other['messageid']); - $this->assertEquals($user1->id, $event->other['useridfrom']); - $this->assertEquals($user2->id, $event->other['useridto']); } /** @@ -336,7 +336,7 @@ class core_message_events_testcase extends core_message_messagelib_testcase { $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); - // The person doing the search. + // The person doing the deletion. $this->setUser($user1); // Send some messages back and forth. @@ -364,6 +364,7 @@ class core_message_events_testcase extends core_message_messagelib_testcase { // Trigger and capture the event. $sink = $this->redirectEvents(); \core_message\api::delete_conversation($user1->id, $user2->id); + $this->assertDebuggingCalled(); $events = $sink->get_events(); // Get the user actions for the messages deleted by that user. @@ -383,18 +384,14 @@ class core_message_events_testcase extends core_message_messagelib_testcase { // Check that the event data is valid. $i = 1; foreach ($events as $event) { - $useridfromid = ($i % 2 == 0) ? $user2->id : $user1->id; - $useridtoid = ($i % 2 == 0) ? $user1->id : $user2->id; $messageid = $messages[$i - 1]; $this->assertInstanceOf('\core\event\message_deleted', $event); $this->assertEquals($muatest[$messageid]->id, $event->objectid); $this->assertEquals($user1->id, $event->userid); - $this->assertEquals($user2->id, $event->relateduserid); + $this->assertEquals($user1->id, $event->relateduserid); $this->assertEquals($messageid, $event->other['messageid']); - $this->assertEquals($useridfromid, $event->other['useridfrom']); - $this->assertEquals($useridtoid, $event->other['useridto']); $i++; } diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 784ad149ee2..16d38817f27 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -1648,7 +1648,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $result = core_message_external::delete_message(-1, $user1->id); $this->fail('Exception expected due invalid messageid.'); } catch (dml_missing_record_exception $e) { - $this->assertEquals('invalidrecordunknown', $e->errorcode); + $this->assertEquals('invalidrecord', $e->errorcode); } // Invalid user. @@ -3445,6 +3445,13 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $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); + // The person wanting to delete the conversation. $this->setUser($user3); @@ -3476,6 +3483,173 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { core_message_external::delete_conversation($user1->id, $user2->id); } + /** + * Test deleting conversations. + */ + public function test_delete_conversations_by_id() { + global $DB; + + $this->resetAfterTest(true); + + // Create some users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // The person wanting to delete the conversation. + $this->setUser($user1); + + // Send some messages back and forth. + $time = time(); + $m1id = $this->send_message($user1, $user2, 'Yo!', 0, $time); + $m2id = $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $m3id = $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $m4id = $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // Delete the conversation. + core_message_external::delete_conversations_by_id($user1->id, [$conversationid]); + + $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); + $this->assertCount(4, $muas); + // Sort by id. + ksort($muas); + + $mua1 = array_shift($muas); + $mua2 = array_shift($muas); + $mua3 = array_shift($muas); + $mua4 = array_shift($muas); + + $this->assertEquals($user1->id, $mua1->userid); + $this->assertEquals($m1id, $mua1->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action); + + $this->assertEquals($user1->id, $mua2->userid); + $this->assertEquals($m2id, $mua2->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action); + + $this->assertEquals($user1->id, $mua3->userid); + $this->assertEquals($m3id, $mua3->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action); + + $this->assertEquals($user1->id, $mua4->userid); + $this->assertEquals($m4id, $mua4->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua4->action); + } + + /** + * Test deleting conversations as other user. + */ + public function test_delete_conversations_by_id_as_other_user() { + global $DB; + + $this->resetAfterTest(true); + + $this->setAdminUser(); + + // Create some users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + // Send some messages back and forth. + $time = time(); + $m1id = $this->send_message($user1, $user2, 'Yo!', 0, $time); + $m2id = $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $m3id = $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $m4id = $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + + $conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id]); + + // Delete the conversation. + core_message_external::delete_conversations_by_id($user1->id, [$conversationid]); + + $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); + $this->assertCount(4, $muas); + // Sort by id. + ksort($muas); + + $mua1 = array_shift($muas); + $mua2 = array_shift($muas); + $mua3 = array_shift($muas); + $mua4 = array_shift($muas); + + $this->assertEquals($user1->id, $mua1->userid); + $this->assertEquals($m1id, $mua1->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action); + + $this->assertEquals($user1->id, $mua2->userid); + $this->assertEquals($m2id, $mua2->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action); + + $this->assertEquals($user1->id, $mua3->userid); + $this->assertEquals($m3id, $mua3->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action); + + $this->assertEquals($user1->id, $mua4->userid); + $this->assertEquals($m4id, $mua4->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua4->action); + } + + /** + * Test deleting conversations as other user without proper capability. + */ + public function test_delete_conversations_by_id_as_other_user_without_cap() { + $this->resetAfterTest(true); + + // 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 = 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]); + + // The person wanting to delete the conversation. + $this->setUser($user3); + + // Ensure an exception is thrown. + $this->expectException('moodle_exception'); + core_message_external::delete_conversations_by_id($user1->id, [$conversationid]); + } + + /** + * Test deleting conversations with messaging disabled. + */ + public function test_delete_conversations_by_id_messaging_disabled() { + global $CFG; + + $this->resetAfterTest(true); + + // Create some users. + $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]); + + // The person wanting to delete the conversation. + $this->setUser($user1); + + // Disable messaging. + $CFG->messaging = 0; + + // Ensure an exception is thrown. + $this->expectException('moodle_exception'); + core_message_external::delete_conversations_by_id($user1->id, [$conversationid]); + } + /** * Test get message processor. */ diff --git a/message/upgrade.txt b/message/upgrade.txt index 2ada6522aee..c543dc8f2b4 100644 --- a/message/upgrade.txt +++ b/message/upgrade.txt @@ -28,10 +28,14 @@ information provided here is intended especially for developers. Please see their declaration in lib/deprecatedlib.php to view their alternatives (if applicable). * The following methods have been deprecated and should not be used any more: - \core_message\api::is_user_blocked() + - \core_message\api::delete_conversation() +* The method \core_message\api::can_delete_conversation() now expects a 'conversationid' to be passed + as the second parameter. * The following web services have been deprecated. Please do not call these any more. - - core_message_external::block_contacts, please use core_message_external::block_user instead. - - 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::block_contacts(), please use core_message_external::block_user() instead. + - 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. === 3.5 === diff --git a/version.php b/version.php index 7ae9209295b..2b6955fc885 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018101600.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2018101700.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.