Merge branch 'MDL-63632-master' of git://github.com/andrewnicols/moodle
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Thu, 18 Oct 2018 00:38:57 +0000 (02:38 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Thu, 18 Oct 2018 00:38:57 +0000 (02:38 +0200)
55 files changed:
backup/moodle2/backup_stepslib.php
backup/moodle2/restore_stepslib.php
config-dist.php
enrol/externallib.php
enrol/tests/externallib_test.php
enrol/upgrade.txt
grade/report/history/classes/output/tablelog.php
grade/report/user/lib.php
lang/en/admin.php
lang/en/deprecated.txt
lang/en/message.php
lib/amd/build/templates.min.js
lib/amd/src/templates.js
lib/classes/event/message_deleted.php
lib/db/services.php
lib/db/upgrade.php
lib/filelib.php
lib/grade/constants.php
lib/grade/grade_grade.php
lib/grade/grade_item.php
lib/grade/grade_object.php
lib/gradelib.php
lib/upgrade.txt
message/amd/build/message_preferences.min.js
message/amd/src/message_preferences.js
message/classes/api.php
message/externallib.php
message/lib.php
message/output/popup/tests/behat/message_popover_unread.feature
message/renderer.php
message/templates/message_preferences.mustache
message/tests/api_test.php
message/tests/behat/delete_all_messages.feature
message/tests/behat/delete_messages.feature
message/tests/behat/reply_message.feature
message/tests/behat/search_messages.feature
message/tests/behat/view_messages.feature
message/tests/events_test.php
message/tests/externallib_test.php
message/tests/privacy_provider_test.php
message/upgrade.txt
mod/assign/feedback/comments/backup/moodle2/backup_assignfeedback_comments_subplugin.class.php
mod/assign/feedback/comments/backup/moodle2/restore_assignfeedback_comments_subplugin.class.php
mod/assign/feedback/comments/classes/privacy/provider.php
mod/assign/feedback/comments/lang/en/assignfeedback_comments.php
mod/assign/feedback/comments/lib.php [new file with mode: 0644]
mod/assign/feedback/comments/locallib.php
mod/assign/feedback/comments/tests/privacy_test.php
mod/assign/feedbackplugin.php
mod/assign/locallib.php
mod/assign/tests/locallib_test.php
mod/assign/upgrade.txt
user/profile/definelib.php
user/tests/behat/delete_users.feature
version.php

index b90490a..ef99fa6 100644 (file)
@@ -2418,6 +2418,9 @@ class backup_activity_grades_structure_step extends backup_structure_step {
     }
 
     protected function define_structure() {
+        global $CFG;
+
+        require_once($CFG->libdir . '/grade/constants.php');
 
         // To know if we are including userinfo
         $userinfo = $this->get_setting_value('userinfo');
@@ -2474,6 +2477,7 @@ class backup_activity_grades_structure_step extends backup_structure_step {
         // This only happens if we are including user info
         if ($userinfo) {
             $grade->set_source_table('grade_grades', array('itemid' => backup::VAR_PARENTID));
+            $grade->annotate_files(GRADE_FILE_COMPONENT, GRADE_FEEDBACK_FILEAREA, 'id');
         }
 
         $letter->set_source_table('grade_letters', array('contextid' => backup::VAR_CONTEXTID));
@@ -2510,6 +2514,9 @@ class backup_activity_grade_history_structure_step extends backup_structure_step
     }
 
     protected function define_structure() {
+        global $CFG;
+
+        require_once($CFG->libdir . '/grade/constants.php');
 
         // Settings to use.
         $userinfo = $this->get_setting_value('userinfo');
@@ -2537,6 +2544,7 @@ class backup_activity_grade_history_structure_step extends backup_structure_step
                                      JOIN {backup_ids_temp} bi ON ggh.itemid = bi.itemid
                                     WHERE bi.backupid = ?
                                       AND bi.itemname = 'grade_item'", array(backup::VAR_BACKUPID));
+            $grade->annotate_files(GRADE_FILE_COMPONENT, GRADE_HISTORY_FEEDBACK_FILEAREA, 'id');
         }
 
         // Annotations.
index 250aeb2..1e78ee3 100644 (file)
@@ -3705,6 +3705,10 @@ class restore_activity_grades_structure_step extends restore_structure_step {
     }
 
     protected function process_grade_grade($data) {
+        global $CFG;
+
+        require_once($CFG->libdir . '/grade/constants.php');
+
         $data = (object)($data);
         $olduserid = $data->userid;
         $oldid = $data->id;
@@ -3719,7 +3723,16 @@ class restore_activity_grades_structure_step extends restore_structure_step {
 
             $grade = new grade_grade($data, false);
             $grade->insert('restore');
-            $this->set_mapping('grade_grades', $oldid, $grade->id);
+
+            $this->set_mapping('grade_grades', $oldid, $grade->id, true);
+
+            $this->add_related_files(
+                GRADE_FILE_COMPONENT,
+                GRADE_FEEDBACK_FILEAREA,
+                'grade_grades',
+                null,
+                $oldid
+            );
         } else {
             debugging("Mapped user id not found for user id '{$olduserid}', grade item id '{$data->itemid}'");
         }
@@ -3794,9 +3807,12 @@ class restore_activity_grade_history_structure_step extends restore_structure_st
     }
 
     protected function process_grade_grade($data) {
-        global $DB;
+        global $CFG, $DB;
+
+        require_once($CFG->libdir . '/grade/constants.php');
 
         $data = (object) $data;
+        $oldhistoryid = $data->id;
         $olduserid = $data->userid;
         unset($data->id);
 
@@ -3807,13 +3823,23 @@ class restore_activity_grade_history_structure_step extends restore_structure_st
             $data->oldid = $this->get_mappingid('grade_grades', $data->oldid);
             $data->usermodified = $this->get_mappingid('user', $data->usermodified, null);
             $data->rawscaleid = $this->get_mappingid('scale', $data->rawscaleid);
-            $DB->insert_record('grade_grades_history', $data);
+
+            $newhistoryid = $DB->insert_record('grade_grades_history', $data);
+
+            $this->set_mapping('grade_grades_history', $oldhistoryid, $newhistoryid, true);
+
+            $this->add_related_files(
+                GRADE_FILE_COMPONENT,
+                GRADE_HISTORY_FEEDBACK_FILEAREA,
+                'grade_grades_history',
+                null,
+                $oldhistoryid
+            );
         } else {
             $message = "Mapped user id not found for user id '{$olduserid}', grade item id '{$data->itemid}'";
             $this->log($message, backup::LOG_DEBUG);
         }
     }
-
 }
 
 /**
index f2437cb..9b9e371 100644 (file)
@@ -589,6 +589,11 @@ $CFG->admin = 'admin';
 //
 //      $CFG->pdfexportfont = 'freesans';
 //
+// Use the following flag to enable messagingallusers and set the default preference
+// value for existing users to allow them to be contacted by other site users.
+//
+//      $CFG->keepmessagingallusersenabled = true;
+//
 //=========================================================================
 // 7. SETTINGS FOR DEVELOPMENT SERVERS - not intended for production use!!!
 //=========================================================================
index 55b074c..6b06aa4 100644 (file)
@@ -295,15 +295,18 @@ class core_enrol_external extends external_api {
         global $CFG, $USER, $DB;
 
         require_once($CFG->dirroot . '/course/lib.php');
+        require_once($CFG->libdir . '/completionlib.php');
 
         // Do basic automatic PARAM checks on incoming data, using params description
         // If any problems are found then exceptions are thrown with helpful error messages
         $params = self::validate_parameters(self::get_users_courses_parameters(), array('userid'=>$userid));
 
-        $courses = enrol_get_users_courses($params['userid'], true, 'id, shortname, fullname, idnumber, visible,
-                   summary, summaryformat, format, showgrades, lang, enablecompletion, category, startdate, enddate');
+        $courses = enrol_get_users_courses($params['userid'], true, '*');
         $result = array();
 
+        // Get user data including last access to courses.
+        $user = get_complete_user_data('id', $userid);
+
         foreach ($courses as $course) {
             $context = context_course::instance($course->id, IGNORE_MISSING);
             try {
@@ -313,7 +316,8 @@ class core_enrol_external extends external_api {
                 continue;
             }
 
-            if ($userid != $USER->id and !course_can_view_participants($context)) {
+            $sameuser = $USER->id == $userid;
+            if (!$sameuser and !course_can_view_participants($context)) {
                 // we need capability to view participants
                 continue;
             }
@@ -322,20 +326,58 @@ class core_enrol_external extends external_api {
             $enrolledsql = "SELECT COUNT('x') FROM ($enrolledsqlselect) enrolleduserids";
             $enrolledusercount = $DB->count_records_sql($enrolledsql, $enrolledparams);
 
+            $displayname = external_format_string(get_course_display_name_for_list($course), $context->id);
             list($course->summary, $course->summaryformat) =
                 external_format_text($course->summary, $course->summaryformat, $context->id, 'course', 'summary', null);
             $course->fullname = external_format_string($course->fullname, $context->id);
             $course->shortname = external_format_string($course->shortname, $context->id);
 
             $progress = null;
-            if ($course->enablecompletion) {
-                $progress = \core_completion\progress::get_course_progress_percentage($course);
+            $completed = null;
+
+            // Return only private information if the user should be able to see it.
+            if ($sameuser || completion_can_view_data($userid, $course)) {
+                if ($course->enablecompletion) {
+                    $completion = new completion_info($course);
+                    $completed = $completion->is_course_complete($userid);
+                    $progress = \core_completion\progress::get_course_progress_percentage($course, $userid);
+                }
+            }
+
+            $lastaccess = null;
+            // Check if last access is a hidden field.
+            $hiddenfields = array_flip(explode(',', $CFG->hiddenuserfields));
+            $canviewlastaccess = $sameuser || !isset($hiddenfields['lastaccess']);
+            if (!$canviewlastaccess) {
+                $canviewlastaccess = has_capability('moodle/course:viewhiddenuserfields', $context);
+            }
+
+            if ($canviewlastaccess && isset($user->lastcourseaccess[$course->id])) {
+                $lastaccess = $user->lastcourseaccess[$course->id];
+            }
+
+            // Retrieve course overview used files.
+            $courselist = new core_course_list_element($course);
+            $overviewfiles = array();
+            foreach ($courselist->get_course_overviewfiles() as $file) {
+                $fileurl = moodle_url::make_webservice_pluginfile_url($file->get_contextid(), $file->get_component(),
+                                                                        $file->get_filearea(), null, $file->get_filepath(),
+                                                                        $file->get_filename())->out(false);
+                $overviewfiles[] = array(
+                    'filename' => $file->get_filename(),
+                    'fileurl' => $fileurl,
+                    'filesize' => $file->get_filesize(),
+                    'filepath' => $file->get_filepath(),
+                    'mimetype' => $file->get_mimetype(),
+                    'timemodified' => $file->get_timemodified(),
+                );
             }
 
             $result[] = array(
                 'id' => $course->id,
                 'shortname' => $course->shortname,
                 'fullname' => $course->fullname,
+                'displayname' => $displayname,
                 'idnumber' => $course->idnumber,
                 'visible' => $course->visible,
                 'enrolledusercount' => $enrolledusercount,
@@ -347,8 +389,12 @@ class core_enrol_external extends external_api {
                 'enablecompletion' => $course->enablecompletion,
                 'category' => $course->category,
                 'progress' => $progress,
+                'completed' => $completed,
                 'startdate' => $course->startdate,
                 'enddate' => $course->enddate,
+                'marker' => $course->marker,
+                'lastaccess' => $lastaccess,
+                'overviewfiles' => $overviewfiles,
             );
         }
 
@@ -367,6 +413,7 @@ class core_enrol_external extends external_api {
                     'id'        => new external_value(PARAM_INT, 'id of course'),
                     'shortname' => new external_value(PARAM_RAW, 'short name of course'),
                     'fullname'  => new external_value(PARAM_RAW, 'long name of course'),
+                    'displayname' => new external_value(PARAM_TEXT, 'course display name for lists.', VALUE_OPTIONAL),
                     'enrolledusercount' => new external_value(PARAM_INT, 'Number of enrolled users in this course'),
                     'idnumber'  => new external_value(PARAM_RAW, 'id number of course'),
                     'visible'   => new external_value(PARAM_INT, '1 means visible, 0 means hidden course'),
@@ -379,8 +426,12 @@ class core_enrol_external extends external_api {
                                                                 VALUE_OPTIONAL),
                     'category' => new external_value(PARAM_INT, 'course category id', VALUE_OPTIONAL),
                     'progress' => new external_value(PARAM_FLOAT, 'Progress percentage', VALUE_OPTIONAL),
+                    'completed' => new external_value(PARAM_BOOL, 'Whether the course is completed.', VALUE_OPTIONAL),
                     'startdate' => new external_value(PARAM_INT, 'Timestamp when the course start', VALUE_OPTIONAL),
                     'enddate' => new external_value(PARAM_INT, 'Timestamp when the course end', VALUE_OPTIONAL),
+                    'marker' => new external_value(PARAM_INT, 'Course section marker.', VALUE_OPTIONAL),
+                    'lastaccess' => new external_value(PARAM_INT, 'Last access to the course (timestamp).', VALUE_OPTIONAL),
+                    'overviewfiles' => new external_files('Overview files attached to this course.', VALUE_OPTIONAL),
                 )
             )
         );
index 22c3c5c..90029e2 100644 (file)
@@ -359,9 +359,10 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
      * Test get_users_courses
      */
     public function test_get_users_courses() {
-        global $USER;
+        global $CFG, $DB;
 
         $this->resetAfterTest(true);
+        $CFG->enablecompletion = 1;
 
         $timenow = time();
         $coursedata1 = array(
@@ -373,7 +374,8 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
             'enablecompletion' => true,
             'showgrades'       => true,
             'startdate'        => $timenow,
-            'enddate'          => $timenow + WEEKSECS
+            'enddate'          => $timenow + WEEKSECS,
+            'marker'           => 1
         );
 
         $coursedata2 = array(
@@ -383,21 +385,32 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
         $course1 = self::getDataGenerator()->create_course($coursedata1);
         $course2 = self::getDataGenerator()->create_course($coursedata2);
         $courses = array($course1, $course2);
+        $contexts = array ($course1->id => context_course::instance($course1->id),
+            $course2->id => context_course::instance($course2->id));
 
-        // Enrol $USER in the courses.
-        // We use the manual plugin.
-        $roleid = null;
-        $contexts = array();
-        foreach ($courses as $course) {
-            $contexts[$course->id] = context_course::instance($course->id);
-            $roleid = $this->assignUserCapability('moodle/course:viewparticipants',
-                    $contexts[$course->id]->id, $roleid);
+        $student = $this->getDataGenerator()->create_user();
+        $otherstudent = $this->getDataGenerator()->create_user();
+        $studentroleid = $DB->get_field('role', 'id', array('shortname' => 'student'));
+        $this->getDataGenerator()->enrol_user($student->id, $course1->id, $studentroleid);
+        $this->getDataGenerator()->enrol_user($otherstudent->id, $course1->id, $studentroleid);
+        $this->getDataGenerator()->enrol_user($student->id, $course2->id, $studentroleid);
 
-            $this->getDataGenerator()->enrol_user($USER->id, $course->id, $roleid, 'manual');
-        }
+        // Force last access.
+        $timenow = time();
+        $lastaccess = array(
+            'userid' => $student->id,
+            'courseid' => $course1->id,
+            'timeaccess' => $timenow
+        );
+        $DB->insert_record('user_lastaccess', $lastaccess);
+
+        // Force completion.
+        $ccompletion = new completion_completion(array('course' => $course1->id, 'userid' => $student->id));
+        $ccompletion->mark_complete();
 
+        $this->setUser($student);
         // Call the external function.
-        $enrolledincourses = core_enrol_external::get_users_courses($USER->id);
+        $enrolledincourses = core_enrol_external::get_users_courses($student->id);
 
         // We need to execute the return values cleaning process to simulate the web service server.
         $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses);
@@ -418,11 +431,54 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
                 foreach ($coursedata1 as $fieldname => $value) {
                     $this->assertEquals($courseenrol[$fieldname], $course1->$fieldname);
                 }
+                // Text extra fields.
+                $this->assertEquals($course1->fullname, $courseenrol['displayname']);
+                $this->assertEquals([], $courseenrol['overviewfiles']);
+                $this->assertEquals($timenow, $courseenrol['lastaccess']);
+                $this->assertEquals(100.0, $courseenrol['progress']);
+                $this->assertEquals(true, $courseenrol['completed']);
             } else {
                 // Check language pack. Should be empty since an incorrect one was used when creating the course.
                 $this->assertEmpty($courseenrol['lang']);
+                $this->assertEquals($course2->fullname, $courseenrol['displayname']);
+                $this->assertEquals([], $courseenrol['overviewfiles']);
+                $this->assertEquals(0, $courseenrol['lastaccess']);
+                $this->assertEquals(0, $courseenrol['progress']);
+                $this->assertEquals(false, $courseenrol['completed']);
             }
         }
