From 2234b6c9dc0d2ede23a9a6a37293fcddbf9ca0d0 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 13 Feb 2019 13:15:29 +0100 Subject: [PATCH 1/7] MDL-64588 comment: Return whether the user can post in a comments area --- comment/classes/external.php | 2 ++ comment/tests/externallib_test.php | 1 + comment/upgrade.txt | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/comment/classes/external.php b/comment/classes/external.php index b76483d75b9..22995e7d12e 100644 --- a/comment/classes/external.php +++ b/comment/classes/external.php @@ -131,6 +131,7 @@ class core_comment_external extends external_api { 'comments' => $comments, 'count' => $commentobject->count(), 'perpage' => (!empty($CFG->commentsperpage)) ? $CFG->commentsperpage : 15, + 'canpost' => $commentobject->can_post(), 'warnings' => $warnings ); return $results; @@ -164,6 +165,7 @@ class core_comment_external extends external_api { ), 'count' => new external_value(PARAM_INT, 'Total number of comments.', VALUE_OPTIONAL), 'perpage' => new external_value(PARAM_INT, 'Number of comments per page.', VALUE_OPTIONAL), + 'canpost' => new external_value(PARAM_BOOL, 'Whether the user can post in this comment area.', VALUE_OPTIONAL), 'warnings' => new external_warnings() ) ); diff --git a/comment/tests/externallib_test.php b/comment/tests/externallib_test.php index e9eb0c2b98b..b34a553720b 100644 --- a/comment/tests/externallib_test.php +++ b/comment/tests/externallib_test.php @@ -125,6 +125,7 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->assertCount(2, $result['comments']); $this->assertEquals(2, $result['count']); $this->assertEquals(15, $result['perpage']); + $this->assertTrue($result['canpost']); $this->assertEquals($user->id, $result['comments'][0]['userid']); $this->assertEquals($user->id, $result['comments'][1]['userid']); diff --git a/comment/upgrade.txt b/comment/upgrade.txt index 2510a99b488..551e77d8302 100644 --- a/comment/upgrade.txt +++ b/comment/upgrade.txt @@ -4,4 +4,4 @@ information provided here is intended especially for developers. === 3.8 === * External function get_comments now returns the total count of comments and the number of comments per page. It also has a new parameter to indicate the sorting direction (defaulted to DESC). - + * The Webservice core_comment_get_comments now indicates if the current user can post comments in the requested area. From 8e4a9ed8544af3657cced293ed1faed7fcd762da Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 13 Feb 2019 15:12:33 +0100 Subject: [PATCH 2/7] MDL-64588 comment: New WebService core_comment_add_comment --- comment/classes/external.php | 115 ++++++++++++++ comment/tests/externallib_test.php | 232 +++++++++++++++++++++++++---- lib/db/services.php | 7 + version.php | 2 +- 4 files changed, 327 insertions(+), 29 deletions(-) diff --git a/comment/classes/external.php b/comment/classes/external.php index 22995e7d12e..d19e2c48eba 100644 --- a/comment/classes/external.php +++ b/comment/classes/external.php @@ -170,4 +170,119 @@ class core_comment_external extends external_api { ) ); } + + /** + * Helper to get the structure of a single comment. + * + * @return external_single_structure the comment structure. + */ + protected static function get_comment_structure() { + return new external_single_structure( + array( + 'id' => new external_value(PARAM_INT, 'Comment ID'), + 'content' => new external_value(PARAM_RAW, 'The content text formatted'), + 'format' => new external_format_value('content'), + 'timecreated' => new external_value(PARAM_INT, 'Time created (timestamp)'), + 'strftimeformat' => new external_value(PARAM_NOTAGS, 'Time format'), + 'profileurl' => new external_value(PARAM_URL, 'URL profile'), + 'fullname' => new external_value(PARAM_NOTAGS, 'fullname'), + 'time' => new external_value(PARAM_NOTAGS, 'Time in human format'), + 'avatar' => new external_value(PARAM_RAW, 'HTML user picture'), + 'userid' => new external_value(PARAM_INT, 'User ID'), + 'delete' => new external_value(PARAM_BOOL, 'Permission to delete=true/false', VALUE_OPTIONAL) + ), 'comment' + ); + } + + /** + * Returns description of method parameters for the add_comments method. + * + * @return external_function_parameters + */ + public static function add_comments_parameters() { + return new external_function_parameters( + [ + 'comments' => new external_multiple_structure( + new external_single_structure( + [ + 'contextlevel' => new external_value(PARAM_ALPHA, 'contextlevel system, course, user...'), + 'instanceid' => new external_value(PARAM_INT, 'the id of item associated with the contextlevel'), + 'component' => new external_value(PARAM_COMPONENT, 'component'), + 'content' => new external_value(PARAM_RAW, 'component'), + 'itemid' => new external_value(PARAM_INT, 'associated id'), + 'area' => new external_value(PARAM_AREA, 'string comment area', VALUE_DEFAULT, ''), + ] + ) + ) + ] + ); + } + + /** + * Add a comment or comments. + * + * @param array $comments the array of comments to create. + * @return array the array containing those comments created. + * @throws comment_exception + */ + public static function add_comments($comments) { + global $CFG, $SITE; + + if (empty($CFG->usecomments)) { + throw new comment_exception('commentsnotenabled', 'moodle'); + } + + $params = self::validate_parameters(self::add_comments_parameters(), ['comments' => $comments]); + + // Validate every intended comment before creating anything, storing the validated comment for use below. + foreach ($params['comments'] as $index => $comment) { + $context = self::get_context_from_params($comment); + self::validate_context($context); + + list($context, $course, $cm) = get_context_info_array($context->id); + if ($context->id == SYSCONTEXTID) { + $course = $SITE; + } + + // Initialising comment object. + $args = new stdClass(); + $args->context = $context; + $args->course = $course; + $args->cm = $cm; + $args->component = $comment['component']; + $args->itemid = $comment['itemid']; + $args->area = $comment['area']; + + $manager = new comment($args); + if (!$manager->can_post()) { + throw new comment_exception('nopermissiontocomment'); + } + + $params['comments'][$index]['preparedcomment'] = $manager; + } + + // Create the comments. + $results = []; + foreach ($params['comments'] as $comment) { + $manager = $comment['preparedcomment']; + $newcomment = $manager->add($comment['content']); + if (!empty($newcomment) && is_object($newcomment)) { + $results[] = $newcomment; + } + $newcomment->delete = true; // USER created the comment, so they can delete it. + } + + return $results; + } + + /** + * Returns description of method result value for the add_comments method. + * + * @return external_description + */ + public static function add_comments_returns() { + return new external_multiple_structure( + self::get_comment_structure() + ); + } } diff --git a/comment/tests/externallib_test.php b/comment/tests/externallib_test.php index b34a553720b..b43347967b8 100644 --- a/comment/tests/externallib_test.php +++ b/comment/tests/externallib_test.php @@ -45,34 +45,25 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Tests set up */ protected function setUp() { - global $CFG; + global $CFG, $DB; require_once($CFG->dirroot . '/comment/lib.php'); - } - - /** - * Test get_comments - */ - public function test_get_comments() { - global $DB, $CFG; - - $this->resetAfterTest(true); $CFG->usecomments = true; - $user = $this->getDataGenerator()->create_user(); - $course = $this->getDataGenerator()->create_course(array('enablecomment' => 1)); + $this->student = $this->getDataGenerator()->create_user(); + $this->course = $this->getDataGenerator()->create_course(array('enablecomment' => 1)); $studentrole = $DB->get_record('role', array('shortname' => 'student')); - $this->getDataGenerator()->enrol_user($user->id, $course->id, $studentrole->id); + $this->getDataGenerator()->enrol_user($this->student->id, $this->course->id, $studentrole->id); $record = new stdClass(); - $record->course = $course->id; + $record->course = $this->course->id; $record->name = "Mod data test"; $record->intro = "Some intro of some sort"; $record->comments = 1; - $module = $this->getDataGenerator()->create_module('data', $record); - $field = data_get_field_new('text', $module); + $this->module = $this->getDataGenerator()->create_module('data', $record); + $field = data_get_field_new('text', $this->module); $fielddetail = new stdClass(); $fielddetail->name = 'Name'; @@ -80,28 +71,37 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $field->define_field($fielddetail); $field->insert_field(); - $recordid = data_add_record($module); + $this->recordid = data_add_record($this->module); $datacontent = array(); $datacontent['fieldid'] = $field->field->id; - $datacontent['recordid'] = $recordid; + $datacontent['recordid'] = $this->recordid; $datacontent['content'] = 'Asterix'; $contentid = $DB->insert_record('data_content', $datacontent); - $cm = get_coursemodule_from_instance('data', $module->id, $course->id); + $this->cm = get_coursemodule_from_instance('data', $this->module->id, $this->course->id); - $context = context_module::instance($module->cmid); + $this->context = context_module::instance($this->module->cmid); + } - $this->setUser($user); + /** + * Test get_comments + */ + public function test_get_comments() { + global $DB; + + $this->resetAfterTest(true); + + $this->setUser($this->student); // We need to add the comments manually, the comment API uses the global OUTPUT and this is going to make the WS to fail. $newcmt = new stdClass; - $newcmt->contextid = $context->id; + $newcmt->contextid = $this->context->id; $newcmt->commentarea = 'database_entry'; - $newcmt->itemid = $recordid; + $newcmt->itemid = $this->recordid; $newcmt->content = 'New comment'; $newcmt->format = 0; - $newcmt->userid = $user->id; + $newcmt->userid = $this->student->id; $newcmt->timecreated = time(); $cmtid1 = $DB->insert_record('comments', $newcmt); @@ -110,9 +110,9 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $cmtid2 = $DB->insert_record('comments', $newcmt); $contextlevel = 'module'; - $instanceid = $cm->id; + $instanceid = $this->cm->id; $component = 'mod_data'; - $itemid = $recordid; + $itemid = $this->recordid; $area = 'database_entry'; $page = 0; @@ -127,8 +127,8 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(15, $result['perpage']); $this->assertTrue($result['canpost']); - $this->assertEquals($user->id, $result['comments'][0]['userid']); - $this->assertEquals($user->id, $result['comments'][1]['userid']); + $this->assertEquals($this->student->id, $result['comments'][0]['userid']); + $this->assertEquals($this->student->id, $result['comments'][1]['userid']); $this->assertEquals($cmtid2, $result['comments'][0]['id']); // Default ordering newer first. $this->assertEquals($cmtid1, $result['comments'][1]['id']); @@ -154,4 +154,180 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($CFG->commentsperpage, $result['perpage']); $this->assertEquals($cmtid2, $result['comments'][0]['id']); } + + /** + * Test add_comments not enabled site level + */ + public function test_add_comments_not_enabled_site_level() { + global $CFG; + $this->resetAfterTest(true); + + $CFG->usecomments = false; + $this->setUser($this->student); + $this->expectException(comment_exception::class); + core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]); + } + + /** + * Test add_comments not enabled module level + */ + public function test_add_comments_not_enabled_module_level() { + global $DB; + $this->resetAfterTest(true); + + $DB->set_field('data', 'comments', 0, array('id' => $this->module->id)); + $this->setUser($this->student); + $this->expectException(comment_exception::class); + core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]); + } + + /** + * Test add_comments + */ + public function test_add_comments_single() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + $result = core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]); + $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + + $expectedkeys = [ + 'id', + 'content', + 'format', + 'timecreated', + 'strftimeformat', + 'profileurl', + 'fullname', + 'time', + 'avatar', + 'userid', + 'delete', + ]; + + // Verify the result contains 1 result having the correct structure. + $this->assertCount(1, $result); + foreach ($expectedkeys as $key) { + $this->assertArrayHasKey($key, $result[0]); + } + } + + /** + * Test add_comments when one of the comments contains invalid data and cannot be created. + * + * This simply verifies that the entire operation fails. + */ + public function test_add_comments_multiple_contains_invalid() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + $this->expectException(comment_exception::class); + core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ], + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'areanotfound' + ], + ]); + } + + /** + * Test add_comments when one of the comments contains invalid data and cannot be created. + * + * This simply verifies that the entire operation fails. + */ + public function test_add_comments_multiple_all_valid() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + $inputdata = [ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'cat', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ], + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'dog', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]; + $result = core_comment_external::add_comments($inputdata); + $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + + // Two comments should have been created. + $this->assertCount(2, $result); + + // The content for each comment should come back formatted. + foreach ($result as $index => $comment) { + $formatoptions = array('overflowdiv' => true, 'blanktarget' => true); + $expectedcontent = format_text($inputdata[$index]['content'], FORMAT_MOODLE, $formatoptions); + $this->assertEquals($expectedcontent, $comment['content']); + } + } + + /** + * Test add_comments invalid area + */ + public function test_add_comments_invalid_area() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + $comments = [ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'rhomboid' + ] + ]; + $this->expectException(comment_exception::class); + core_comment_external::add_comments($comments); + } } diff --git a/lib/db/services.php b/lib/db/services.php index fd519a10501..7b3cf3edf66 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -334,6 +334,13 @@ $functions = array( 'capabilities' => 'moodle/comment:view', 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), ), + 'core_comment_add_comments' => array( + 'classname' => 'core_comment_external', + 'methodname' => 'add_comments', + 'description' => 'Adds a comment or comments.', + 'type' => 'write', + 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), + ), 'core_completion_get_activities_completion_status' => array( 'classname' => 'core_completion_external', 'methodname' => 'get_activities_completion_status', diff --git a/version.php b/version.php index 32852abd11a..00e2a0063c4 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019092700.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019092700.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 09899abc55c6eccb96d61830cf77402d18978059 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 13 Feb 2019 16:04:21 +0100 Subject: [PATCH 3/7] MDL-64588 comment: New WebService core_comment_delete_comment --- comment/classes/external.php | 87 +++++++++++++++++++++ comment/tests/externallib_test.php | 118 ++++++++++++++++++++++++++++- lib/db/services.php | 7 ++ 3 files changed, 210 insertions(+), 2 deletions(-) diff --git a/comment/classes/external.php b/comment/classes/external.php index d19e2c48eba..8bbf7f84bdb 100644 --- a/comment/classes/external.php +++ b/comment/classes/external.php @@ -285,4 +285,91 @@ class core_comment_external extends external_api { self::get_comment_structure() ); } + + /** + * Returns description of method parameters for the delete_comments() method. + * + * @return external_function_parameters + */ + public static function delete_comments_parameters() { + return new external_function_parameters( + [ + 'comments' => new external_multiple_structure( + new external_value(PARAM_INT, 'id of the comment', VALUE_DEFAULT, 0) + ) + ] + ); + } + + /** + * Deletes a comment or comments. + * + * @param array $comments array of comment ids to be deleted + * @return array + * @throws comment_exception + */ + public static function delete_comments(array $comments) { + global $CFG, $DB, $USER, $SITE; + + if (empty($CFG->usecomments)) { + throw new comment_exception('commentsnotenabled', 'moodle'); + } + + $params = self::validate_parameters(self::delete_comments_parameters(), ['comments' => $comments]); + $commentids = $params['comments']; + + list($insql, $inparams) = $DB->get_in_or_equal($commentids); + $commentrecords = $DB->get_records_select('comments', "id {$insql}", $inparams); + + // If one or more of the records could not be found, report this and fail early. + if (count($commentrecords) != count($comments)) { + $invalidcomments = array_diff($commentids, array_column($commentrecords, 'id')); + $invalidcommentsstr = implode(',', $invalidcomments); + throw new comment_exception("One or more comments could not be found by id: $invalidcommentsstr"); + } + + // Make sure we can delete every one of the comments before actually doing so. + $comments = []; // Holds the comment objects, for later deletion. + foreach ($commentrecords as $commentrecord) { + // Validate the context. + list($context, $course, $cm) = get_context_info_array($commentrecord->contextid); + if ($context->id == SYSCONTEXTID) { + $course = $SITE; + } + self::validate_context($context); + + // Make sure the user is allowed to delete the comment. + $args = new stdClass; + $args->context = $context; + $args->course = $course; + $args->cm = $cm; + $args->component = $commentrecord->component; + $args->itemid = $commentrecord->itemid; + $args->area = $commentrecord->commentarea; + $manager = new comment($args); + + if ($commentrecord->userid != $USER->id && !$manager->can_delete($commentrecord->id)) { + throw new comment_exception('nopermissiontodelentry'); + } + + // User is allowed to delete it, so store the comment object, for use below in final deletion. + $comments[$commentrecord->id] = $manager; + } + + // All comments can be deleted by the user. Make it so. + foreach ($comments as $commentid => $comment) { + $comment->delete($commentid); + } + + return []; + } + + /** + * Returns description of method result value for the delete_comments() method. + * + * @return external_description + */ + public static function delete_comments_returns() { + return new external_warnings(); + } } diff --git a/comment/tests/externallib_test.php b/comment/tests/externallib_test.php index b43347967b8..fd7a4b49062 100644 --- a/comment/tests/externallib_test.php +++ b/comment/tests/externallib_test.php @@ -53,8 +53,8 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->student = $this->getDataGenerator()->create_user(); $this->course = $this->getDataGenerator()->create_course(array('enablecomment' => 1)); - $studentrole = $DB->get_record('role', array('shortname' => 'student')); - $this->getDataGenerator()->enrol_user($this->student->id, $this->course->id, $studentrole->id); + $this->studentrole = $DB->get_record('role', array('shortname' => 'student')); + $this->getDataGenerator()->enrol_user($this->student->id, $this->course->id, $this->studentrole->id); $record = new stdClass(); $record->course = $this->course->id; @@ -330,4 +330,118 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->expectException(comment_exception::class); core_comment_external::add_comments($comments); } + + /** + * Test delete_comment invalid comment. + */ + public function test_delete_comments_invalid_comments() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + $this->expectException(comment_exception::class); + core_comment_external::delete_comments([-1, 0]); + } + + /** + * Test delete_comment own user. + */ + public function test_delete_comments_own_user() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + // Create a few comments. + $result = core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ], + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'def', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]); + $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + + + // Delete those comments we just created. + $result = core_comment_external::delete_comments([$result[0]['id'], $result[1]['id']]); + $result = external_api::clean_returnvalue(core_comment_external::delete_comments_returns(), $result); + $this->assertEquals([], $result); + } + + /** + * Test delete_comment other student. + */ + public function test_delete_comment_other_student() { + $this->resetAfterTest(true); + $this->setUser($this->student); + + // Create a comment as student 1. + $result = core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]); + $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + + $this->assertNotEquals(0, $result[0]['id']); + + // Create another student. + $otherstudent = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($otherstudent->id, $this->course->id, $this->studentrole->id); + + $this->setUser($otherstudent); + $this->expectException(comment_exception::class); + core_comment_external::delete_comments([$result[0]['id']]); + } + + /** + * Test delete_comment as teacher. + */ + public function test_delete_comments_as_teacher() { + global $DB; + $this->resetAfterTest(true); + $this->setUser($this->student); + + $result = core_comment_external::add_comments([ + [ + 'contextlevel' => 'module', + 'instanceid' => $this->cm->id, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $this->recordid, + 'area' => 'database_entry' + ] + ]); + $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + + $this->assertNotEquals(0, $result[0]['id']); + + // Create teacher. + $teacher = $this->getDataGenerator()->create_user(); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + $this->getDataGenerator()->enrol_user($teacher->id, $this->course->id, $teacherrole->id); + + $this->setUser($teacher); + $result = external_api::clean_returnvalue( + core_comment_external::delete_comments_returns(), + core_comment_external::delete_comments([$result[0]['id']]) + ); + + $this->assertEquals([], $result); + + } } diff --git a/lib/db/services.php b/lib/db/services.php index 7b3cf3edf66..28402189063 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -341,6 +341,13 @@ $functions = array( 'type' => 'write', 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), ), + 'core_comment_delete_comments' => array( + 'classname' => 'core_comment_external', + 'methodname' => 'delete_comments', + 'description' => 'Deletes a comment or comments.', + 'type' => 'write', + 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), + ), 'core_completion_get_activities_completion_status' => array( 'classname' => 'core_completion_external', 'methodname' => 'get_activities_completion_status', From 6cd3d398a9c29e9fd6e7d7a85ee4e6b7f2c9f25e Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Mon, 18 Mar 2019 08:53:19 +0800 Subject: [PATCH 4/7] MDL-64588 core_comment: use comment structure in external get_comments Other minor changes include: - added the since tag to newly added external functions - Changed 'web service' to 'external function' in comment/upgrade.txt --- comment/classes/external.php | 22 +++++++--------------- comment/upgrade.txt | 3 ++- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/comment/classes/external.php b/comment/classes/external.php index 8bbf7f84bdb..dd3afbdb82d 100644 --- a/comment/classes/external.php +++ b/comment/classes/external.php @@ -147,21 +147,7 @@ class core_comment_external extends external_api { return new external_single_structure( array( 'comments' => new external_multiple_structure( - new external_single_structure( - array( - 'id' => new external_value(PARAM_INT, 'Comment ID'), - 'content' => new external_value(PARAM_RAW, 'The content text formated'), - 'format' => new external_format_value('content'), - 'timecreated' => new external_value(PARAM_INT, 'Time created (timestamp)'), - 'strftimeformat' => new external_value(PARAM_NOTAGS, 'Time format'), - 'profileurl' => new external_value(PARAM_URL, 'URL profile'), - 'fullname' => new external_value(PARAM_NOTAGS, 'fullname'), - 'time' => new external_value(PARAM_NOTAGS, 'Time in human format'), - 'avatar' => new external_value(PARAM_RAW, 'HTML user picture'), - 'userid' => new external_value(PARAM_INT, 'User ID'), - 'delete' => new external_value(PARAM_BOOL, 'Permission to delete=true/false', VALUE_OPTIONAL) - ), 'comment' - ), 'List of comments' + self::get_comment_structure(), 'List of comments' ), 'count' => new external_value(PARAM_INT, 'Total number of comments.', VALUE_OPTIONAL), 'perpage' => new external_value(PARAM_INT, 'Number of comments per page.', VALUE_OPTIONAL), @@ -198,6 +184,7 @@ class core_comment_external extends external_api { * Returns description of method parameters for the add_comments method. * * @return external_function_parameters + * @since Moodle 3.8 */ public static function add_comments_parameters() { return new external_function_parameters( @@ -224,6 +211,7 @@ class core_comment_external extends external_api { * @param array $comments the array of comments to create. * @return array the array containing those comments created. * @throws comment_exception + * @since Moodle 3.8 */ public static function add_comments($comments) { global $CFG, $SITE; @@ -279,6 +267,7 @@ class core_comment_external extends external_api { * Returns description of method result value for the add_comments method. * * @return external_description + * @since Moodle 3.8 */ public static function add_comments_returns() { return new external_multiple_structure( @@ -290,6 +279,7 @@ class core_comment_external extends external_api { * Returns description of method parameters for the delete_comments() method. * * @return external_function_parameters + * @since Moodle 3.8 */ public static function delete_comments_parameters() { return new external_function_parameters( @@ -307,6 +297,7 @@ class core_comment_external extends external_api { * @param array $comments array of comment ids to be deleted * @return array * @throws comment_exception + * @since Moodle 3.8 */ public static function delete_comments(array $comments) { global $CFG, $DB, $USER, $SITE; @@ -368,6 +359,7 @@ class core_comment_external extends external_api { * Returns description of method result value for the delete_comments() method. * * @return external_description + * @since Moodle 3.8 */ public static function delete_comments_returns() { return new external_warnings(); diff --git a/comment/upgrade.txt b/comment/upgrade.txt index 551e77d8302..029e8a3b6f7 100644 --- a/comment/upgrade.txt +++ b/comment/upgrade.txt @@ -4,4 +4,5 @@ information provided here is intended especially for developers. === 3.8 === * External function get_comments now returns the total count of comments and the number of comments per page. It also has a new parameter to indicate the sorting direction (defaulted to DESC). - * The Webservice core_comment_get_comments now indicates if the current user can post comments in the requested area. + * The external function core_comment_get_comments now indicates if the current user can post comments in the requested + area. From ae94d477a64540304b5e381a7d4067122d5f13e6 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Tue, 17 Sep 2019 11:26:45 +0800 Subject: [PATCH 5/7] MDL-64588 core_comment: fix get_comment ordering when timestamps match Include id in the sorting, to be sure that we always get the correct record in cases where the comment timestamps are the same. --- comment/lib.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/comment/lib.php b/comment/lib.php index 463fa003258..58f8151d380 100644 --- a/comment/lib.php +++ b/comment/lib.php @@ -566,7 +566,7 @@ class comment { c.commentarea = :commentarea AND c.itemid = :itemid AND $componentwhere - ORDER BY c.timecreated $sortdirection"; + ORDER BY c.timecreated $sortdirection, c.id $sortdirection"; $params['contextid'] = $this->contextid; $params['commentarea'] = $this->commentarea; $params['itemid'] = $this->itemid; From 93ea6612bd553afc01db7898ff3d6e0fbbfb6716 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Mon, 18 Mar 2019 14:21:45 +0800 Subject: [PATCH 6/7] MDL-64588 core_comment: make external test code use helper method Instead of using setUp to create testing objects, use a helper. --- comment/tests/externallib_test.php | 263 +++++++++++++++-------------- 1 file changed, 138 insertions(+), 125 deletions(-) diff --git a/comment/tests/externallib_test.php b/comment/tests/externallib_test.php index fd7a4b49062..b9473923d54 100644 --- a/comment/tests/externallib_test.php +++ b/comment/tests/externallib_test.php @@ -45,25 +45,40 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Tests set up */ protected function setUp() { + $this->resetAfterTest(); + } + + /** + * Helper used to set up a course, with a module, a teacher and two students. + * + * @return array the array of records corresponding to the course, teacher, and students. + */ + protected function setup_course_and_users_basic() { global $CFG, $DB; require_once($CFG->dirroot . '/comment/lib.php'); $CFG->usecomments = true; - $this->student = $this->getDataGenerator()->create_user(); - $this->course = $this->getDataGenerator()->create_course(array('enablecomment' => 1)); - $this->studentrole = $DB->get_record('role', array('shortname' => 'student')); - $this->getDataGenerator()->enrol_user($this->student->id, $this->course->id, $this->studentrole->id); + $student1 = $this->getDataGenerator()->create_user(); + $student2 = $this->getDataGenerator()->create_user(); + $teacher1 = $this->getDataGenerator()->create_user(); + $course1 = $this->getDataGenerator()->create_course(array('enablecomment' => 1)); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + $this->getDataGenerator()->enrol_user($student1->id, $course1->id, $studentrole->id); + $this->getDataGenerator()->enrol_user($student2->id, $course1->id, $studentrole->id); + $this->getDataGenerator()->enrol_user($teacher1->id, $course1->id, $teacherrole->id); + // Create a database module instance. $record = new stdClass(); - $record->course = $this->course->id; - $record->name = "Mod data test"; + $record->course = $course1->id; + $record->name = "Mod data test"; $record->intro = "Some intro of some sort"; $record->comments = 1; - $this->module = $this->getDataGenerator()->create_module('data', $record); - $field = data_get_field_new('text', $this->module); + $module1 = $this->getDataGenerator()->create_module('data', $record); + $field = data_get_field_new('text', $module1); $fielddetail = new stdClass(); $fielddetail->name = 'Name'; @@ -71,55 +86,57 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $field->define_field($fielddetail); $field->insert_field(); - $this->recordid = data_add_record($this->module); + $recordid = data_add_record($module1); $datacontent = array(); $datacontent['fieldid'] = $field->field->id; - $datacontent['recordid'] = $this->recordid; + $datacontent['recordid'] = $recordid; $datacontent['content'] = 'Asterix'; + $DB->insert_record('data_content', $datacontent); - $contentid = $DB->insert_record('data_content', $datacontent); - $this->cm = get_coursemodule_from_instance('data', $this->module->id, $this->course->id); - - $this->context = context_module::instance($this->module->cmid); + return [$module1, $recordid, $teacher1, $student1, $student2]; } /** * Test get_comments */ public function test_get_comments() { - global $DB; + global $CFG; + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); - $this->resetAfterTest(true); - - $this->setUser($this->student); - - // We need to add the comments manually, the comment API uses the global OUTPUT and this is going to make the WS to fail. - $newcmt = new stdClass; - $newcmt->contextid = $this->context->id; - $newcmt->commentarea = 'database_entry'; - $newcmt->itemid = $this->recordid; - $newcmt->content = 'New comment'; - $newcmt->format = 0; - $newcmt->userid = $this->student->id; - $newcmt->timecreated = time(); - $cmtid1 = $DB->insert_record('comments', $newcmt); - - $newcmt->content = 'New comment 2'; - $newcmt->timecreated = time() + 1; - $cmtid2 = $DB->insert_record('comments', $newcmt); + // Create some comments as student 1. + $this->setUser($student1); + $inputdata = [ + [ + 'contextlevel' => 'module', + 'instanceid' => $module1->cmid, + 'component' => 'mod_data', + 'content' => 'abc', + 'itemid' => $recordid, + 'area' => 'database_entry' + ], + [ + 'contextlevel' => 'module', + 'instanceid' => $module1->cmid, + 'component' => 'mod_data', + 'content' => 'def', + 'itemid' => $recordid, + 'area' => 'database_entry' + ] + ]; + $result = core_comment_external::add_comments($inputdata); + $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + $ids = array_column($result, 'id'); + // Verify we can get the comments. $contextlevel = 'module'; - $instanceid = $this->cm->id; + $instanceid = $module1->cmid; $component = 'mod_data'; - $itemid = $this->recordid; + $itemid = $recordid; $area = 'database_entry'; $page = 0; - $result = core_comment_external::get_comments($contextlevel, $instanceid, $component, $itemid, $area, $page); - // We need to execute the return values cleaning process to simulate the web service server. - $result = external_api::clean_returnvalue( - core_comment_external::get_comments_returns(), $result); + $result = external_api::clean_returnvalue(core_comment_external::get_comments_returns(), $result); $this->assertCount(0, $result['warnings']); $this->assertCount(2, $result['comments']); @@ -127,11 +144,11 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(15, $result['perpage']); $this->assertTrue($result['canpost']); - $this->assertEquals($this->student->id, $result['comments'][0]['userid']); - $this->assertEquals($this->student->id, $result['comments'][1]['userid']); + $this->assertEquals($student1->id, $result['comments'][0]['userid']); + $this->assertEquals($student1->id, $result['comments'][1]['userid']); - $this->assertEquals($cmtid2, $result['comments'][0]['id']); // Default ordering newer first. - $this->assertEquals($cmtid1, $result['comments'][1]['id']); + $this->assertEquals($ids[1], $result['comments'][0]['id']); // Default ordering newer first. + $this->assertEquals($ids[0], $result['comments'][1]['id']); // Test sort direction and pagination. $CFG->commentsperpage = 1; @@ -142,7 +159,7 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->assertCount(1, $result['comments']); // Only one per page. $this->assertEquals(2, $result['count']); $this->assertEquals($CFG->commentsperpage, $result['perpage']); - $this->assertEquals($cmtid1, $result['comments'][0]['id']); // Comments order older first. + $this->assertEquals($ids[0], $result['comments'][0]['id']); // Comments order older first. // Next page. $result = core_comment_external::get_comments($contextlevel, $instanceid, $component, $itemid, $area, $page + 1, 'ASC'); @@ -152,7 +169,7 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { $this->assertCount(1, $result['comments']); $this->assertEquals(2, $result['count']); $this->assertEquals($CFG->commentsperpage, $result['perpage']); - $this->assertEquals($cmtid2, $result['comments'][0]['id']); + $this->assertEquals($ids[1], $result['comments'][0]['id']); } /** @@ -160,18 +177,20 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { */ public function test_add_comments_not_enabled_site_level() { global $CFG; - $this->resetAfterTest(true); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + // Try to add a comment, as student 1, when comments is disabled at site level. + $this->setUser($student1); $CFG->usecomments = false; - $this->setUser($this->student); + $this->expectException(comment_exception::class); core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ] ]); @@ -182,18 +201,21 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { */ public function test_add_comments_not_enabled_module_level() { global $DB; - $this->resetAfterTest(true); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); - $DB->set_field('data', 'comments', 0, array('id' => $this->module->id)); - $this->setUser($this->student); + // Disable comments for the module. + $DB->set_field('data', 'comments', 0, array('id' => $module1->id)); + + // Verify we can't add a comment. + $this->setUser($student1); $this->expectException(comment_exception::class); core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ] ]); @@ -203,21 +225,25 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Test add_comments */ public function test_add_comments_single() { - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + // Add a comment as student 1. + $this->setUser($student1); $result = core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ] ]); $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); + // Verify the result contains 1 result having the correct structure. + $this->assertCount(1, $result); + $expectedkeys = [ 'id', 'content', @@ -231,9 +257,6 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { 'userid', 'delete', ]; - - // Verify the result contains 1 result having the correct structure. - $this->assertCount(1, $result); foreach ($expectedkeys as $key) { $this->assertArrayHasKey($key, $result[0]); } @@ -245,26 +268,27 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * This simply verifies that the entire operation fails. */ public function test_add_comments_multiple_contains_invalid() { - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + // Try to create some comments as student 1, but provide a bad area for the second comment. + $this->setUser($student1); $this->expectException(comment_exception::class); core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ], [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', - 'content' => 'abc', - 'itemid' => $this->recordid, - 'area' => 'areanotfound' + 'content' => 'def', + 'itemid' => $recordid, + 'area' => 'badarea' ], ]); } @@ -275,24 +299,25 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * This simply verifies that the entire operation fails. */ public function test_add_comments_multiple_all_valid() { - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + // Try to create some comments as student 1. + $this->setUser($student1); $inputdata = [ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', - 'content' => 'cat', - 'itemid' => $this->recordid, + 'content' => 'abc', + 'itemid' => $recordid, 'area' => 'database_entry' ], [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', - 'content' => 'dog', - 'itemid' => $this->recordid, + 'content' => 'def', + 'itemid' => $recordid, 'area' => 'database_entry' ] ]; @@ -314,17 +339,18 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Test add_comments invalid area */ public function test_add_comments_invalid_area() { - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + // Try to create a comment with an invalid area, verifying failure. + $this->setUser($student1); $comments = [ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, - 'area' => 'rhomboid' + 'itemid' => $recordid, + 'area' => 'spaghetti' ] ]; $this->expectException(comment_exception::class); @@ -334,9 +360,9 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { /** * Test delete_comment invalid comment. */ - public function test_delete_comments_invalid_comments() { - $this->resetAfterTest(true); - $this->setUser($this->student); + public function test_delete_comments_invalid_comment_id() { + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + $this->setUser($student1); $this->expectException(comment_exception::class); core_comment_external::delete_comments([-1, 0]); @@ -346,33 +372,35 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Test delete_comment own user. */ public function test_delete_comments_own_user() { - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); - // Create a few comments. + // Create a few comments as student 1. + $this->setUser($student1); $result = core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ], [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'def', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ] ]); $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); - // Delete those comments we just created. - $result = core_comment_external::delete_comments([$result[0]['id'], $result[1]['id']]); + $result = core_comment_external::delete_comments([ + $result[0]['id'], + $result[1]['id'] + ]); $result = external_api::clean_returnvalue(core_comment_external::delete_comments_returns(), $result); $this->assertEquals([], $result); } @@ -381,29 +409,24 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Test delete_comment other student. */ public function test_delete_comment_other_student() { - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); - // Create a comment as student 1. + // Create a comment as the student. + $this->setUser($student1); $result = core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ] ]); $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); - $this->assertNotEquals(0, $result[0]['id']); - - // Create another student. - $otherstudent = $this->getDataGenerator()->create_user(); - $this->getDataGenerator()->enrol_user($otherstudent->id, $this->course->id, $this->studentrole->id); - - $this->setUser($otherstudent); + // Now, as student 2, try to delete the comment made by student 1. Verify we can't. + $this->setUser($student2); $this->expectException(comment_exception::class); core_comment_external::delete_comments([$result[0]['id']]); } @@ -412,36 +435,26 @@ class core_comment_externallib_testcase extends externallib_advanced_testcase { * Test delete_comment as teacher. */ public function test_delete_comments_as_teacher() { - global $DB; - $this->resetAfterTest(true); - $this->setUser($this->student); + [$module1, $recordid, $teacher1, $student1, $student2] = $this->setup_course_and_users_basic(); + // Create a comment as the student. + $this->setUser($student1); $result = core_comment_external::add_comments([ [ 'contextlevel' => 'module', - 'instanceid' => $this->cm->id, + 'instanceid' => $module1->cmid, 'component' => 'mod_data', 'content' => 'abc', - 'itemid' => $this->recordid, + 'itemid' => $recordid, 'area' => 'database_entry' ] ]); $result = external_api::clean_returnvalue(core_comment_external::add_comments_returns(), $result); - $this->assertNotEquals(0, $result[0]['id']); - - // Create teacher. - $teacher = $this->getDataGenerator()->create_user(); - $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); - $this->getDataGenerator()->enrol_user($teacher->id, $this->course->id, $teacherrole->id); - - $this->setUser($teacher); - $result = external_api::clean_returnvalue( - core_comment_external::delete_comments_returns(), - core_comment_external::delete_comments([$result[0]['id']]) - ); - + // Verify teachers can delete the comment. + $this->setUser($teacher1); + $result = core_comment_external::delete_comments([$result[0]['id']]); + $result = external_api::clean_returnvalue(core_comment_external::delete_comments_returns(), $result); $this->assertEquals([], $result); - } } From 0c3eaf9ee68e353e916792566be7c80e50e91e1f Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Tue, 17 Sep 2019 11:43:22 +0800 Subject: [PATCH 7/7] MDL-64588 core_comment: fix unnecessary type check in add_comments We should rely on the return type, else an exception with be thrown. --- comment/classes/external.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/comment/classes/external.php b/comment/classes/external.php index dd3afbdb82d..41a8b4108bc 100644 --- a/comment/classes/external.php +++ b/comment/classes/external.php @@ -254,10 +254,8 @@ class core_comment_external extends external_api { foreach ($params['comments'] as $comment) { $manager = $comment['preparedcomment']; $newcomment = $manager->add($comment['content']); - if (!empty($newcomment) && is_object($newcomment)) { - $results[] = $newcomment; - } $newcomment->delete = true; // USER created the comment, so they can delete it. + $results[] = $newcomment; } return $results;