MDL-63724 core_message: add send_message_to_conversation() to api
authorJake Dallimore <jake@moodle.com>
Mon, 12 Nov 2018 02:05:17 +0000 (10:05 +0800)
committerJake Dallimore <jake@moodle.com>
Mon, 12 Nov 2018 02:09:27 +0000 (10:09 +0800)
lib/messagelib.php
lib/tests/message_test.php
lib/tests/messagelib_test.php
message/classes/api.php
message/tests/api_test.php

index 6d7f597..3008d21 100644 (file)
@@ -74,18 +74,72 @@ function message_send(\core\message\message $eventdata) {
         $eventdata->notification = 1;
     }
 
-    // This is a message directed to a conversation, not a specific user as was the way in legacy messaging .
-    // We must call send_message_to_conversation(), which handles per-member processor iteration and triggers
-    // a per-conversation event.
-    if (!$eventdata->notification && $eventdata->convid) {
-        if (!is_object($eventdata->userfrom)) {
-            $eventdata->userfrom = core_user::get_user($eventdata->userfrom);
+    if (!is_object($eventdata->userfrom)) {
+        $eventdata->userfrom = core_user::get_user($eventdata->userfrom);
+    }
+    if (!$eventdata->userfrom) {
+        debugging('Attempt to send msg from unknown user', DEBUG_NORMAL);
+        return false;
+    }
+
+    // Legacy messages (FROM a single user TO a single user) must be converted into conversation messages.
+    // Then, these will be passed through the conversation messages code below.
+    if (!$eventdata->notification && !$eventdata->convid) {
+        // If messaging is disabled at the site level, then the 'instantmessage' provider is always disabled.
+        // Given this is the only 'message' type message provider, we can exit now if this is the case.
+        // Don't waste processing time trying to work out the other conversation member, if it's an individual
+        // conversation, just throw a generic debugging notice and return.
+        if (empty($CFG->messaging) || $eventdata->component !== 'moodle' || $eventdata->name !== 'instantmessage') {
+            debugging('Attempt to send msg from a provider '.$eventdata->component.'/'.$eventdata->name.
+                ' that is inactive or not allowed for the user id='.$eventdata->userto->id, DEBUG_NORMAL);
+            return false;
         }
-        if (!$eventdata->userfrom) {
-            debugging('Attempt to send msg from unknown user', DEBUG_NORMAL);
+
+        if (!is_object($eventdata->userto)) {
+            $eventdata->userto = core_user::get_user($eventdata->userto);
+        }
+        if (!$eventdata->userto) {
+            debugging('Attempt to send msg to unknown user', DEBUG_NORMAL);
             return false;
         }
 
+        // Verify all necessary data fields are present.
+        if (!isset($eventdata->userto->auth) or !isset($eventdata->userto->suspended)
+            or !isset($eventdata->userto->deleted) or !isset($eventdata->userto->emailstop)) {
+
+            debugging('Necessary properties missing in userto object, fetching full record', DEBUG_DEVELOPER);
+            $eventdata->userto = core_user::get_user($eventdata->userto->id);
+        }
+
+        $usertoisrealuser = (core_user::is_real_user($eventdata->userto->id) != false);
+        // If recipient is internal user (noreply user), and emailstop is set then don't send any msg.
+        if (!$usertoisrealuser && !empty($eventdata->userto->emailstop)) {
+            debugging('Attempt to send msg to internal (noreply) user', DEBUG_NORMAL);
+            return false;
+        }
+
+        if (!$conversationid = \core_message\api::get_conversation_between_users([$eventdata->userfrom->id,
+                                                                                  $eventdata->userto->id])) {
+            $conversation = \core_message\api::create_conversation(
+                \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
+                [
+                    $eventdata->userfrom->id,
+                    $eventdata->userto->id
+                ]
+            );
+        }
+        // We either have found a conversation, or created one.
+        $conversationid = $conversationid ? $conversationid : $conversation->id;
+        $eventdata->convid = $conversationid;
+    }
+
+    // This is a message directed to a conversation, not a specific user as was the way in legacy messaging.
+    // The above code has adapted the legacy messages into conversation messages.
+    // We must call send_message_to_conversation(), which handles per-member processor iteration and triggers
+    // a per-conversation event.
+    // All eventdata for messages should now have a convid, as we fixed this above.
+    if (!$eventdata->notification) {
+
         // Only one message will be saved to the DB.
         $conversationid = $eventdata->convid;
         $table = 'messages';
@@ -122,6 +176,11 @@ function message_send(\core\message\message $eventdata) {
                 $messageid = $DB->insert_record($table, $tabledata);
                 $message = $DB->get_record($table, array('id' => $messageid));
 
+                // Add the useridto attribute for BC.
+                if (isset($eventdata->userto)) {
+                    $message->useridto = $eventdata->userto->id;
+                }
+
                 // Mark the message as read for each of the other users.
                 $sql = "SELECT u.*
                   FROM {message_conversation_members} mcm
@@ -154,23 +213,14 @@ function message_send(\core\message\message $eventdata) {
         return \core\message\manager::send_message_to_conversation($eventdata, $tabledata);
     }
 
-    // Notifications and legacy messaging code:
-    // Most of the next steps are shared by both the legacy message code (those being sent to a single 'userto', not to a
-    // conversation), and for notifications. Any message-specific or notification-specific steps are clearly marked.
+    // Else the message is a notification.
     if (!is_object($eventdata->userto)) {
         $eventdata->userto = core_user::get_user($eventdata->userto);
     }
-    if (!is_object($eventdata->userfrom)) {
-        $eventdata->userfrom = core_user::get_user($eventdata->userfrom);
-    }
     if (!$eventdata->userto) {
         debugging('Attempt to send msg to unknown user', DEBUG_NORMAL);
         return false;
     }
-    if (!$eventdata->userfrom) {
-        debugging('Attempt to send msg from unknown user', DEBUG_NORMAL);
-        return false;
-    }
 
     // If the provider's component is disabled or the user can't receive messages from it, don't send the message.
     $isproviderallowed = false;
@@ -216,58 +266,30 @@ function message_send(\core\message\message $eventdata) {
     }
 
     // Check if we are creating a notification or message.
-    if ($eventdata->notification) {
-        $table = 'notifications';
-
-        $tabledata = new stdClass();
-        $tabledata->useridfrom = $eventdata->userfrom->id;
-        $tabledata->useridto = $eventdata->userto->id;
-        $tabledata->subject = $eventdata->subject;
-        $tabledata->fullmessage = $eventdata->fullmessage;
-        $tabledata->fullmessageformat = $eventdata->fullmessageformat;
-        $tabledata->fullmessagehtml = $eventdata->fullmessagehtml;
-        $tabledata->smallmessage = $eventdata->smallmessage;
-        $tabledata->eventtype = $eventdata->name;
-        $tabledata->component = $eventdata->component;
-
-        if (!empty($eventdata->contexturl)) {
-            $tabledata->contexturl = (string)$eventdata->contexturl;
-        } else {
-            $tabledata->contexturl = null;
-        }
-
-        if (!empty($eventdata->contexturlname)) {
-            $tabledata->contexturlname = (string)$eventdata->contexturlname;
-        } else {
-            $tabledata->contexturlname = null;
-        }
+    $table = 'notifications';
+
+    $tabledata = new stdClass();
+    $tabledata->useridfrom = $eventdata->userfrom->id;
+    $tabledata->useridto = $eventdata->userto->id;
+    $tabledata->subject = $eventdata->subject;
+    $tabledata->fullmessage = $eventdata->fullmessage;
+    $tabledata->fullmessageformat = $eventdata->fullmessageformat;
+    $tabledata->fullmessagehtml = $eventdata->fullmessagehtml;
+    $tabledata->smallmessage = $eventdata->smallmessage;
+    $tabledata->eventtype = $eventdata->name;
+    $tabledata->component = $eventdata->component;
+    $tabledata->timecreated = time();
+    if (!empty($eventdata->contexturl)) {
+        $tabledata->contexturl = (string)$eventdata->contexturl;
     } else {
-        $table = 'messages';
-
-        if (!$conversationid = \core_message\api::get_conversation_between_users([$eventdata->userfrom->id,
-                $eventdata->userto->id])) {
-            $conversation = \core_message\api::create_conversation(
-                \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
-                [
-                    $eventdata->userfrom->id,
-                    $eventdata->userto->id
-                ]
-            );
-            $conversationid = $conversation->id;
-        }
-
-        $tabledata = new stdClass();
-        $tabledata->courseid = $eventdata->courseid;
-        $tabledata->useridfrom = $eventdata->userfrom->id;
-        $tabledata->conversationid = $conversationid;
-        $tabledata->subject = $eventdata->subject;
-        $tabledata->fullmessage = $eventdata->fullmessage;
-        $tabledata->fullmessageformat = $eventdata->fullmessageformat;
-        $tabledata->fullmessagehtml = $eventdata->fullmessagehtml;
-        $tabledata->smallmessage = $eventdata->smallmessage;
+        $tabledata->contexturl = null;
     }
 
-    $tabledata->timecreated = time();
+    if (!empty($eventdata->contexturlname)) {
+        $tabledata->contexturlname = (string)$eventdata->contexturlname;
+    } else {
+        $tabledata->contexturlname = null;
+    }
 
     if (PHPUNIT_TEST and class_exists('phpunit_util')) {
         // Add some more tests to make sure the normal code can actually work.
@@ -293,12 +315,8 @@ function message_send(\core\message\message $eventdata) {
             // Add the useridto attribute for BC.
             $message->useridto = $eventdata->userto->id;
 
-            // Mark the message/notification as read.
-            if ($eventdata->notification) {
-                \core_message\api::mark_notification_as_read($message);
-            } else {
-                \core_message\api::mark_message_as_read($eventdata->userto->id, $message);
-            }
+            // Mark the notification as read.
+            \core_message\api::mark_notification_as_read($message);
 
             // Unit tests need this detail.
             $message->notification = $eventdata->notification;
@@ -308,18 +326,7 @@ function message_send(\core\message\message $eventdata) {
     }
 
     // Fetch enabled processors.
-    // If we are dealing with a message some processors may want to handle it regardless of user and site settings.
-    if (!$eventdata->notification) {
-        $processors = array_filter(get_message_processors(false), function($processor) {
-            if ($processor->object->force_process_messages()) {
-                return true;
-            }
-
-            return ($processor->enabled && $processor->configured);
-        });
-    } else {
-        $processors = get_message_processors(true);
-    }
+    $processors = get_message_processors(true);
 
     // Preset variables
     $processorlist = array();
@@ -352,9 +359,7 @@ function message_send(\core\message\message $eventdata) {
         }
 
         // Populate the list of processors we will be using
-        if (!$eventdata->notification && $processor->object->force_process_messages()) {
-            $processorlist[] = $processor->name;
-        } else if ($permitted == 'forced' && $userisconfigured) {
+        if ($permitted == 'forced' && $userisconfigured) {
             // An admin is forcing users to use this message processor. Use this processor unconditionally.
             $processorlist[] = $processor->name;
         } else if ($permitted == 'permitted' && $userisconfigured && !$eventdata->userto->emailstop) {
@@ -373,15 +378,6 @@ function message_send(\core\message\message $eventdata) {
         }
     }
 
-    // We have either created or derived a conversationid, which we can use to
-    // update the 'message_time_last_message_between_users' cache.
-    if (!empty($tabledata->conversationid)) {
-        // Cache the timecreated value of the last message in this conversation.
-        $cache = cache::make('core', 'message_time_last_message_between_users');
-        $key = \core_message\helper::get_last_message_time_created_cache_key($tabledata->conversationid);
-        $cache->set($key, $tabledata->timecreated);
-    }
-
     // Store unread message just in case we get a fatal error any time later.
     $tabledata->id = $DB->insert_record($table, $tabledata);
     $eventdata->savedmessageid = $tabledata->id;
index 3f1d3de..d2e0265 100644 (file)
@@ -171,10 +171,10 @@ class core_message_testcase extends advanced_testcase {
         $this->assertSame(true, $recordexists);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);
-        $this->assertSame($message->subject, $email->subject);
+        $this->assertSame(get_string('unreadnewmessage', 'message', fullname($user1)), $email->subject);
         $this->assertNotEmpty($email->header);
         $this->assertNotEmpty($email->body);
-        $this->assertRegExp('/test message body test/', $email->body);
+        $this->assertRegExp('/test message body.*test/s', $email->body);
         $sink->clear();
 
         // Test that event fired includes the courseid.
@@ -211,7 +211,7 @@ class core_message_testcase extends advanced_testcase {
         $this->assertSame(true, $recordexists);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);
-        $this->assertSame($message->subject, $email->subject);
+        $this->assertSame(get_string('unreadnewmessage', 'message', fullname($user1)), $email->subject);
         $this->assertNotEmpty($email->header);
         $this->assertNotEmpty($email->body);
         $this->assertNotRegExp('/test message body test/', $email->body);
index 0817631..226de53 100644 (file)
@@ -65,7 +65,7 @@ class core_messagelib_testcase extends advanced_testcase {
         message_send($message);
         $emails = $sink->get_messages();
         $email = reset($emails);
-        $this->assertEquals($email->subject, 'message subject 1');
+        $this->assertEquals(get_string('unreadnewmessage', 'message', fullname(get_admin())), $email->subject);
     }
     public function test_message_get_providers_for_user() {
         global $CFG, $DB;
@@ -548,7 +548,7 @@ class core_messagelib_testcase extends advanced_testcase {
         $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);
-        $this->assertSame($message->subject, $email->subject);
+        $this->assertSame(get_string('unreadnewmessage', 'message', fullname($user1)), $email->subject);
         $this->assertNotEmpty($email->header);
         $this->assertNotEmpty($email->body);
         $sink->clear();
@@ -581,7 +581,7 @@ class core_messagelib_testcase extends advanced_testcase {
         $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);
-        $this->assertSame($message->subject, $email->subject);
+        $this->assertSame(get_string('unreadnewmessage', 'message', fullname($user1)), $email->subject);
         $this->assertNotEmpty($email->header);
         $this->assertNotEmpty($email->body);
         $sink->clear();
index fe8f099..75acfb2 100644 (file)
@@ -1471,6 +1471,62 @@ class api {
         }
     }
 
+    /**
+     * Send a message from a user to a conversation.
+     *
+     * This method will create the basic eventdata and delegate to message creation to message_send.
+     * The message_send() method is responsible for event data that is specific to each recipient.
+     *
+     * @param int $userid the sender id.
+     * @param int $conversationid the conversation id.
+     * @param string $message the message to send.
+     * @param int $format the format of the message to send.
+     * @return \stdClass the message created.
+     * @throws \coding_exception
+     * @throws \moodle_exception if the user is not permitted to send a message to the conversation.
+     */
+    public static function send_message_to_conversation(int $userid, int $conversationid, string $message,
+                                                        int $format) : \stdClass {
+        global $DB;
+
+        if (!self::can_send_message_to_conversation($userid, $conversationid)) {
+            throw new \moodle_exception("User $userid cannot send a message to conversation $conversationid");
+        }
+
+        $eventdata = new \core\message\message();
+        $eventdata->courseid         = 1;
+        $eventdata->component        = 'moodle';
+        $eventdata->name             = 'instantmessage';
+        $eventdata->userfrom         = $userid;
+        $eventdata->convid           = $conversationid;
+
+        if ($format == FORMAT_HTML) {
+            $eventdata->fullmessagehtml  = $message;
+            // Some message processors may revert to sending plain text even if html is supplied,
+            // so we keep both plain and html versions if we're intending to send html.
+            $eventdata->fullmessage = html_to_text($eventdata->fullmessagehtml);
+        } else {
+            $eventdata->fullmessage      = $message;
+            $eventdata->fullmessagehtml  = '';
+        }
+
+        $eventdata->fullmessageformat = $format;
+        $eventdata->smallmessage = $message; // Store the message unfiltered. Clean up on output.
+
+        $eventdata->timecreated     = time();
+        $eventdata->notification    = 0;
+        $messageid = message_send($eventdata);
+
+        $messagerecord = $DB->get_record('messages', ['id' => $messageid], 'id, useridfrom, fullmessage, timecreated');
+        $message = (object) [
+            'id' => $messagerecord->id,
+            'useridfrom' => $messagerecord->useridfrom,
+            'text' => $messagerecord->fullmessage,
+            'timecreated' => $messagerecord->timecreated
+        ];
+        return $message;
+    }
+
     /**
      * Get the messaging preference for a user.
      * If the user has not any messaging privacy preference:
index 2067dc4..c1d5a94 100644 (file)
@@ -4946,6 +4946,146 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->assertEquals($user3->id, $request2->requesteduserid);
     }
 
+    /**
+     * Test verifying that messages can be sent to existing individual conversations.
+     */
+    public function test_send_message_to_conversation_individual_conversation() {
+        // Get a bunch of conversations, some group, some individual and in different states.
+        list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3,
+            $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
+
+        // Enrol the users into the same course so the privacy checks will pass using default (contact+course members) setting.
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course->id);
+
+        // Redirect messages.
+        // This marks messages as read, but we can still observe and verify the number of conversation recipients,
+        // based on the message_viewed events generated as part of marking the message as read for each user.
+        $this->preventResetByRollback();
+        $sink = $this->redirectMessages();
+
+        // Send a message to an individual conversation.
+        $sink = $this->redirectEvents();
+        $message1 = \core_message\api::send_message_to_conversation($user1->id, $ic1->id, 'this is a message', FORMAT_MOODLE);
+        $events = $sink->get_events();
+
+        // Verify the message returned.
+        $this->assertInstanceOf(\stdClass::class, $message1);
+        $this->assertObjectHasAttribute('id', $message1);
+        $this->assertAttributeEquals($user1->id, 'useridfrom', $message1);
+        $this->assertAttributeEquals('this is a message', 'text', $message1);
+        $this->assertObjectHasAttribute('timecreated', $message1);
+
+        // Verify events. Note: the event is a message read event because of an if (PHPUNIT) conditional within message_send(),
+        // however, we can still determine the number and ids of any recipients this way.
+        $this->assertCount(1, $events);
+        $userids = array_column($events, 'userid');
+        $this->assertNotContains($user1->id, $userids);
+        $this->assertContains($user2->id, $userids);
+    }
+
+    /**
+     * Test verifying that messages can be sent to existing group conversations.
+     */
+    public function test_send_message_to_conversation_group_conversation() {
+        // Get a bunch of conversations, some group, some individual and in different states.
+        list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3,
+            $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
+
+        // Enrol the users into the same course so the privacy checks will pass using default (contact+course members) setting.
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course->id);
+
+        // Redirect messages.
+        // This marks messages as read, but we can still observe and verify the number of conversation recipients,
+        // based on the message_viewed events generated as part of marking the message as read for each user.
+        $this->preventResetByRollback();
+        $sink = $this->redirectMessages();
+
+        // Send a message to a group conversation.
+        $sink = $this->redirectEvents();
+        $message1 = \core_message\api::send_message_to_conversation($user1->id, $gc2->id, 'message to the group', FORMAT_MOODLE);
+        $events = $sink->get_events();
+
+        // Verify the message returned.
+        $this->assertInstanceOf(\stdClass::class, $message1);
+        $this->assertObjectHasAttribute('id', $message1);
+        $this->assertAttributeEquals($user1->id, 'useridfrom', $message1);
+        $this->assertAttributeEquals('message to the group', 'text', $message1);
+        $this->assertObjectHasAttribute('timecreated', $message1);
+
+        // Verify events. Note: the event is a message read event because of an if (PHPUNIT) conditional within message_send(),
+        // however, we can still determine the number and ids of any recipients this way.
+        $this->assertCount(2, $events);
+        $userids = array_column($events, 'userid');
+        $this->assertNotContains($user1->id, $userids);
+        $this->assertContains($user3->id, $userids);
+        $this->assertContains($user4->id, $userids);
+    }
+
+    /**
+     * Test verifying that messages cannot be sent to conversations that don't exist.
+     */
+    public function test_send_message_to_conversation_non_existent_conversation() {
+        // Get a bunch of conversations, some group, some individual and in different states.
+        list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3,
+            $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
+
+        $this->expectException(\moodle_exception::class);
+        \core_message\api::send_message_to_conversation($user1->id, 0, 'test', FORMAT_MOODLE);
+    }
+
+    /**
+     * Test verifying that messages cannot be sent to conversations by users who are not members.
+     */
+    public function test_send_message_to_conversation_non_member() {
+        // Get a bunch of conversations, some group, some individual and in different states.
+        list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3,
+            $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
+
+        // Enrol the users into the same course so the privacy checks will pass using default (contact+course members) setting.
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course->id);
+
+        $this->expectException(\moodle_exception::class);
+        \core_message\api::send_message_to_conversation($user3->id, $ic1->id, 'test', FORMAT_MOODLE);
+    }
+
+    /**
+     * Test verifying that messages cannot be sent to conversations by users who are not members.
+     */
+    public function test_send_message_to_conversation_blocked_user() {
+        // Get a bunch of conversations, some group, some individual and in different states.
+        list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3,
+            $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
+
+        // Enrol the users into the same course so the privacy checks will pass using default (contact+course members) setting.
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course->id);
+
+        // User 1 blocks user 2.
+        \core_message\api::block_user($user1->id, $user2->id);
+
+        // Verify that a message can be sent to any group conversation in which user1 and user2 are members.
+        $this->assertNotEmpty(\core_message\api::send_message_to_conversation($user1->id, $gc2->id, 'Hey guys', FORMAT_PLAIN));
+
+        // User 2 cannot send a message to the conversation with user 1.
+        $this->expectException(\moodle_exception::class);
+        \core_message\api::send_message_to_conversation($user2->id, $ic1->id, 'test', FORMAT_MOODLE);
+    }
+
     /**
      * Comparison function for sorting contacts.
      *