+
+        // Now check that admin users can see all the info.
+        $this->setAdminUser();
+
+        $enrolledincourses = core_enrol_external::get_users_courses($student->id);
+        $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses);
+        $this->assertEquals(2, count($enrolledincourses));
+        foreach ($enrolledincourses as $courseenrol) {
+            if ($courseenrol['id'] == $course1->id) {
+                $this->assertEquals($timenow, $courseenrol['lastaccess']);
+                $this->assertEquals(100.0, $courseenrol['progress']);
+            } else {
+                $this->assertEquals(0, $courseenrol['progress']);
+            }
+        }
+
+        // Check other users can't see private info.
+        $this->setUser($otherstudent);
+
+        $enrolledincourses = core_enrol_external::get_users_courses($student->id);
+        $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses);
+        $this->assertEquals(1, count($enrolledincourses));
+
+        $this->assertEquals($timenow, $enrolledincourses[0]['lastaccess']); // I can see this, not hidden.
+        $this->assertEquals(null, $enrolledincourses[0]['progress']);   // I can't see this, private.
+
+        // Change some global profile visibility fields.
+        $CFG->hiddenuserfields = 'lastaccess';
+        $enrolledincourses = core_enrol_external::get_users_courses($student->id);
+        $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses);
+
+        $this->assertEquals(0, $enrolledincourses[0]['lastaccess']); // I can't see this, hidden by global setting.
     }
 
     /**
index 09ff3ba..69d47fe 100644 (file)
@@ -1,6 +1,16 @@
 This files describes API changes in /enrol/* - plugins,
 information provided here is intended especially for developers.
 
+=== 3.6 ===
+
+* External function core_enrol_external::get_users_courses now return more information to avoid multiple queries to build the
+  user dashboard:
+  - displayname: Course display name for lists.
+  - marker: Course section active marker.
+  - completed: Whether the given user completed the course or not.
+  - lastaccess: Last time the user accessed the course.
+  - overviewfiles: Course overview files.
+
 === 3.5 ===
 
 * Default sorting in enrol_get_my_courses(), enrol_get_all_users_courses() and enrol_get_users_courses() now respects
index b6e7d1d..4d9c9fc 100644 (file)
@@ -314,7 +314,20 @@ class tablelog extends \table_sql implements \renderable {
         if ($this->is_downloading()) {
             return $history->feedback;
         } else {
-            return format_text($history->feedback, $history->feedbackformat, array('context' => $this->context));
+            // We need the activity context, not the course context.
+            $gradeitem = $this->gradeitems[$history->itemid];
+            $context = $gradeitem->get_context();
+
+            $feedback = file_rewrite_pluginfile_urls(
+                $history->feedback,
+                'pluginfile.php',
+                $context->id,
+                GRADE_FILE_COMPONENT,
+                GRADE_HISTORY_FEEDBACK_FILEAREA,
+                $history->id
+            );
+
+            return format_text($feedback, $history->feedbackformat, array('context' => $context));
         }
     }
 
index ea40100..dab9bb5 100644 (file)
@@ -685,16 +685,30 @@ class grade_report_user extends grade_report {
                     $gradeitemdata['feedback'] = '';
                     $gradeitemdata['feedbackformat'] = $grade_grade->feedbackformat;
 
+                    if ($grade_grade->feedback) {
+                        $grade_grade->feedback = file_rewrite_pluginfile_urls(
+                            $grade_grade->feedback,
+                            'pluginfile.php',
+                            $grade_grade->get_context()->id,
+                            GRADE_FILE_COMPONENT,
+                            GRADE_FEEDBACK_FILEAREA,
+                            $grade_grade->id
+                        );
+                    }
+
                     if ($grade_grade->overridden > 0 AND ($type == 'categoryitem' OR $type == 'courseitem')) {
                     $data['feedback']['class'] = $classfeedback.' feedbacktext';
-                        $data['feedback']['content'] = get_string('overridden', 'grades').': ' . format_text($grade_grade->feedback, $grade_grade->feedbackformat);
+                        $data['feedback']['content'] = get_string('overridden', 'grades').': ' .
+                            format_text($grade_grade->feedback, $grade_grade->feedbackformat,
+                                ['context' => $grade_grade->get_context()]);
                         $gradeitemdata['feedback'] = $grade_grade->feedback;
                     } else if (empty($grade_grade->feedback) or (!$this->canviewhidden and $grade_grade->is_hidden())) {
                         $data['feedback']['class'] = $classfeedback.' feedbacktext';
                         $data['feedback']['content'] = '&nbsp;';
                     } else {
                         $data['feedback']['class'] = $classfeedback.' feedbacktext';
-                        $data['feedback']['content'] = format_text($grade_grade->feedback, $grade_grade->feedbackformat);
+                        $data['feedback']['content'] = format_text($grade_grade->feedback, $grade_grade->feedbackformat,
+                            ['context' => $grade_grade->get_context()]);
                         $gradeitemdata['feedback'] = $grade_grade->feedback;
                     }
                     $data['feedback']['headers'] = "$header_cat $header_row feedback";
index ad1b1a9..dd31c0c 100644 (file)
@@ -919,6 +919,7 @@ $string['profilerequired'] = 'Is this field required?';
 $string['profileroles'] = 'Profile visible roles';
 $string['profilesforenrolledusersonly'] = 'Profiles for enrolled users only';
 $string['profileshortname'] = 'Short name (must be unique)';
+$string['profileshortnameinvalid'] = 'This short name can only contain alphanumeric characters (letters and numbers) or underscore (_).';
 $string['profileshortnamenotunique'] = 'This short name is already in use';
 $string['profilesignup'] = 'Display on signup page?';
 $string['profilespecificsettings'] = 'Specific settings';
index 873ad79..c1939ef 100644 (file)
@@ -137,3 +137,5 @@ previewhtml,core
 messagedselecteduserfailed,core
 eventmessagecontactblocked,core_message
 eventmessagecontactunblocked,core_message
+userisblockingyou,core_message
+userisblockingyounoncontact,core_message
\ No newline at end of file
index 41fb26e..562723b 100644 (file)
@@ -31,6 +31,10 @@ $string['blockcontact'] = 'Block contact';
 $string['blockedusers'] = 'Blocked users';
 $string['blocknoncontacts'] = 'Prevent non-contacts from messaging me';
 $string['canceledit'] = 'Cancel editing messages';
+$string['contactableprivacy'] = 'Accept messages from:';
+$string['contactableprivacy_onlycontacts'] = 'My contacts only';
+$string['contactableprivacy_coursemember'] = 'My contacts and anyone in my courses';
+$string['contactableprivacy_site'] = 'Anyone on the site';
 $string['contactblocked'] = 'Contact blocked';
 $string['contactrequests'] = 'Contact requests';
 $string['contacts'] = 'Contacts';
@@ -181,8 +185,7 @@ $string['unblockcontact'] = 'Unblock contact';
 $string['unknownuser'] = 'Unknown user';
 $string['unreadnotification'] = 'Unread notification: {$a}';
 $string['unreadnewmessage'] = 'New message from {$a}';
-$string['userisblockingyou'] = 'This user has blocked you from sending messages to them';
-$string['userisblockingyounoncontact'] = '{$a} only accepts messages from their contacts.';
+$string['usercantbemessaged'] = 'You can\'t message {$a} due to their message preferences. Try adding them as a contact.';
 $string['viewfullnotification'] = 'View full notification';
 $string['viewinganotherusersmessagearea'] = 'You are viewing another user\'s message area.';
 $string['viewmessageswith'] = 'View messages with {$a}';
@@ -195,3 +198,5 @@ $string['you'] = 'You:';
 $string['eventmessagecontactblocked'] = 'Message contact blocked';
 $string['eventmessagecontactunblocked'] = 'Message contact unblocked';
 $string['messagingdisabled'] = 'Messaging is disabled on this site, emails will be sent instead';
+$string['userisblockingyou'] = 'This user has blocked you from sending messages to them';
+$string['userisblockingyounoncontact'] = '{$a} only accepts messages from their contacts.';
\ No newline at end of file
index 0036edc..3418f7f 100644 (file)
Binary files a/lib/amd/build/templates.min.js and b/lib/amd/build/templates.min.js differ
index 27adb6a..b0ad1ed 100644 (file)
@@ -50,6 +50,9 @@ define(['core/mustache',
     /** @var {Promise[]} templatePromises - Cache of already loaded template promises */
     var templatePromises = {};
 
+    /** @var {Promise[]} cachePartialPromises - Cache of already loaded template partial promises */
+    var cachePartialPromises = {};
+
     /** @var {Object} iconSystem - Object extending core/iconsystem */
     var iconSystem = {};
 
@@ -648,24 +651,26 @@ define(['core/mustache',
      * @return {Promise} JQuery promise object resolved when all partials are in the cache.
      */
     Renderer.prototype.cachePartials = function(templateName) {
-        return this.getTemplate(templateName).then(function(templateSource) {
-            var i;
+        var searchKey = this.currentThemeName + '/' + templateName;
+
+        if (searchKey in cachePartialPromises) {
+            return cachePartialPromises[searchKey];
+        }
+
+        var cachePartialPromise = this.getTemplate(templateName).then(function(templateSource) {
             var partials = this.scanForPartials(templateSource);
-            var fetchThemAll = [];
-
-            for (i = 0; i < partials.length; i++) {
-                var searchKey = this.currentThemeName + '/' + partials[i];
-                if (searchKey in templatePromises) {
-                    fetchThemAll.push(templatePromises[searchKey]);
-                } else {
-                    fetchThemAll.push(this.cachePartials(partials[i]));
-                }
-            }
+            var fetchThemAll = partials.map(function(partialName) {
+                return this.cachePartials(partialName);
+            }.bind(this));
 
             return $.when.apply($, fetchThemAll).then(function() {
                 return templateSource;
             });
         }.bind(this));
+
+        cachePartialPromises[searchKey] = cachePartialPromise;
+
+        return cachePartialPromise;
     };
 
     /**
index b991853..ab38448 100644 (file)
@@ -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'.";
+        }
+
+        $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 "The user with id '$this->userid' deleted a message sent $str 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;
     }
 }
index 10bd788..f187293 100644 (file)
@@ -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,
index a6087d3..cd3dc8e 100644 (file)
@@ -2337,22 +2337,6 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2018091200.00);
     }
 
-    if ($oldversion < 2018091400.01) {
-        if (!isset($CFG->messagingallusers)) {
-            // For existing instances, $CFG->messagingallusers would be same value $CFG->messaging has.
-            if (isset($CFG->messaging)) {
-                set_config('messagingallusers', $CFG->messaging);
-            } else {
-                // When $CFG->messaging is not set, default value for $CFG->messaging should be true,
-                // so $CFG->messagingallusers value should be true as well.
-                set_config('messagingallusers', 1);
-            }
-        }
-
-        // Main savepoint reached.
-        upgrade_main_savepoint(true, 2018091400.01);
-    }
-
     if ($oldversion < 2018091700.01) {
         // Remove unused setting.
         unset_config('messaginghidereadnotifications');
@@ -2524,5 +2508,30 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2018092800.03);
     }
 
+    if ($oldversion < 2018101700.01) {
+        if (empty($CFG->keepmessagingallusersenabled)) {
+            // When it is not set, $CFG->messagingallusers should be disabled by default.
+            // When $CFG->messagingallusers = false, the default user preference is MESSAGE_PRIVACY_COURSEMEMBER
+            // (contacted by users sharing a course).
+            set_config('messagingallusers', false);
+        } else {
+            // When $CFG->keepmessagingallusersenabled is set to true, $CFG->messagingallusers is set to true.
+            set_config('messagingallusers', true);
+
+            // When $CFG->messagingallusers = true, the default user preference is MESSAGE_PRIVACY_SITE
+            // (contacted by all users site). So we need to set existing values from 0 (MESSAGE_PRIVACY_COURSEMEMBER)
+            // to 2 (MESSAGE_PRIVACY_SITE).
+            $DB->set_field(
+                'user_preferences',
+                'value',
+                \core_message\api::MESSAGE_PRIVACY_SITE,
+                array('name' => 'message_blocknoncontacts', 'value' => 0)
+            );
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2018101700.01);
+    }
+
     return true;
 }
index ded335b..b096a5b 100644 (file)
@@ -4198,6 +4198,9 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null, $offlin
 
     // ========================================================================================================================
     } else if ($component === 'grade') {
+
+        require_once($CFG->libdir . '/grade/constants.php');
+
         if (($filearea === 'outcome' or $filearea === 'scale') and $context->contextlevel == CONTEXT_SYSTEM) {
             // Global gradebook files
             if ($CFG->forcelogin) {
@@ -4213,15 +4216,35 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null, $offlin
             \core\session\manager::write_close(); // Unlock session during file serving.
             send_stored_file($file, 60*60, 0, $forcedownload, $sendfileoptions);
 
-        } else if ($filearea === 'feedback' and $context->contextlevel == CONTEXT_COURSE) {
-            //TODO: nobody implemented this yet in grade edit form!!
-            send_file_not_found();
+        } else if ($filearea == GRADE_FEEDBACK_FILEAREA || $filearea == GRADE_HISTORY_FEEDBACK_FILEAREA) {
+            if ($context->contextlevel != CONTEXT_MODULE) {
+                send_file_not_found;
+            }
 
-            if ($CFG->forcelogin || $course->id != SITEID) {
-                require_login($course);
+            require_login($course, false);
+
+            $gradeid = (int) array_shift($args);
+            $filename = array_pop($args);
+            if ($filearea == GRADE_HISTORY_FEEDBACK_FILEAREA) {
+                $grade = $DB->get_record('grade_grades_history', ['id' => $gradeid]);
+            } else {
+                $grade = $DB->get_record('grade_grades', ['id' => $gradeid]);
             }
 
-            $fullpath = "/$context->id/$component/$filearea/".implode('/', $args);
+            if (!$grade) {
+                send_file_not_found();
+            }
+
+            $iscurrentuser = $USER->id == $grade->userid;
+
+            if (!$iscurrentuser) {
+                $coursecontext = context_course::instance($course->id);
+                if (!has_capability('moodle/grade:viewall', $coursecontext)) {
+                    send_file_not_found();
+                }
+            }
+
+            $fullpath = "/$context->id/$component/$filearea/$gradeid/$filename";
 
             if (!$file = $fs->get_file_by_hash(sha1($fullpath)) or $file->is_directory()) {
                 send_file_not_found();
index 662965e..039ed3b 100644 (file)
@@ -263,3 +263,18 @@ define('GRADE_MIN_MAX_FROM_GRADE_ITEM', 1);
  * GRADE_MIN_MAX_FROM_GRADE_GRADE - Get the grade min/max from the grade grade.
  */
 define('GRADE_MIN_MAX_FROM_GRADE_GRADE', 2);
+
+/**
+ * The component to store grade files.
+ */
+define('GRADE_FILE_COMPONENT', 'grade');
+
+/**
+ * The file area to store the associated grade_grades feedback files.
+ */
+define('GRADE_FEEDBACK_FILEAREA', 'feedback');
+
+/**
+ * The file area to store the associated grade_grades_history feedback files.
+ */
+define('GRADE_HISTORY_FEEDBACK_FILEAREA', 'historyfeedback');
index f2b30af..aef9f51 100644 (file)
@@ -172,6 +172,22 @@ class grade_grade extends grade_object {
      */
     public $aggregationweight = null;
 
+    /**
+     * Feedback files to copy.
+     *
+     * Example -
+     *
+     * [
+     *     'contextid' => 1,
+     *     'component' => 'mod_xyz',
+     *     'filearea' => 'mod_xyz_feedback',
+     *     'itemid' => 2
+     * ];
+     *
+     * @var array
+     */
+    public $feedbackfiles = [];
+
     /**
      * Returns array of grades for given grade_item+users
      *
@@ -1017,13 +1033,60 @@ class grade_grade extends grade_object {
      * @return bool success
      */
     public function update($source=null) {
-        $this->rawgrade    = grade_floatval($this->rawgrade);
-        $this->finalgrade  = grade_floatval($this->finalgrade);
+        $this->rawgrade = grade_floatval($this->rawgrade);
+        $this->finalgrade = grade_floatval($this->finalgrade);
         $this->rawgrademin = grade_floatval($this->rawgrademin);
         $this->rawgrademax = grade_floatval($this->rawgrademax);
         return parent::update($source);
     }
 
+
+    /**
+     * Handles adding feedback files in the gradebook.
+     *
+     * @param int|null $historyid
+     */
+    protected function add_feedback_files(int $historyid = null) {
+        global $CFG;
+
+        // We only support feedback files for modules atm.
+        if ($this->grade_item && $this->grade_item->is_external_item()) {
+            $context = $this->get_context();
+            $this->copy_feedback_files($context, GRADE_FEEDBACK_FILEAREA, $this->id);
+
+            if (empty($CFG->disablegradehistory) && $historyid) {
+                $this->copy_feedback_files($context, GRADE_HISTORY_FEEDBACK_FILEAREA, $historyid);
+            }
+        }
+
+        return $this->id;
+    }
+
+    /**
+     * Handles updating feedback files in the gradebook.
+     *
+     * @param int|null $historyid
+     */
+    protected function update_feedback_files(int $historyid = null){
+        global $CFG;
+
+        // We only support feedback files for modules atm.
+        if ($this->grade_item && $this->grade_item->is_external_item()) {
+            $context = $this->get_context();
+
+            $fs = new file_storage();
+            $fs->delete_area_files($context->id, GRADE_FILE_COMPONENT, GRADE_FEEDBACK_FILEAREA, $this->id);
+
+            $this->copy_feedback_files($context, GRADE_FEEDBACK_FILEAREA, $this->id);
+
+            if (empty($CFG->disablegradehistory) && $historyid) {
+                $this->copy_feedback_files($context, GRADE_HISTORY_FEEDBACK_FILEAREA, $historyid);
+            }
+        }
+
+        return true;
+    }
+
     /**
      * Deletes the grade_grade instance from the database.
      *
@@ -1121,4 +1184,44 @@ class grade_grade extends grade_object {
         return array('status' => $this->get_aggregationstatus(),
                      'weight' => $this->get_aggregationweight());
     }
+
+    /**
+     * Handles copying feedback files to a specified gradebook file area.
+     *
+     * @param context $context
+     * @param string $filearea
+     * @param int $itemid
+     */
+    private function copy_feedback_files(context $context, string $filearea, int $itemid) {
+        if ($this->feedbackfiles) {
+            $filestocopycontextid = $this->feedbackfiles['contextid'];
+            $filestocopycomponent = $this->feedbackfiles['component'];
+            $filestocopyfilearea = $this->feedbackfiles['filearea'];
+            $filestocopyitemid = $this->feedbackfiles['itemid'];
+
+            $fs = new file_storage();
+            if ($filestocopy = $fs->get_area_files($filestocopycontextid, $filestocopycomponent, $filestocopyfilearea,
+                    $filestocopyitemid)) {
+                foreach ($filestocopy as $filetocopy) {
+                    $destination = [
+                        'contextid' => $context->id,
+                        'component' => GRADE_FILE_COMPONENT,
+                        'filearea' => $filearea,
+                        'itemid' => $itemid
+                    ];
+                    $fs->create_file_from_storedfile($destination, $filetocopy);
+                }
+            }
+        }
+    }
+
+    /**
+     * Determine the correct context for this grade_grade.
+     *
+     * @return context
+     */
+    public function get_context() {
+        $this->load_grade_item();
+        return $this->grade_item->get_context();
+    }
 }
index 81d8ef8..d3b12f6 100644 (file)
@@ -1864,9 +1864,19 @@ class grade_item extends grade_object {
      * @param int $dategraded A timestamp of when the student's work was graded
      * @param int $datesubmitted A timestamp of when the student's work was submitted
      * @param grade_grade $grade A grade object, useful for bulk upgrades
+     * @param array $feedbackfiles An array identifying the location of files we want to copy to the gradebook feedback area.
+     *        Example -
+     *        [
+     *            'contextid' => 1,
+     *            'component' => 'mod_xyz',
+     *            'filearea' => 'mod_xyz_feedback',
+     *            'itemid' => 2
+     *        ];
      * @return bool success
      */
-    public function update_raw_grade($userid, $rawgrade=false, $source=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE, $usermodified=null, $dategraded=null, $datesubmitted=null, $grade=null) {
+    public function update_raw_grade($userid, $rawgrade = false, $source = null, $feedback = false,
+            $feedbackformat = FORMAT_MOODLE, $usermodified = null, $dategraded = null, $datesubmitted=null,
+            $grade = null, array $feedbackfiles = []) {
         global $USER;
 
         $result = true;
@@ -1929,6 +1939,7 @@ class grade_item extends grade_object {
         if ($feedback !== false and !$grade->is_overridden()) {
             $grade->feedback       = $feedback;
             $grade->feedbackformat = $feedbackformat;
+            $grade->feedbackfiles  = $feedbackfiles;
         }
 
         // update final grade if possible
@@ -2469,4 +2480,19 @@ class grade_item extends grade_object {
             \availability_grade\callbacks::grade_item_changed($this->courseid);
         }
     }
+
+    /**
+     * Helper function to get the accurate context for this grade column.
+     *
+     * @return context
+     */
+    public function get_context() {
+        if ($this->itemtype == 'mod') {
+            $cm = get_fast_modinfo($this->courseid)->instances[$this->itemmodule][$this->iteminstance];
+            $context = \context_module::instance($cm->id);
+        } else {
+            $context = \context_course::instance($this->courseid);
+        }
+        return $context;
+    }
 }
index 654228e..33e907e 100644 (file)
@@ -252,6 +252,7 @@ abstract class grade_object {
 
         $DB->update_record($this->table, $data);
 
+        $historyid = null;
         if (empty($CFG->disablegradehistory)) {
             unset($data->timecreated);
             $data->action       = GRADE_HISTORY_UPDATE;
@@ -259,10 +260,13 @@ abstract class grade_object {
             $data->source       = $source;
             $data->timemodified = time();
             $data->loggeduser   = $USER->id;
-            $DB->insert_record($this->table.'_history', $data);
+            $historyid = $DB->insert_record($this->table.'_history', $data);
         }
 
         $this->notify_changed(false);
+
+        $this->update_feedback_files($historyid);
+
         return true;
     }
 
@@ -346,6 +350,7 @@ abstract class grade_object {
 
         $data = $this->get_record_data();
 
+        $historyid = null;
         if (empty($CFG->disablegradehistory)) {
             unset($data->timecreated);
             $data->action       = GRADE_HISTORY_INSERT;
@@ -353,10 +358,13 @@ abstract class grade_object {
             $data->source       = $source;
             $data->timemodified = time();
             $data->loggeduser   = $USER->id;
-            $DB->insert_record($this->table.'_history', $data);
+            $historyid = $DB->insert_record($this->table.'_history', $data);
         }
 
         $this->notify_changed(false);
+
+        $this->add_feedback_files($historyid);
+
         return $this->id;
     }
 
@@ -411,6 +419,22 @@ abstract class grade_object {
     protected function notify_changed($deleted) {
     }
 
+    /**
+     * Handles adding feedback files in the gradebook.
+     *
+     * @param int|null $historyid
+     */
+    protected function add_feedback_files(int $historyid = null) {
+    }
+
+    /**
+     * Handles updating feedback files in the gradebook.
+     *
+     * @param int|null $historyid
+     */
+    protected function update_feedback_files(int $historyid = null) {
+    }
+
     /**
      * Returns the current hidden state of this grade_item
      *
index 9e37c42..a9a6829 100644 (file)
@@ -252,6 +252,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance,
         $rawgrade       = false;
         $feedback       = false;
         $feedbackformat = FORMAT_MOODLE;
+        $feedbackfiles = [];
         $usermodified   = $USER->id;
         $datesubmitted  = null;
         $dategraded     = null;
@@ -268,6 +269,10 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance,
             $feedbackformat = $grade['feedbackformat'];
         }
 
+        if (array_key_exists('feedbackfiles', $grade)) {
+            $feedbackfiles = $grade['feedbackfiles'];
+        }
+
         if (array_key_exists('usermodified', $grade)) {
             $usermodified = $grade['usermodified'];
         }
@@ -281,7 +286,8 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance,
         }
 
         // update or insert the grade
-        if (!$grade_item->update_raw_grade($userid, $rawgrade, $source, $feedback, $feedbackformat, $usermodified, $dategraded, $datesubmitted, $grade_grade)) {
+        if (!$grade_item->update_raw_grade($userid, $rawgrade, $source, $feedback, $feedbackformat, $usermodified,
+                $dategraded, $datesubmitted, $grade_grade, $feedbackfiles)) {
             $failed = true;
         }
     }
@@ -537,7 +543,17 @@ function grade_get_grades($courseid, $itemtype, $itemmodule, $iteminstance, $use
                         if (is_null($grade->feedback)) {
                             $grade->str_feedback = '';
                         } else {
-                            $grade->str_feedback = format_text($grade->feedback, $grade->feedbackformat);
+                            $feedback = file_rewrite_pluginfile_urls(
+                                $grade->feedback,
+                                'pluginfile.php',
+                                $grade_grades[$userid]->get_context()->id,
+                                GRADE_FILE_COMPONENT,
+                                GRADE_FEEDBACK_FILEAREA,
+                                $grade_grades[$userid]->id
+                            );
+
+                            $grade->str_feedback = format_text($feedback, $grade->feedbackformat,
+                                ['context' => $grade_grades[$userid]->get_context()]);
                         }
 
                         $item->grades[$userid] = $grade;
index 288ed76..bdb18ab 100644 (file)
@@ -135,6 +135,30 @@ 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!
+* The gradebook now supports the ability to accept files as feedback. This can be achieved by adding
+  'feedbackfiles' to the $grades parameter passed to grade_update().
+    For example -
+        $grades['feedbackfiles'] = [
+            'contextid' => 1,
+            'component' => 'mod_xyz',
+            'filearea' => 'mod_xyz_feedback',
+            'itemid' => 2
+        ];
+  These files will be then copied to the gradebook file area.
+* Allow users to choose who can message them for privacy reasons, with a 'growing circle of contactability':
+  - Added $CFG->messagingallusers, for enabling messaging to all site users. Default value: 0.
+    When $CFG->messagingallusers = false users can choose being contacted by only contacts or contacts and users sharing a course with them.
+    In that case, the default user preference is MESSAGE_PRIVACY_COURSEMEMBER (users sharing a course).
+    When $CFG->messagingallusers = true users have a new option for the privacy messaging preferences: "Anyone on the site". In that case,
+    the default user preference is MESSAGE_PRIVACY_SITE (all site users).
+  - Added $CFG->keepmessagingallusersenabled setting to config.php to force enabling $CFG->messagingallusers during the upgrading process.
+    Default value: 0.
+    When $CFG->keepmessagingallusersenabled is set to true, $CFG->messagingallusers will be also set to true to enable messaging site users.
+    However, when it is empty, $CFG->messagingallusers will be disabled during the upgrading process, so the users will only be able to
+    message contacts and users sharing a course with them.
 
 === 3.5 ===
 
index 436b3bb..54bf917 100644 (file)
Binary files a/message/amd/build/message_preferences.min.js and b/message/amd/build/message_preferences.min.js differ
index c8a4372..c5944b6 100644 (file)
@@ -29,8 +29,7 @@ define(['jquery', 'core/ajax', 'core/notification',
     var SELECTORS = {
         PREFERENCE: '[data-state]',
         PREFERENCES_CONTAINER: '[data-region="preferences-container"]',
-        BLOCK_NON_CONTACTS: '[data-region="block-non-contacts-container"] [data-block-non-contacts]',
-        BLOCK_NON_CONTACTS_CONTAINER: '[data-region="block-non-contacts-container"]',
+        CONTACTABLE_PRIVACY_CONTAINER: '[data-region="privacy-setting-container"]',
     };
 
     /**
@@ -56,16 +55,15 @@ define(['jquery', 'core/ajax', 'core/notification',
     };
 
     /**
-     * Update the block messages from non-contacts user preference in the DOM and
+     * Update the contactable privacy user preference in the DOM and
      * send a request to update on the server.
      *
      * @return {Promise}
-     * @method saveBlockNonContactsStatus
+     * @method saveContactablePrivacySetting
      */
-    MessagePreferences.prototype.saveBlockNonContactsStatus = function() {
-        var checkbox = this.root.find(SELECTORS.BLOCK_NON_CONTACTS);
-        var container = this.root.find(SELECTORS.BLOCK_NON_CONTACTS_CONTAINER);
-        var ischecked = checkbox.prop('checked');
+    MessagePreferences.prototype.saveContactablePrivacySetting = function() {
+        var container = this.root.find(SELECTORS.CONTACTABLE_PRIVACY_CONTAINER);
+        var value = $("input[type='radio']:checked").val();
 
         if (container.hasClass('loading')) {
             return $.Deferred().resolve();
@@ -79,8 +77,8 @@ define(['jquery', 'core/ajax', 'core/notification',
                 userid: this.userId,
                 preferences: [
                     {
-                        type: checkbox.attr('data-preference-key'),
-                        value: ischecked ? 1 : 0,
+                        type: container.attr('data-preference-key'),
+                        value: value,
                     }
                 ]
             }
@@ -103,20 +101,22 @@ define(['jquery', 'core/ajax', 'core/notification',
             CustomEvents.events.activate
         ]);
 
-        this.root.on(CustomEvents.events.activate, SELECTORS.BLOCK_NON_CONTACTS, function() {
-            this.saveBlockNonContactsStatus();
-        }.bind(this));
-
         this.root.on('change', function(e) {
-            if (!this.preferencesDisabled()) {
-                var preferencesContainer = $(e.target).closest(SELECTORS.PREFERENCES_CONTAINER);
-                var preferenceElement = $(e.target).closest(SELECTORS.PREFERENCE);
-                var messagePreference = new MessageNotificationPreference(preferencesContainer, this.userId);
+            // Add listener for privacy setting radio buttons change.
+            if (e.target.name == 'message_blocknoncontacts') {
+                this.saveContactablePrivacySetting();
+            } else {
+                // Add listener for processor preferences.
+                if (!this.preferencesDisabled()) {
+                    var preferencesContainer = $(e.target).closest(SELECTORS.PREFERENCES_CONTAINER);
+                    var preferenceElement = $(e.target).closest(SELECTORS.PREFERENCE);
+                    var messagePreference = new MessageNotificationPreference(preferencesContainer, this.userId);
 
-                preferenceElement.addClass('loading');
-                messagePreference.save().always(function() {
-                    preferenceElement.removeClass('loading');
-                });
+                    preferenceElement.addClass('loading');
+                    messagePreference.save().always(function() {
+                        preferenceElement.removeClass('loading');
+                    });
+                }
             }
         }.bind(this));
     };
index 8bc93a7..d9b507e 100644 (file)
@@ -46,6 +46,21 @@ class api {
      */
     const MESSAGE_ACTION_DELETED = 2;
 
+    /**
+     * The privacy setting for being messaged by anyone within courses user is member of.
+     */
+    const MESSAGE_PRIVACY_COURSEMEMBER = 0;
+
+    /**
+     * The privacy setting for being messaged only by contacts.
+     */
+    const MESSAGE_PRIVACY_ONLYCONTACTS = 1;
+
+    /**
+     * The privacy setting for being messaged by anyone on the site.
+     */
+    const MESSAGE_PRIVACY_SITE = 2;
+
     /**
      * Handles searching for messages in the message area.
      *
@@ -628,17 +643,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 +678,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 +695,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 +733,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;
     }
 
     /**
@@ -857,7 +897,7 @@ class api {
         }
 
         // Load general messaging preferences.
-        $preferences->blocknoncontacts = get_user_preferences('message_blocknoncontacts', '', $user->id);
+        $preferences->blocknoncontacts = self::get_user_privacy_messaging_preference($user->id);
         $preferences->mailformat = $user->mailformat;
         $preferences->mailcharset = get_user_preferences('mailcharset', '', $user->id);
 
@@ -927,6 +967,36 @@ class api {
         return true;
     }
 
+    /**
+     * Get the messaging preference for a user.
+     * If the user has not any messaging privacy preference:
+     * - When $CFG->messagingallusers = false the default user preference is MESSAGE_PRIVACY_COURSEMEMBER.
+     * - When $CFG->messagingallusers = true the default user preference is MESSAGE_PRIVACY_SITE.
+     *
+     * @param  int    $userid The user identifier.
+     * @return int    The default messaging preference.
+     */
+    public static function get_user_privacy_messaging_preference(int $userid) : int {
+        global $CFG;
+
+        // When $CFG->messagingallusers is enabled, default value for the messaging preference will be "Anyone on the site";
+        // otherwise, the default value will be "My contacts and anyone in my courses".
+        if (empty($CFG->messagingallusers)) {
+            $defaultprefvalue = self::MESSAGE_PRIVACY_COURSEMEMBER;
+        } else {
+            $defaultprefvalue = self::MESSAGE_PRIVACY_SITE;
+        }
+        $privacypreference = get_user_preferences('message_blocknoncontacts', $defaultprefvalue, $userid);
+
+        // When the $CFG->messagingallusers privacy setting is disabled, MESSAGE_PRIVACY_SITE is
+        // also disabled, so it has to be replaced to MESSAGE_PRIVACY_COURSEMEMBER.
+        if (empty($CFG->messagingallusers) && $privacypreference == self::MESSAGE_PRIVACY_SITE) {
+            $privacypreference = self::MESSAGE_PRIVACY_COURSEMEMBER;
+        }
+
+        return $privacypreference;
+    }
+
     /**
      * Checks if the recipient is allowing messages from users that aren't a
      * contact. If not then it checks to make sure the sender is in the
@@ -937,23 +1007,31 @@ class api {
      * @return bool true if $sender is blocked, false otherwise.
      */
     public static function is_user_non_contact_blocked($recipient, $sender = null) {
-        global $USER;
+        global $USER, $CFG;
 
         if (is_null($sender)) {
             // The message is from the logged in user, unless otherwise specified.
             $sender = $USER;
         }
 
-        $blockednoncontacts = get_user_preferences('message_blocknoncontacts', '', $recipient->id);
-        if (!empty($blockednoncontacts)) {
-            // Confirm the sender is a contact of the recipient.
-            if (self::is_contact($sender->id, $recipient->id)) {
-                // All good, the recipient is a contact of the sender.
-                return false;
-            } else {
-                // Oh no, the recipient is not a contact. Looks like we can't send the message.
-                return true;
-            }
+        $privacypreference = self::get_user_privacy_messaging_preference($recipient->id);
+        switch ($privacypreference) {
+            case self::MESSAGE_PRIVACY_SITE:
+                if (!empty($CFG->messagingallusers)) {
+                    // Users can be messaged without being contacts or members of the same course.
+                    break;
+                }
+                // When the $CFG->messagingallusers privacy setting is disabled, continue with the next
+                // case, because MESSAGE_PRIVACY_SITE is replaced to MESSAGE_PRIVACY_COURSEMEMBER.
+            case self::MESSAGE_PRIVACY_COURSEMEMBER:
+                // Confirm the sender and the recipient are both members of the same course.
+                if (enrol_sharing_course($recipient, $sender)) {
+                    // All good, the recipient and the sender are members of the same course.
+                    return false;
+                }
+            case self::MESSAGE_PRIVACY_ONLYCONTACTS:
+                // True if they aren't contacts (they can't send a message because of the privacy settings), false otherwise.
+                return !self::is_contact($sender->id, $recipient->id);
         }
 
         return false;
@@ -1202,30 +1280,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);
-
-        if ($message->useridfrom == $userid) {
-            $userdeleting = 'useridfrom';
-        } else if ($message->useridto == $userid) {
-            $userdeleting = 'useridto';
-        } else {
-            return false;
+        $systemcontext = \context_system::instance();
+
+        $conversationid = $DB->get_field('messages', 'conversationid', ['id' => $messageid], MUST_EXIST);
+
+        if (has_capability('moodle/site:deleteanymessage', $systemcontext)) {
+            return true;
         }
 
-        $systemcontext = \context_system::instance();
+        if (!self::is_user_in_conversation($userid, $conversationid)) {
+            return false;
+        }
 
-        // 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 +1311,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 +1328,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 +1674,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]);
+
+    }
 }
index f678c2f..89016ec 100644 (file)
@@ -92,72 +92,36 @@ class core_message_external extends external_api {
         }
         list($sqluserids, $sqlparams) = $DB->get_in_or_equal($receivers);
         $tousers = $DB->get_records_select("user", "id " . $sqluserids . " AND deleted = 0", $sqlparams);
-        $blocklist   = array();
-        $contactlist = array();
-        $contactsqlparams = array_merge($sqlparams, [$USER->id], [$USER->id], $sqlparams);
-        $rs = $DB->get_recordset_sql("SELECT *
-                                        FROM {message_contacts}
-                                       WHERE (userid $sqluserids AND contactid = ?)
-                                          OR (userid = ? AND contactid $sqluserids)", $contactsqlparams);
-        foreach ($rs as $record) {
-            $useridtouse = $record->userid;
-            if ($record->userid == $USER->id) {
-                $useridtouse = $record->contactid;
-            }
-            $contactlist[$useridtouse] = true;
-        }
-        $rs->close();
-        $blocksqlparams = array_merge($sqlparams, [$USER->id]);
-        $rs = $DB->get_recordset_sql("SELECT *
-                                        FROM {message_users_blocked}
-                                       WHERE userid $sqluserids
-                                         AND blockeduserid = ?", $blocksqlparams);
-        foreach ($rs as $record) {
-            $blocklist[$record->userid] = true;
-        }
-        $rs->close();
-
-        $canreadallmessages = has_capability('moodle/site:readallmessages', $context);
 
         $resultmessages = array();
         foreach ($params['messages'] as $message) {
             $resultmsg = array(); //the infos about the success of the operation
 
-            //we are going to do some checking
-            //code should match /messages/index.php checks
+            // We are going to do some checking.
+            // Code should match /messages/index.php checks.
             $success = true;
 
-            //check the user exists
+            // Check the user exists.
             if (empty($tousers[$message['touserid']])) {
                 $success = false;
                 $errormessage = get_string('touserdoesntexist', 'message', $message['touserid']);
             }
 
-            //check that the touser is not blocking the current user
-            if ($success and !empty($blocklist[$message['touserid']]) and !$canreadallmessages) {
+            // TODO MDL-31118 performance improvement - edit the function so we can pass an array instead userid
+            // Check if the recipient can be messaged by the sender.
+            if ($success && !\core_message\api::can_post_message($tousers[$message['touserid']], $USER)) {
                 $success = false;
-                $errormessage = get_string('userisblockingyou', 'message');
+                $errormessage = get_string('usercantbemessaged', 'message', fullname(\core_user::get_user($message['touserid'])));
             }
 
-            // Check if the user is a contact
-            //TODO MDL-31118 performance improvement - edit the function so we can pass an array instead userid
-            $blocknoncontacts = get_user_preferences('message_blocknoncontacts', NULL, $message['touserid']);
-            // message_blocknoncontacts option is on and current user is not in contact list
-            if ($success && empty($contactlist[$message['touserid']]) && !empty($blocknoncontacts)) {
-                // The user isn't a contact and they have selected to block non contacts so this message won't be sent.
-                $success = false;
-                $errormessage = get_string('userisblockingyounoncontact', 'message',
-                        fullname(core_user::get_user($message['touserid'])));
-            }
-
-            //now we can send the message (at least try)
+            // Now we can send the message (at least try).
             if ($success) {
-                //TODO MDL-31118 performance improvement - edit the function so we can pass an array instead one touser object
+                // TODO MDL-31118 performance improvement - edit the function so we can pass an array instead one touser object.
                 $success = message_post_message($USER, $tousers[$message['touserid']],
                         $message['text'], external_validate_format($message['textformat']));
             }
 
-            //build the resultmsg
+            // Build the resultmsg.
             if (isset($message['clientmsgid'])) {
                 $resultmsg['clientmsgid'] = $message['clientmsgid'];
             }
@@ -571,7 +535,6 @@ class core_message_external extends external_api {
     /**
      * Unblock contacts.
      *
-     * @deprecated since Moodle 3.6
      * @param array $userids array of user IDs.
      * @param int $userid The id of the user we are unblocking the contacts for
      * @return null
@@ -2541,6 +2504,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 +2520,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 +2552,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 +2574,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 +2587,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 +3062,6 @@ class core_message_external extends external_api {
         );
     }
 
-
     /**
      * Returns description of method parameters
      *
@@ -3068,7 +3117,7 @@ class core_message_external extends external_api {
         $result = array(
             'warnings' => array(),
             'preferences' => $notificationlistoutput->export_for_template($renderer),
-            'blocknoncontacts' => get_user_preferences('message_blocknoncontacts', '', $user->id) ? true : false,
+            'blocknoncontacts' => \core_message\api::get_user_privacy_messaging_preference($user->id),
         );
         return $result;
     }
@@ -3083,7 +3132,7 @@ class core_message_external extends external_api {
         return new external_function_parameters(
             array(
                 'preferences' => self::get_preferences_structure(),
-                'blocknoncontacts' => new external_value(PARAM_BOOL, 'Whether to block or not messages from non contacts'),
+                'blocknoncontacts' => new external_value(PARAM_INT, 'Privacy messaging setting to define who can message you'),
                 'warnings' => new external_warnings(),
             )
         );
index 1fbe61c..a85e65a 100644 (file)
@@ -716,10 +716,26 @@ function core_message_can_edit_message_profile($user) {
  * @return array
  */
 function core_message_user_preferences() {
-
     $preferences = [];
-    $preferences['message_blocknoncontacts'] = array('type' => PARAM_INT, 'null' => NULL_NOT_ALLOWED, 'default' => 0,
-        'choices' => array(0, 1));
+    $preferences['message_blocknoncontacts'] = array(
+        'type' => PARAM_INT,
+        'null' => NULL_NOT_ALLOWED,
+        'default' => 0,
+        'choices' => array(
+            \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS,
+            \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER,
+            \core_message\api::MESSAGE_PRIVACY_SITE
+        ),
+        'cleancallback' => function ($value) {
+            global $CFG;
+
+            // When site-wide messaging between users is disabled, MESSAGE_PRIVACY_SITE should be converted.
+            if (empty($CFG->messagingallusers) && $value === \core_message\api::MESSAGE_PRIVACY_SITE) {
+                return \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER;
+            }
+            return $value;
+        }
+    );
     $preferences['/^message_provider_([\w\d_]*)_logged(in|off)$/'] = array('isregex' => true, 'type' => PARAM_NOTAGS,
         'null' => NULL_NOT_ALLOWED, 'default' => 'none',
         'permissioncallback' => function ($user, $preferencename) {
index 67289a2..b27e22e 100644 (file)
@@ -6,9 +6,16 @@ Feature: Message popover unread messages
 
   Background:
     Given the following "users" exist:
-      | username | firstname | lastname | email |
+      | username | firstname | lastname | email       |
       | student1 | Student | 1 | student1@example.com |
       | student2 | Student | 2 | student2@example.com |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | student1 | C1     | student        |
+      | student2 | C1     | student        |
     And I log in as "student2"
     And I send "Test message" message to "Student 1" user
     And I log out
index 1aac259..17af06d 100644 (file)
@@ -227,6 +227,8 @@ class core_message_renderer extends plugin_renderer_base {
      * @return string The text to render
      */
     public function render_user_message_preferences($user) {
+        global $CFG;
+
         // Filter out enabled, available system_configured and user_configured processors only.
         $readyprocessors = array_filter(get_message_processors(), function($processor) {
             return $processor->enabled &&
@@ -243,7 +245,29 @@ class core_message_renderer extends plugin_renderer_base {
         $notificationlistoutput = new \core_message\output\preferences\message_notification_list($readyprocessors,
             $providers, $preferences, $user);
         $context = $notificationlistoutput->export_for_template($this);
-        $context['blocknoncontacts'] = get_user_preferences('message_blocknoncontacts', '', $user->id) ? true : false;
+
+        // Get the privacy settings options for being messaged.
+        $privacysetting = \core_message\api::get_user_privacy_messaging_preference($user->id);
+        $choices = array();
+        $choices[] = [
+            'value' => \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS,
+            'text' => get_string('contactableprivacy_onlycontacts', 'message'),
+            'checked' => ($privacysetting == \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS)
+        ];
+        $choices[] = [
+            'value' => \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER,
+            'text' => get_string('contactableprivacy_coursemember', 'message'),
+            'checked' => ($privacysetting == \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER)
+        ];
+        if (!empty($CFG->messagingallusers)) {
+            // Add the MESSAGE_PRIVACY_SITE option when site-wide messaging between users is enabled.
+            $choices[] = [
+                'value' => \core_message\api::MESSAGE_PRIVACY_SITE,
+                'text' => get_string('contactableprivacy_site', 'message'),
+                'checked' => ($privacysetting == \core_message\api::MESSAGE_PRIVACY_SITE)
+            ];
+        }
+        $context['privacychoices'] = $choices;
 
         return $this->render_from_template('message/message_preferences', $context);
     }
index 43e2ea9..ebe1c40 100644 (file)
@@ -29,6 +29,7 @@
     * userid The logged in user id
     * disableall If the user has disabled notifications
     * components The list of notification components
+    * privacychoices The choice options for the contactable privacy setting
 
     Example context (json):
     {
                     }
                 ]
             }
+        ],
+        "privacychoices": [
+            {
+                "value": 1,
+                "text": "My contacts only",
+                "checked": 0
+            },
+            {
+                "value": 2,
+                "text": "Anyone within courses I am a member of",
+                "checked": 1
+            }
         ]
     }
 }}
 <div class="preferences-page-container" data-region="preferences-page-container">
     <h2>{{#str}} messagepreferences, message {{/str}}</h2>
-    <div class="checkbox-container" data-region="block-non-contacts-container">
-        <input id="block-non-contacts"
-                type="checkbox"
-                data-user-id="{{userid}}"
-                data-block-non-contacts
-                data-preference-key="message_blocknoncontacts"
-                {{#blocknoncontacts}}checked{{/blocknoncontacts}} />
-        <label for="block-non-contacts">{{#str}} blocknoncontacts, message {{/str}}</label>
-        {{> core/loading }}
-    </div>
+    <div class="privacy-setting-container"
+         data-user-id="{{userid}}"
+         data-region="privacy-setting-container"
+         data-preference-key="message_blocknoncontacts">
+        <p>{{#str}} contactableprivacy, message {{/str}}</p>
+       {{#privacychoices}}
+        <input id="action-selection-option-{{value}}"
+               type="radio"
+               name="message_blocknoncontacts"
+               value="{{value}}"
+               {{#checked}}checked="checked"{{/checked}}/>
+        <label for="action-selection-option-{{value}}">{{text}}</label>
+        <br>
+       {{/privacychoices}}
+    </div><br>
     <div class="preferences-container {{#disableall}}disabled{{/disableall}}"
         data-user-id="{{userid}}"
         data-region="preferences-container">
index 0278d67..8be0f3f 100644 (file)
@@ -109,6 +109,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
 
         // Create user to add to the admin's block list.
         $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
 
         $this->assertEquals(0, \core_message\api::count_blocked_users());
 
@@ -1146,17 +1147,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 +1191,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);
@@ -1273,10 +1335,17 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $user1 = self::getDataGenerator()->create_user();
         $user2 = self::getDataGenerator()->create_user();
 
-        // Set as the user 1.
+        // Set as the first user.
         $this->setUser($user1);
 
-        // They can post to someone else.
+        // With the default privacy setting, users can't message them.
+        $this->assertFalse(\core_message\api::can_post_message($user2));
+
+        // Enrol users to the same course.
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id);
+        // After enrolling users to the course, they should be able to message them with the default privacy setting.
         $this->assertTrue(\core_message\api::can_post_message($user2));
     }
 
@@ -1302,6 +1371,27 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->assertFalse(\core_message\api::can_post_message($user2));
     }
 
+    /**
+     * Tests the user can post a message when they are contact.
+     */
+    public function test_can_post_message_when_contact() {
+        // Create some users.
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        // Set as the first user.
+        $this->setUser($user1);
+
+        // Check that we can not send user2 a message.
+        $this->assertFalse(\core_message\api::can_post_message($user2));
+
+        // Add users as contacts.
+        \core_message\api::add_contact($user1->id, $user2->id);
+
+        // Check that the return result is now true.
+        $this->assertTrue(\core_message\api::can_post_message($user2));
+    }
+
     /**
      * Tests the user can't post a message if they are not a contact and the user
      * has requested messages only from contacts.
@@ -1315,7 +1405,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->setUser($user1);
 
         // Set the second user's preference to not receive messages from non-contacts.
-        set_user_preference('message_blocknoncontacts', 1, $user2->id);
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS, $user2->id);
 
         // Check that we can not send user 2 a message.
         $this->assertFalse(\core_message\api::can_post_message($user2));
@@ -1339,6 +1429,77 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->assertFalse(\core_message\api::can_post_message($user1, $user2));
     }
 
+    /**
+     * Tests the user can post a message when site-wide messaging setting is enabled,
+     * even if they are not a contact and are not members of the same course.
+     */
+    public function test_can_post_message_site_messaging_setting() {
+        // Create some users.
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        // Set as the first user.
+        $this->setUser($user1);
+
+        // Set the second user's preference to receive messages from everybody. As site-wide messaging setting
+        // is disabled by default, the value will be changed to MESSAGE_PRIVACY_COURSEMEMBER.
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_SITE, $user2->id);
+        $this->assertFalse(\core_message\api::can_post_message($user2));
+
+        // Enable site-wide messagging privacy setting. The user will be able to receive messages from everybody.
+        set_config('messagingallusers', true);
+        // Check that we can send user2 a message.
+        $this->assertTrue(\core_message\api::can_post_message($user2));
+    }
+
+    /**
+     * Tests get_user_privacy_messaging_preference method.
+     */
+    public function test_get_user_privacy_messaging_preference() {
+        // Create some users.
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+        $user3 = self::getDataGenerator()->create_user();
+
+        // Enable site-wide messagging privacy setting. The user will be able to receive messages from everybody.
+        set_config('messagingallusers', true);
+
+        // Set some user preferences.
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_SITE, $user1->id);
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS, $user2->id);
+
+        // Check the returned value for each user.
+        $this->assertEquals(
+            \core_message\api::MESSAGE_PRIVACY_SITE,
+            \core_message\api::get_user_privacy_messaging_preference($user1->id)
+        );
+        $this->assertEquals(
+            \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS,
+            \core_message\api::get_user_privacy_messaging_preference($user2->id)
+        );
+        $this->assertEquals(
+            \core_message\api::MESSAGE_PRIVACY_SITE,
+            \core_message\api::get_user_privacy_messaging_preference($user3->id)
+        );
+
+        // Disable site-wide messagging privacy setting. The user will be able to receive messages from members of their course.
+        set_config('messagingallusers', false);
+
+        // Check the returned value for each user.
+        $this->assertEquals(
+            \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER,
+            \core_message\api::get_user_privacy_messaging_preference($user1->id)
+        );
+        $this->assertEquals(
+            \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS,
+            \core_message\api::get_user_privacy_messaging_preference($user2->id)
+        );
+        $this->assertEquals(
+            \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER,
+            \core_message\api::get_user_privacy_messaging_preference($user3->id)
+        );
+    }
+
     /**
      * Tests that when blocking messages from non-contacts is enabled that
      * non-contacts trying to send a message return false.
@@ -1351,13 +1512,18 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // Set as the first user.
         $this->setUser($user1);
 
-        // User hasn't sent their preference to block non-contacts, so should return false.
+        // By default, user only can be messaged by contacts and members of any of his/her courses.
+        $this->assertTrue(\core_message\api::is_user_non_contact_blocked($user2));
+
+        // Enable all users privacy messaging and check now the default user's preference has been set to allow receiving
+        // messages from everybody.
+        set_config('messagingallusers', true);
+        // Check that the return result is now false because any site user can contact him/her.
         $this->assertFalse(\core_message\api::is_user_non_contact_blocked($user2));
 
         // Set the second user's preference to not receive messages from non-contacts.
-        set_user_preference('message_blocknoncontacts', 1, $user2->id);
-
-        // Check that the return result is now true.
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS, $user2->id);
+        // Check that the return result is still true (because is even more restricted).
         $this->assertTrue(\core_message\api::is_user_non_contact_blocked($user2));
 
         // Add the first user as a contact for the second user.
@@ -1366,12 +1532,10 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // Check that the return result is now false.
         $this->assertFalse(\core_message\api::is_user_non_contact_blocked($user2));
 
-        // Set the first user's preference to not receive messages from non-contacts.
-        set_user_preference('message_blocknoncontacts', 1, $user1->id);
-        $this->setUser($user2);
-        // Confirm it is still false. We want to ensure a contact request works both ways
-        // as it is now an agreement between users.
-        $this->assertFalse(\core_message\api::is_user_non_contact_blocked($user1));
+        // Set the second user's preference to receive messages from course members.
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_COURSEMEMBER, $user2->id);
+        // Check that the return result is still false (because $user1 is still his/her contact).
+        $this->assertFalse(\core_message\api::is_user_non_contact_blocked($user2));
     }
 
     /**
@@ -1696,6 +1860,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
 
         // Get the contacts and the unread message count.
         $messages = \core_message\api::get_contacts_with_unread_message_count($user2->id);
+
         // Confirm the size is correct.
         $this->assertCount(2, $messages);
         ksort($messages);
@@ -2226,6 +2391,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.
      *
index 25d0c08..c6f3546 100644 (file)
@@ -6,9 +6,16 @@ Feature: Delete all messages
 
   Scenario: Delete all messages
     Given the following "users" exist:
-      | username | firstname | lastname | email            |
+      | username | firstname | lastname | email                |
       | user1    | User      | 1        | user1@example.com    |
       | user2    | User      | 2        | user2@example.com    |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role        |
+      | user1    | C1     | student     |
+      | user2    | C1     | student     |
     And I log in as "user2"
     And I send "User 2 to User 1 message 1" message to "User 1" user
     And I send "User 2 to User 1 message 2" message in the message area
index 600eb43..a97087d 100644 (file)
@@ -6,9 +6,16 @@ Feature: Delete messages
 
   Scenario: Delete messages
     Given the following "users" exist:
-      | username | firstname | lastname | email            |
+      | username | firstname | lastname | email                |
       | user1    | User      | 1        | user1@example.com    |
       | user2    | User      | 2        | user2@example.com    |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | user1    | C1     | student        |
+      | user2    | C1     | student        |
     And I log in as "user2"
     And I send "User 2 to User 1 message 1" message to "User 1" user
     And I send "User 2 to User 1 message 2" message in the message area
index d6eb761..a237ac3 100644 (file)
@@ -9,6 +9,13 @@ Feature: Reply message
       | username | firstname | lastname | email            |
       | user1    | User      | 1        | user1@example.com    |
       | user2    | User      | 2        | user2@example.com    |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | user1    | C1     | student        |
+      | user2    | C1     | student        |
     And I log in as "user2"
     And I send "User 2 to User 1" message to "User 1" user
     And I log out
index 96a2e0d..5c7d3e9 100644 (file)
@@ -6,10 +6,18 @@ Feature: Search messages
 
   Background:
     Given the following "users" exist:
-      | username | firstname | lastname | email            |
+      | username | firstname | lastname | email                |
       | user1    | User      | 1        | user1@example.com    |
       | user2    | User      | 2        | user2@example.com    |
       | user3    | User      | 3        | user3@example.com    |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | user1    | C1     | student        |
+      | user2    | C1     | student        |
+      | user3    | C1     | student        |
     And I log in as "user2"
     And I send "User 2 to User 1" message to "User 1" user
     And I log out
index d4ad844..f979bd9 100644 (file)
@@ -6,10 +6,18 @@ Feature: View messages
 
   Scenario: View messages from multiple users
     Given the following "users" exist:
-      | username | firstname | lastname | email            |
+      | username | firstname | lastname | email                |
       | user1    | User      | 1        | user1@example.com    |
       | user2    | User      | 2        | user2@example.com    |
       | user3    | User      | 3        | user3@example.com    |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | user1    | C1     | student        |
+      | user2    | C1     | student        |
+      | user3    | C1     | student        |
     And I log in as "user2"
     And I send "User 2 to User 1" message to "User 1" user
     And I log out
index 25660ce..714461b 100644 (file)
@@ -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++;
         }
index 784ad14..aa7f61f 100644 (file)
@@ -115,6 +115,18 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         $sentmessages = core_message_external::send_instant_messages($messages);
         $sentmessages = external_api::clean_returnvalue(core_message_external::send_instant_messages_returns(), $sentmessages);
+        $this->assertEquals(
+            get_string('usercantbemessaged', 'message', fullname(\core_user::get_user($message1['touserid']))),
+            array_pop($sentmessages)['errormessage']
+        );
+
+        // Add the user1 as a contact.
+        \core_message\api::add_contact($user1->id, $user2->id);
+
+        // Send message again. Now it should work properly.
+        $sentmessages = core_message_external::send_instant_messages($messages);
+        // We need to execute the return values cleaning process to simulate the web service server.
+        $sentmessages = external_api::clean_returnvalue(core_message_external::send_instant_messages_returns(), $sentmessages);
 
         $sentmessage = reset($sentmessages);
 
@@ -165,7 +177,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         $sentmessage = reset($sentmessages);
 
-        $this->assertEquals(get_string('userisblockingyou', 'message'), $sentmessage['errormessage']);
+        $this->assertEquals(get_string('usercantbemessaged', 'message', fullname($user2)), $sentmessage['errormessage']);
 
         $this->assertEquals(0, $DB->count_records('messages'));
     }
@@ -187,7 +199,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->setUser($user1);
 
         // Set the user preference so user 2 does not accept messages from non-contacts.
-        set_user_preference('message_blocknoncontacts', 1, $user2);
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS, $user2);
 
         // Create test message data.
         $message1 = array();
@@ -201,7 +213,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         $sentmessage = reset($sentmessages);
 
-        $this->assertEquals(get_string('userisblockingyounoncontact', 'message', fullname($user2)), $sentmessage['errormessage']);
+        $this->assertEquals(get_string('usercantbemessaged', 'message', fullname($user2)), $sentmessage['errormessage']);
 
         $this->assertEquals(0, $DB->count_records('messages'));
     }
@@ -223,7 +235,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->setUser($user1);
 
         // Set the user preference so user 2 does not accept messages from non-contacts.
-        set_user_preference('message_blocknoncontacts', 1, $user2);
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS, $user2);
 
         \core_message\api::add_contact($user1->id, $user2->id);
 
@@ -1648,7 +1660,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 +3457,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 +3495,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.
      */
@@ -3507,10 +3693,13 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $user = self::getDataGenerator()->create_user();
         $this->setUser($user);
 
+        // Enable site-wide messagging privacy setting. The user will be able to receive messages from everybody.
+        set_config('messagingallusers', true);
+
         // Set a couple of preferences to test.
         set_user_preference('message_provider_moodle_instantmessage_loggedin', 'email', $user);
         set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'email', $user);
-        set_user_preference('message_blocknoncontacts', 1, $user);
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_SITE, $user);
 
         $prefs = core_message_external::get_user_message_preferences();
         $prefs = external_api::clean_returnvalue(core_message_external::get_user_message_preferences_returns(), $prefs);
@@ -3518,7 +3707,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         // Check components.
         $this->assertCount(1, $prefs['preferences']['components']);
-        $this->assertTrue($prefs['blocknoncontacts']);
+        $this->assertEquals(\core_message\api::MESSAGE_PRIVACY_SITE, $prefs['blocknoncontacts']);
 
         // Check some preferences that we previously set.
         $found = false;
index 3500c79..27e2232 100644 (file)
@@ -160,7 +160,7 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide
         // Set some message user preferences.
         set_user_preference('message_provider_moodle_instantmessage_loggedin', 'airnotifier', $USER->id);
         set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'popup', $USER->id);
-        set_user_preference('message_blocknoncontacts', 1, $USER->id);
+        set_user_preference('message_blocknoncontacts', \core_message\api::MESSAGE_PRIVACY_ONLYCONTACTS, $USER->id);
         set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'inbound', $user->id);
 
         // Set an unrelated preference.
index 2ada652..b623af2 100644 (file)
@@ -28,10 +28,16 @@ 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.
+* The following function has been added for getting the privacy messaging preference:
+  - get_user_privacy_messaging_preference()
 
 === 3.5 ===
 
index d885b34..5d314bd 100644 (file)
@@ -56,6 +56,12 @@ class backup_assignfeedback_comments_subplugin extends backup_subplugin {
         $subpluginelement->set_source_table('assignfeedback_comments',
                                             array('grade' => backup::VAR_PARENTID));
 
+        $subpluginelement->annotate_files(
+            'assignfeedback_comments',
+            'feedback',
+            'grade'
+        );
+
         return $subplugin;
     }
 }
index 9b95aed..1df4099 100644 (file)
@@ -71,5 +71,13 @@ class restore_assignfeedback_comments_subplugin extends restore_subplugin {
         $data->grade = $this->get_mappingid('grade', $data->grade);
 
         $DB->insert_record('assignfeedback_comments', $data);
+
+        $this->add_related_files(
+            'assignfeedback_comments',
+            'feedback',
+            'grade',
+            null,
+            $oldgradeid
+        );
     }
 }
index ac9aa7f..9302526 100644 (file)
@@ -58,6 +58,8 @@ class provider implements metadataprovider, assignfeedback_provider {
             'commenttext' => 'privacy:metadata:commentpurpose'
         ];
         $collection->add_database_table('assignfeedback_comments', $data, 'privacy:metadata:tablesummary');
+        $collection->link_subsystem('core_files', 'privacy:metadata:filepurpose');
+
         return $collection;
     }
 
@@ -91,13 +93,29 @@ class provider implements metadataprovider, assignfeedback_provider {
         // Get that comment information and jam it into that exporter.
         $assign = $exportdata->get_assign();
         $plugin = $assign->get_plugin_by_type('assignfeedback', 'comments');
-        $comments = $plugin->get_feedback_comments($exportdata->get_pluginobject()->id);
+        $gradeid = $exportdata->get_pluginobject()->id;
+        $comments = $plugin->get_feedback_comments($gradeid);
         if ($comments && !empty($comments->commenttext)) {
-            $data = (object)['commenttext' => format_text($comments->commenttext, $comments->commentformat,
-                    ['context' => $exportdata->get_context()])];
-            writer::with_context($exportdata->get_context())
-                    ->export_data(array_merge($exportdata->get_subcontext(),
-                            [get_string('privacy:commentpath', 'assignfeedback_comments')]), $data);
+            $comments->commenttext = writer::with_context($assign->get_context())->rewrite_pluginfile_urls(
+                [],
+                ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+                ASSIGNFEEDBACK_COMMENTS_FILEAREA,
+                $gradeid,
+                $comments->commenttext
+            );
+
+            $currentpath = array_merge(
+                $exportdata->get_subcontext(),
+                [get_string('privacy:commentpath', 'assignfeedback_comments')]
+            );
+            $data = (object)
+            [
+                'commenttext' => format_text($comments->commenttext, $comments->commentformat,
+                    ['context' => $exportdata->get_context()])
+            ];
+            writer::with_context($exportdata->get_context())->export_data($currentpath, $data);
+            writer::with_context($exportdata->get_context())->export_area_files($currentpath,
+                ASSIGNFEEDBACK_COMMENTS_COMPONENT, ASSIGNFEEDBACK_COMMENTS_FILEAREA, $gradeid);
         }
     }
 
@@ -108,6 +126,10 @@ class provider implements metadataprovider, assignfeedback_provider {
      */
     public static function delete_feedback_for_context(assign_plugin_request_data $requestdata) {
         $assign = $requestdata->get_assign();
+        $fs = get_file_storage();
+        $fs->delete_area_files($requestdata->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA);
+
         $plugin = $assign->get_plugin_by_type('assignfeedback', 'comments');
         $plugin->delete_instance();
     }
@@ -119,6 +141,11 @@ class provider implements metadataprovider, assignfeedback_provider {
      */
     public static function delete_feedback_for_grade(assign_plugin_request_data $requestdata) {
         global $DB;
+
+        $fs = new \file_storage();
+        $fs->delete_area_files($requestdata->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA, $requestdata->get_pluginobject()->id);
+
         $DB->delete_records('assignfeedback_comments', ['assignment' => $requestdata->get_assign()->get_instance()->id,
                 'grade' => $requestdata->get_pluginobject()->id]);
     }
index b8c7551..ee4c540 100644 (file)
@@ -30,6 +30,7 @@ $string['pluginname'] = 'Feedback comments';
 $string['privacy:commentpath'] = 'Feedback comments';
 $string['privacy:metadata:assignmentid'] = 'Assignment ID';
 $string['privacy:metadata:commentpurpose'] = 'The comment text.';
+$string['privacy:metadata:filepurpose'] = 'Feedback files from the teacher for the student.';
 $string['privacy:metadata:gradepurpose'] = 'The grade ID associated with the comment.';
 $string['privacy:metadata:tablesummary'] = 'This stores comments made by the graders as feedback for the student on their submission.';
 $string['commentinline'] = 'Comment inline';
diff --git a/mod/assign/feedback/comments/lib.php b/mod/assign/feedback/comments/lib.php
new file mode 100644 (file)
index 0000000..1939422
--- /dev/null
@@ -0,0 +1,82 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * This file contains the moodle hooks for the comments feedback plugin
+ *
+ * @package   assignfeedback_comments
+ * @copyright 2018 Mark Nelson <markn@moodle.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Serves assignment comment feedback files.
+ *
+ * @param mixed $course course or id of the course
+ * @param mixed $cm course module or id of the course module
+ * @param context $context
+ * @param string $filearea
+ * @param array $args
+ * @param bool $forcedownload
+ * @param array $options - List of options affecting file serving.
+ * @return bool false if file not found, does not return if found - just send the file
+ */
+function assignfeedback_comments_pluginfile(
+        $course,
+        $cm,
+        context $context,
+        $filearea,
+        $args,
+        $forcedownload,
+        array $options = []) {
+    global $CFG, $DB;
+
+    require_once($CFG->dirroot . '/mod/assign/locallib.php');
+
+    if ($context->contextlevel != CONTEXT_MODULE) {
+        return false;
+    }
+
+    require_login($course, false, $cm);
+    $itemid = (int)array_shift($args);
+    $record = $DB->get_record('assign_grades', array('id' => $itemid), 'userid,assignment', MUST_EXIST);
+    $userid = $record->userid;
+
+    $assign = new assign($context, $cm, $course);
+    $instance = $assign->get_instance();
+
+    if ($instance->id != $record->assignment) {
+        return false;
+    }
+
+    if (!$assign->can_view_submission($userid)) {
+        return false;
+    }
+
+    $relativepath = implode('/', $args);
+
+    $fullpath = "/{$context->id}/assignfeedback_comments/$filearea/$itemid/$relativepath";
+
+    $fs = get_file_storage();
+
+    if (!$file = $fs->get_file_by_hash(sha1($fullpath)) or $file->is_directory()) {
+        return false;
+    }
+
+    // Download MUST be forced - security!
+    send_stored_file($file, 0, 0, true, $options);
+}
index e6c0f4a..4940063 100644 (file)
 
 defined('MOODLE_INTERNAL') || die();
 
+// File component for feedback comments.
+define('ASSIGNFEEDBACK_COMMENTS_COMPONENT', 'assignfeedback_comments');
+
+// File area for feedback comments.
+define('ASSIGNFEEDBACK_COMMENTS_FILEAREA', 'feedback');
+
 /**
  * Library class for comment feedback plugin extending feedback plugin base class.
  *
@@ -116,7 +122,15 @@ class assign_feedback_comments extends assign_feedback_plugin {
             }
         }
 
-        if ($commenttext == $data->assignfeedbackcomments_editor['text']) {
+        $formtext = $data->assignfeedbackcomments_editor['text'];
+
+        // Need to convert the form text to use @@PLUGINFILE@@ and format it so we can compare it with what is stored in the DB.
+        if (isset($data->assignfeedbackcomments_editor['itemid'])) {
+            $formtext = file_rewrite_urls_to_pluginfile($formtext, $data->assignfeedbackcomments_editor['itemid']);
+            $formtext = format_text($formtext, FORMAT_HTML);
+        }
+
+        if ($commenttext == $formtext) {
             return false;
         } else {
             return true;
@@ -254,18 +268,47 @@ class assign_feedback_comments extends assign_feedback_plugin {
      *
      * @param stdClass $submission
      * @param stdClass $data - Form data to be filled with the converted submission text and format.
+     * @param stdClass|null $grade
      * @return boolean - True if feedback text was set.
      */
-    protected function convert_submission_text_to_feedback($submission, $data) {
+    protected function convert_submission_text_to_feedback($submission, $data, $grade) {
+        global $DB;
+
         $format = false;
         $text = '';
 
         foreach ($this->assignment->get_submission_plugins() as $plugin) {
             $fields = $plugin->get_editor_fields();
             if ($plugin->is_enabled() && $plugin->is_visible() && !$plugin->is_empty($submission) && !empty($fields)) {
+                $user = $DB->get_record('user', ['id' => $submission->userid]);
+                // Copy the files to the feedback area.
+                if ($files = $plugin->get_files($submission, $user)) {
+                    $fs = get_file_storage();
+                    $component = 'assignfeedback_comments';
+                    $filearea = ASSIGNFEEDBACK_COMMENTS_FILEAREA;
+                    $itemid = $grade->id;
+                    $fieldupdates = [
+                        'component' => $component,
+                        'filearea' => $filearea,
+                        'itemid' => $itemid
+                    ];
+                    foreach ($files as $file) {
+                        if ($file instanceof stored_file) {
+                            // Before we create it, check that it doesn't already exist.
+                            if (!$fs->file_exists(
+                                    $file->get_contextid(),
+                                    $component,
+                                    $filearea,
+                                    $itemid,
+                                    $file->get_filepath(),
+                                    $file->get_filename())) {
+                                $fs->create_file_from_storedfile($fieldupdates, $file);
+                            }
+                        }
+                    }
+                }
                 foreach ($fields as $key => $description) {
-                    $rawtext = strip_pluginfile_content($plugin->get_editor_text($key, $submission->id));
-
+                    $rawtext = clean_text($plugin->get_editor_text($key, $submission->id));
                     $newformat = $plugin->get_editor_format($key, $submission->id);
 
                     if ($format !== false && $newformat != $format) {
@@ -282,8 +325,8 @@ class assign_feedback_comments extends assign_feedback_plugin {
         if ($format === false) {
             $format = FORMAT_HTML;
         }
-        $data->assignfeedbackcomments_editor['text'] = $text;
-        $data->assignfeedbackcomments_editor['format'] = $format;
+        $data->assignfeedbackcomments = $text;
+        $data->assignfeedbackcommentsformat = $format;
 
         return true;
     }
@@ -306,16 +349,25 @@ class assign_feedback_comments extends assign_feedback_plugin {
         }
 
         if ($feedbackcomments && !empty($feedbackcomments->commenttext)) {
-            $data->assignfeedbackcomments_editor['text'] = $feedbackcomments->commenttext;
-            $data->assignfeedbackcomments_editor['format'] = $feedbackcomments->commentformat;
+            $data->assignfeedbackcomments = $feedbackcomments->commenttext;
+            $data->assignfeedbackcommentsformat = $feedbackcomments->commentformat;
         } else {
             // No feedback given yet - maybe we need to copy the text from the submission?
             if (!empty($commentinlinenabled) && $submission) {
-                $this->convert_submission_text_to_feedback($submission, $data);
+                $this->convert_submission_text_to_feedback($submission, $data, $grade);
             }
         }
 
-        $mform->addElement('editor', 'assignfeedbackcomments_editor', $this->get_name(), null, null);
+        file_prepare_standard_editor(
+            $data,
+            'assignfeedbackcomments',
+            $this->get_editor_options(),
+            $this->assignment->get_context(),
+            ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA,
+            $grade->id
+        );
+        $mform->addElement('editor', 'assignfeedbackcomments_editor', $this->get_name(), null, $this->get_editor_options());
 
         return true;
     }
@@ -329,15 +381,27 @@ class assign_feedback_comments extends assign_feedback_plugin {
      */
     public function save(stdClass $grade, stdClass $data) {
         global $DB;
+
+        // Save the files.
+        $data = file_postupdate_standard_editor(
+            $data,
+            'assignfeedbackcomments',
+            $this->get_editor_options(),
+            $this->assignment->get_context(),
+            ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA,
+            $grade->id
+        );
+
         $feedbackcomment = $this->get_feedback_comments($grade->id);
         if ($feedbackcomment) {
-            $feedbackcomment->commenttext = $data->assignfeedbackcomments_editor['text'];
-            $feedbackcomment->commentformat = $data->assignfeedbackcomments_editor['format'];
+            $feedbackcomment->commenttext = $data->assignfeedbackcomments;
+            $feedbackcomment->commentformat = $data->assignfeedbackcommentsformat;
             return $DB->update_record('assignfeedback_comments', $feedbackcomment);
         } else {
             $feedbackcomment = new stdClass();
-            $feedbackcomment->commenttext = $data->assignfeedbackcomments_editor['text'];
-            $feedbackcomment->commentformat = $data->assignfeedbackcomments_editor['format'];
+            $feedbackcomment->commenttext = $data->assignfeedbackcomments;
+            $feedbackcomment->commentformat = $data->assignfeedbackcommentsformat;
             $feedbackcomment->grade = $grade->id;
             $feedbackcomment->assignment = $this->assignment->get_instance()->id;
             return $DB->insert_record('assignfeedback_comments', $feedbackcomment) > 0;
@@ -354,12 +418,17 @@ class assign_feedback_comments extends assign_feedback_plugin {
     public function view_summary(stdClass $grade, & $showviewlink) {
         $feedbackcomments = $this->get_feedback_comments($grade->id);
         if ($feedbackcomments) {
-            $text = format_text($feedbackcomments->commenttext,
-                                $feedbackcomments->commentformat,
-                                array('context' => $this->assignment->get_context()));
-            $short = shorten_text($text, 140);
+            $text = $this->rewrite_feedback_comments_urls($feedbackcomments->commenttext, $grade->id);
+            $text = format_text(
+                $text,
+                $feedbackcomments->commentformat,
+                [
+                    'context' => $this->assignment->get_context()
+                ]
+            );
 
             // Show the view all link if the text has been shortened.
+            $short = shorten_text($text, 140);
             $showviewlink = $short != $text;
             return $short;
         }
@@ -375,9 +444,16 @@ class assign_feedback_comments extends assign_feedback_plugin {
     public function view(stdClass $grade) {
         $feedbackcomments = $this->get_feedback_comments($grade->id);
         if ($feedbackcomments) {
-            return format_text($feedbackcomments->commenttext,
-                               $feedbackcomments->commentformat,
-                               array('context' => $this->assignment->get_context()));
+            $text = $this->rewrite_feedback_comments_urls($feedbackcomments->commenttext, $grade->id);
+            $text = format_text(
+                $text,
+                $feedbackcomments->commentformat,
+                [
+                    'context' => $this->assignment->get_context()
+                ]
+            );
+
+            return $text;
         }
         return '';
     }
@@ -482,6 +558,21 @@ class assign_feedback_comments extends assign_feedback_plugin {
         return '';
     }
 
+    /**
+     * Return any files this plugin wishes to save to the gradebook.
+     *
+     * @param stdClass $grade The assign_grades object from the db
+     * @return array
+     */
+    public function files_for_gradebook(stdClass $grade) : array {
+        return [
+            'contextid' => $this->assignment->get_context()->id,
+            'component' => ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            'filearea' => ASSIGNFEEDBACK_COMMENTS_FILEAREA,
+            'itemid' => $grade->id
+        ];
+    }
+
     /**
      * The assignment has been deleted - cleanup
      *
@@ -526,4 +617,38 @@ class assign_feedback_comments extends assign_feedback_plugin {
     public function get_config_for_external() {
         return (array) $this->get_config();
     }
+
+    /**
+     * Convert encoded URLs in $text from the @@PLUGINFILE@@/... form to an actual URL.
+     *
+     * @param string $text the Text to check
+     * @param int $gradeid The grade ID which refers to the id in the gradebook
+     */
+    private function rewrite_feedback_comments_urls(string $text, int $gradeid) {
+        return file_rewrite_pluginfile_urls(
+            $text,
+            'pluginfile.php',
+            $this->assignment->get_context()->id,
+            ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA,
+            $gradeid
+        );
+    }
+
+    /**
+     * File format options.
+     *
+     * @return array
+     */
+    private function get_editor_options() {
+        global $COURSE;
+
+        return [
+            'subdirs' => 1,
+            'maxbytes' => $COURSE->maxbytes,
+            'accepted_types' => '*',
+            'context' => $this->assignment->get_context(),
+            'maxfiles' => EDITOR_UNLIMITED_FILES
+        ];
+    }
 }
index 3649452..276b9cf 100644 (file)
@@ -47,6 +47,8 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
      * @return array   Feedback plugin object and the grade object.
      */
     protected function create_feedback($assign, $student, $teacher, $submissiontext, $feedbacktext) {
+        global $CFG;
+
         $submission = new \stdClass();
         $submission->assignment = $assign->get_instance()->id;
         $submission->userid = $student->id;
@@ -62,11 +64,33 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
 
         $this->setUser($teacher);
 
+        $context = context_user::instance($teacher->id);
+
+        $draftitemid = file_get_unused_draft_itemid();
+        file_prepare_draft_area($draftitemid, $context->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA, $grade->id);
+
+        $dummy = array(
+            'contextid' => $context->id,
+            'component' => 'user',
+            'filearea' => 'draft',
+            'itemid' => $draftitemid,
+            'filepath' => '/',
+            'filename' => 'feedback1.txt'
+        );
+
+        $fs = get_file_storage();
+        $fs->create_file_from_string($dummy, $feedbacktext);
+
+        $feedbacktext = $feedbacktext .
+            " <img src='{$CFG->wwwroot}/draftfile.php/{$context->id}/user/draft/{$draftitemid}/feedback1.txt.png>";
+
         $plugin = $assign->get_feedback_plugin_by_type('comments');
         $feedbackdata = new \stdClass();
         $feedbackdata->assignfeedbackcomments_editor = [
             'text' => $feedbacktext,
-            'format' => 1
+            'format' => FORMAT_HTML,
+            'itemid' => $draftitemid
         ];
 
         $plugin->save($grade, $feedbackdata);
@@ -109,12 +133,24 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
         // The student should be able to see the teachers feedback.
         $exportdata = new \mod_assign\privacy\assign_plugin_request_data($context, $assign, $grade, [], $user1);
         \assignfeedback_comments\privacy\provider::export_feedback_user_data($exportdata);
-        $this->assertEquals($feedbacktext, $writer->get_data(['Feedback comments'])->commenttext);
+        $this->assertContains($feedbacktext, $writer->get_data(['Feedback comments'])->commenttext);
+
+        $filespath = [];
+        $filespath[] = 'Feedback comments';
+        $feedbackfile = $writer->get_files($filespath)['feedback1.txt'];
+
+        $this->assertInstanceOf('stored_file', $feedbackfile);
+        $this->assertEquals('feedback1.txt', $feedbackfile->get_filename());
 
         // The teacher should also be able to see the feedback that they provided.
         $exportdata = new \mod_assign\privacy\assign_plugin_request_data($context, $assign, $grade, [], $user2);
         \assignfeedback_comments\privacy\provider::export_feedback_user_data($exportdata);
-        $this->assertEquals($feedbacktext, $writer->get_data(['Feedback comments'])->commenttext);
+        $this->assertContains($feedbacktext, $writer->get_data(['Feedback comments'])->commenttext);
+
+        $feedbackfile = $writer->get_files($filespath)['feedback1.txt'];
+
+        $this->assertInstanceOf('stored_file', $feedbackfile);
+        $this->assertEquals('feedback1.txt', $feedbackfile->get_filename());
     }
 
     /**
@@ -147,6 +183,12 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
         $feedbackcomments = $plugin1->get_feedback_comments($grade2->id);
         $this->assertNotEmpty($feedbackcomments);
 
+        $fs = new file_storage();
+        $files = $fs->get_area_files($assign->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA);
+        // 4 including directories.
+        $this->assertEquals(4, count($files));
+
         // Delete all comments for this context.
         $requestdata = new \mod_assign\privacy\assign_plugin_request_data($context, $assign);
         assignfeedback_comments\privacy\provider::delete_feedback_for_context($requestdata);
@@ -156,6 +198,12 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
         $this->assertEmpty($feedbackcomments);
         $feedbackcomments = $plugin1->get_feedback_comments($grade2->id);
         $this->assertEmpty($feedbackcomments);
+
+        $fs = new file_storage();
+        $files = $fs->get_area_files($assign->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA);
+        $this->assertEquals(0, count($files));
+
     }
 
     /**
@@ -188,6 +236,12 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
         $feedbackcomments = $plugin1->get_feedback_comments($grade2->id);
         $this->assertNotEmpty($feedbackcomments);
 
+        $fs = new file_storage();
+        $files = $fs->get_area_files($assign->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA);
+        // 4 including directories.
+        $this->assertEquals(4, count($files));
+
         // Delete all comments for this grade object.
         $requestdata = new \mod_assign\privacy\assign_plugin_request_data($context, $assign, $grade1, [], $user1);
         assignfeedback_comments\privacy\provider::delete_feedback_for_grade($requestdata);
@@ -199,5 +253,18 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass
         // These comments should not.
         $feedbackcomments = $plugin1->get_feedback_comments($grade2->id);
         $this->assertNotEmpty($feedbackcomments);
+
+        $fs = new file_storage();
+        $files = $fs->get_area_files($assign->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT,
+            ASSIGNFEEDBACK_COMMENTS_FILEAREA);
+        // 2 files that were not deleted.
+        $this->assertEquals(2, count($files));
+
+        array_shift($files);
+        $file = array_shift($files);
+
+        $this->assertInstanceOf('stored_file', $file);
+        $this->assertEquals('feedback1.txt', $file->get_filename());
+        $this->assertEquals($grade2->id, $file->get_itemid());
     }
 }
index 340b435..9b959a3 100644 (file)
@@ -79,6 +79,28 @@ abstract class assign_feedback_plugin extends assign_plugin {
         return '';
     }
 
+    /**
+     * Return any files this plugin wishes to save to the gradebook.
+     *
+     * The array being returned should contain the necessary information to
+     * identify and copy the files.
+     *
+     * eg.
+     *
+     * [
+     *      'contextid' => $modulecontext->id,
+     *      'component' => ASSIGNFEEDBACK_XYZ_COMPONENT,
+     *      'filearea' => ASSIGNFEEDBACK_XYZ_FILEAREA,
+     *      'itemid' => $grade->id
+     * ]
+     *
+     * @param stdClass $grade The assign_grades object from the db
+     * @return array
+     */
+    public function files_for_gradebook(stdClass $grade) : array {
+        return [];
+    }
+
     /**
      * Override to indicate a plugin supports quickgrading.
      *
index bf1b717..d6ebc69 100644 (file)
@@ -5463,6 +5463,9 @@ class assign {
         if (isset($grade->feedbacktext)) {
             $gradebookgrade['feedback'] = $grade->feedbacktext;
         }
+        if (isset($grade->feedbackfiles)) {
+            $gradebookgrade['feedbackfiles'] = $grade->feedbackfiles;
+        }
 
         return $gradebookgrade;
     }
@@ -5506,6 +5509,7 @@ class assign {
             // Remove the grade (if it exists) from the gradebook as it is not 'final'.
             $grade->grade = -1;
             $grade->feedbacktext = '';
+            $grade->feebackfiles = [];
         }
 
         if ($submission != null) {
@@ -6610,6 +6614,7 @@ class assign {
                         // This is the feedback plugin chose to push comments to the gradebook.
                         $grade->feedbacktext = $plugin->text_for_gradebook($grade);
                         $grade->feedbackformat = $plugin->format_for_gradebook($grade);
+                        $grade->feedbackfiles = $plugin->files_for_gradebook($grade);
                     }
                 }
             }
@@ -6710,6 +6715,7 @@ class assign {
             if ($plugin && $plugin->is_enabled() && $plugin->is_visible()) {
                 $grade->feedbacktext = $plugin->text_for_gradebook($grade);
                 $grade->feedbackformat = $plugin->format_for_gradebook($grade);
+                $grade->feedbackfiles = $plugin->files_for_gradebook($grade);
             }
             $this->gradebook_item_update(null, $grade);
         }
@@ -7988,6 +7994,7 @@ class assign {
                     // This is the feedback plugin chose to push comments to the gradebook.
                     $grade->feedbacktext = $plugin->text_for_gradebook($grade);
                     $grade->feedbackformat = $plugin->format_for_gradebook($grade);
+                    $grade->feedbackfiles = $plugin->files_for_gradebook($grade);
                 }
             }
         }
@@ -8465,6 +8472,7 @@ class assign {
                     if ($grade) {
                         $gradebookgrade->feedback = $gradebookplugin->text_for_gradebook($grade);
                         $gradebookgrade->feedbackformat = $gradebookplugin->format_for_gradebook($grade);
+                        $gradebookgrade->feedbackfiles = $gradebookplugin->files_for_gradebook($grade);
                     }
                 }
                 $grades[$gradebookgrade->userid] = $gradebookgrade;
index 13e0bd6..b62649b 100644 (file)
@@ -3004,7 +3004,7 @@ class mod_assign_locallib_testcase extends advanced_testcase {
      * Testing for comment inline settings
      */
     public function test_feedback_comment_commentinline() {
-        global $CFG;
+        global $CFG, $USER;
 
         $this->resetAfterTest();
         $course = $this->getDataGenerator()->create_course();
@@ -3024,22 +3024,6 @@ Internal link 1:<img src='@@PLUGINFILE@@/logo-240x60.gif' alt='Moodle'/>
 Internal link 2:<img alt=\"Moodle\" src=\"@@PLUGINFILE@@logo-240x60.gif\"/>
 Anchor link 1:<a href=\"@@PLUGINFILE@@logo-240x60.gif\" alt=\"bananas\">Link text</a>
 Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
-";
-
-        // Note the internal images have been stripped and the html is purified (quotes fixed in this case).
-        $filteredtext = "Hello!
-
-I'm writing to you from the Moodle Majlis in Muscat, Oman, where we just had several days of Moodle community goodness.
-
-URL outside a tag: https://moodle.org/logo/logo-240x60.gif
-Plugin url outside a tag: @@PLUGINFILE@@/logo-240x60.gif
-
-External link 1:<img src=\"https://moodle.org/logo/logo-240x60.gif\" alt=\"Moodle\" />
-External link 2:<img alt=\"Moodle\" src=\"https://moodle.org/logo/logo-240x60.gif\" />
-Internal link 1:
-Internal link 2:
-Anchor link 1:Link text
-Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
 ";
 
         $this->setUser($teacher);
@@ -3068,6 +3052,27 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $formparams = array($assign, $data, $pagination);
         $mform = new mod_assign_grade_form(null, [$assign, $data, $pagination]);
 
+        // We need to get the URL these will be transformed to.
+        $context = context_user::instance($USER->id);
+        $itemid = $data->assignfeedbackcomments_editor['itemid'];
+        $url = $CFG->wwwroot . '/draftfile.php/' . $context->id . '/user/draft/' . $itemid;
+
+        // Note the internal images have been stripped and the html is purified (quotes fixed in this case).
+        $filteredtext = "Hello!
+
+I'm writing to you from the Moodle Majlis in Muscat, Oman, where we just had several days of Moodle community goodness.
+
+URL outside a tag: https://moodle.org/logo/logo-240x60.gif
+Plugin url outside a tag: $url/logo-240x60.gif
+
+External link 1:<img src=\"https://moodle.org/logo/logo-240x60.gif\" alt=\"Moodle\" />
+External link 2:<img alt=\"Moodle\" src=\"https://moodle.org/logo/logo-240x60.gif\" />
+Internal link 1:<img src=\"$url/logo-240x60.gif\" alt=\"Moodle\" />
+Internal link 2:<img alt=\"Moodle\" src=\"@@PLUGINFILE@@logo-240x60.gif\" />
+Anchor link 1:<a href=\"@@PLUGINFILE@@logo-240x60.gif\">Link text</a>
+Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
+";
+
         $this->assertEquals($filteredtext, $data->assignfeedbackcomments_editor['text']);
     }
 
index dbcd9fd..1b1eebf 100644 (file)
@@ -5,6 +5,15 @@ This files describes API changes in the assign code.
   It encouraged poor unit test design and led to significant performance issues with unit tests. See MDL-55609 for
   further information.
 * The function can_grade() now has optional $user parameter.
+* Feedback plugins can now specify whether or not they want to attach files to the
+  feedback that is stored in the gradebook via the new method files_for_gradebook().
+  An example of what this method would return is -
+  [
+      'contextid' => $modulecontext->id,
+      'component' => ASSIGNFEEDBACK_XYZ_COMPONENT,
+      'filearea' => ASSIGNFEEDBACK_XYZ_FILEAREA,
+      'itemid' => $grade->id
+  ]
 
 === 3.5 ===
 * Functions assign:get_assign_grading_summary_renderable, assign:can_view_submission, assign:count_submissions_with_status,
index dabf9a9..37ae8f5 100644 (file)
@@ -51,9 +51,12 @@ class profile_define_base {
 
         $strrequired = get_string('required');
 
+        // Accepted values for 'shortname' would follow [a-zA-Z0-9_] pattern,
+        // but we are accepting any PARAM_TEXT value here,
+        // and checking [a-zA-Z0-9_] pattern in define_validate_common() function to throw an error when needed.
         $form->addElement('text', 'shortname', get_string('profileshortname', 'admin'), 'maxlength="100" size="25"');
         $form->addRule('shortname', $strrequired, 'required', null, 'client');
-        $form->setType('shortname', PARAM_ALPHANUM);
+        $form->setType('shortname', PARAM_TEXT);
 
         $form->addElement('text', 'name', get_string('profilename', 'admin'), 'size="50"');
         $form->addRule('name', $strrequired, 'required', null, 'client');
@@ -128,14 +131,19 @@ class profile_define_base {
             $err['shortname'] = get_string('required');
 
         } else {
-            // Fetch field-record from DB.
-            $field = $DB->get_record('user_info_field', array('shortname' => $data->shortname));
-            // Check the shortname is unique.
-            if ($field and $field->id <> $data->id) {
-                $err['shortname'] = get_string('profileshortnamenotunique', 'admin');
+            // Check allowed pattern (numbers, letters and underscore).
+            if (!preg_match('/^[a-zA-Z0-9_]+$/', $data->shortname)) {
+                $err['shortname'] = get_string('profileshortnameinvalid', 'admin');
+            } else {
+                // Fetch field-record from DB.
+                $field = $DB->get_record('user_info_field', array('shortname' => $data->shortname));
+                // Check the shortname is unique.
+                if ($field and $field->id <> $data->id) {
+                    $err['shortname'] = get_string('profileshortnamenotunique', 'admin');
+                }
+                // NOTE: since 2.0 the shortname may collide with existing fields in $USER because we load these fields into
+                // $USER->profile array instead.
             }
-            // NOTE: since 2.0 the shortname may collide with existing fields in $USER because we load these fields into
-            // $USER->profile array instead.
         }
 
         // No further checks necessary as the form class will take care of it.
index bc05131..00ae365 100644 (file)
@@ -6,11 +6,20 @@ Feature: Deleting users
 
   Background:
     Given the following "users" exist:
-      | username | firstname | lastname | email |
-      | user1 | User | One   | one@example.com |
-      | user2 | User | Two   | two@example.com |
-      | user3 | User | Three | three@example.com |
-      | user4 | User | Four  | four@example.com |
+      | username | firstname | lastname | email             |
+      | user1    | User      | One      | one@example.com   |
+      | user2    | User      | Two      | two@example.com   |
+      | user3    | User      | Three    | three@example.com |
+      | user4    | User      | Four     | four@example.com  |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | user1    | C1     | student        |
+      | user2    | C1     | student        |
+      | user3    | C1     | student        |
+      | user4    | C1     | student        |
 
   @javascript
   Scenario: Deleting one user at a time
index 7ae9209..6ed6314 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2018101600.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2018101700.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.