Merge branch 'MDL-64525' of https://github.com/NeillM/moodle into master
authorSara Arjona <sara@moodle.com>
Tue, 1 Sep 2020 12:17:48 +0000 (14:17 +0200)
committerSara Arjona <sara@moodle.com>
Tue, 1 Sep 2020 12:17:48 +0000 (14:17 +0200)
comment/classes/external.php
comment/comment_ajax.php
comment/lib.php
comment/tests/context_freeze_test.php [new file with mode: 0644]
lang/en/error.php

index 41a8b41..1eb3d8e 100644 (file)
@@ -337,7 +337,7 @@ class core_comment_external extends external_api {
             $args->area      = $commentrecord->commentarea;
             $manager = new comment($args);
 
-            if ($commentrecord->userid != $USER->id && !$manager->can_delete($commentrecord->id)) {
+            if (!$manager->can_delete($commentrecord)) {
                 throw new comment_exception('nopermissiontodelentry');
             }
 
index c1fe37c..50caa94 100644 (file)
@@ -91,8 +91,8 @@ switch ($action) {
         }
         break;
     case 'delete':
-        $comment_record = $DB->get_record('comments', array('id'=>$commentid));
-        if ($manager->can_delete($commentid) || $comment_record->userid == $USER->id) {
+        $comment = $DB->get_record('comments', ['id' => $commentid]);
+        if ($manager->can_delete($comment)) {
             if ($manager->delete($commentid)) {
                 $result = array(
                     'client_id' => $client_id,
index 58f8151..65d81e3 100644 (file)
@@ -589,8 +589,7 @@ class comment {
             $c->avatar = $OUTPUT->user_picture($u, array('size'=>18));
             $c->userid = $u->id;
 
-            $candelete = $this->can_delete($c->id);
-            if (($USER->id == $u->id) || !empty($candelete)) {
+            if ($this->can_delete($c)) {
                 $c->delete = true;
             }
             $comments[] = $c;
@@ -800,16 +799,22 @@ class comment {
     /**
      * Delete a comment
      *
-     * @param  int $commentid
+     * @param  int|stdClass $comment The id of a comment, or a comment record.
      * @return bool
      */
-    public function delete($commentid) {
-        global $DB, $USER;
-        $candelete = has_capability('moodle/comment:delete', $this->context);
-        if (!$comment = $DB->get_record('comments', array('id'=>$commentid))) {
+    public function delete($comment) {
+        global $DB;
+        if (is_object($comment)) {
+            $commentid = $comment->id;
+        } else {
+            $commentid = $comment;
+            $comment = $DB->get_record('comments', ['id' => $commentid]);
+        }
+
+        if (!$comment) {
             throw new comment_exception('dbupdatefailed');
         }
-        if (!($USER->id == $comment->userid || !empty($candelete))) {
+        if (!$this->can_delete($comment)) {
             throw new comment_exception('nopermissiontocomment');
         }
         $DB->delete_records('comments', array('id'=>$commentid));
@@ -976,13 +981,35 @@ class comment {
     }
 
     /**
-     * Returns true if the user can delete this comment
-     * @param int $commentid
+     * Returns true if the user can delete this comment.
+     *
+     * The user can delete comments if it is one they posted and they can still make posts,
+     * or they have the capability to delete comments.
+     *
+     * A database call is avoided if a comment record is passed.
+     *
+     * @param int|stdClass $comment The id of a comment, or a comment record.
      * @return bool
      */
-    public function can_delete($commentid) {
+    public function can_delete($comment) {
+        global $USER, $DB;
+        if (is_object($comment)) {
+            $commentid = $comment->id;
+        } else {
+            $commentid = $comment;
+        }
+
         $this->validate(array('commentid'=>$commentid));
-        return has_capability('moodle/comment:delete', $this->context);
+
+        if (!is_object($comment)) {
+            // Get the comment record from the database.
+            $comment = $DB->get_record('comments', array('id' => $commentid), 'id, userid', MUST_EXIST);
+        }
+
+        $hascapability = has_capability('moodle/comment:delete', $this->context);
+        $owncomment = $USER->id == $comment->userid;
+
+        return ($hascapability || ($owncomment && $this->can_post()));
     }
 
     /**
diff --git a/comment/tests/context_freeze_test.php b/comment/tests/context_freeze_test.php
new file mode 100644 (file)
index 0000000..36f1d60
--- /dev/null
@@ -0,0 +1,166 @@
+<?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/>.
+
+/**
+ * Tests for comments when the context is frozen.
+ *
+ * @package    core_comment
+ * @copyright  2019 University of Nottingham
+ * @author     Neill Magill <neill.magill@nottingham.ac.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Tests for comments when the context is frozen.
+ *
+ * @package    core_comment
+ * @copyright  2019 University of Nottingham
+ * @author     Neill Magill <neill.magill@nottingham.ac.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class comment_context_freeze_testcase extends advanced_testcase {
+    /**
+     * Creates a comment by a student.
+     *
+     * Returns:
+     * - The comment object
+     * - The sudent that wrote the comment
+     * - The arguments used to create the comment
+     *
+     * @param stdClass $course Moodle course from the datagenerator
+     * @return array
+     */
+    protected function create_student_comment_and_freeze_course($course): array {
+        set_config('contextlocking', 1);
+
+        $context = context_course::instance($course->id);
+        $student = $this->getDataGenerator()->create_and_enrol($course, 'student');
+
+        $args = new stdClass;
+        $args->context = $context;
+        $args->course = $course;
+        $args->area = 'page_comments';
+        $args->itemid = 0;
+        $args->component = 'block_comments';
+        $args->linktext = get_string('showcomments');
+        $args->notoggle = true;
+        $args->autostart = true;
+        $args->displaycancel = false;
+
+        // Create a comment by the student.
+        $this->setUser($student);
+        $comment = new comment($args);
+        $newcomment = $comment->add('New comment');
+
+        // Freeze the context.
+        $this->setAdminUser();
+        $context->set_locked(true);
+
+        return [$newcomment, $student, $args];
+    }
+
+    /**
+     * Test that a student cannot delete their own comments in frozen contexts via the external service.
+     */
+    public function test_delete_student_external() {
+        global $CFG;
+        require_once($CFG->dirroot . '/comment/lib.php');
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+
+        list($newcomment, $student, $args) = $this->create_student_comment_and_freeze_course($course);
+
+        // Check that a student cannot delete their own comment.
+        $this->setUser($student);
+        $studentcomment = new comment($args);
+        $this->assertFalse($studentcomment->can_delete($newcomment->id));
+        $this->assertFalse($studentcomment->can_post());
+        $this->expectException(comment_exception::class);
+        $this->expectExceptionMessage(get_string('nopermissiontodelentry', 'error'));
+        core_comment_external::delete_comments([$newcomment->id]);
+    }
+
+    /**
+     * Test that a student cannot delete their own comments in frozen contexts.
+     */
+    public function test_delete_student() {
+        global $CFG;
+        require_once($CFG->dirroot . '/comment/lib.php');
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+
+        list($newcomment, $student, $args) = $this->create_student_comment_and_freeze_course($course);
+
+        // Check that a student cannot delete their own comment.
+        $this->setUser($student);
+        $studentcomment = new comment($args);
+        $this->assertFalse($studentcomment->can_delete($newcomment->id));
+        $this->assertFalse($studentcomment->can_post());
+        $this->expectException(comment_exception::class);
+        $this->expectExceptionMessage(get_string('nopermissiontocomment', 'error'));
+        $studentcomment->delete($newcomment->id);
+    }
+
+    /**
+     * Test that an admin cannot delete comments in frozen contexts via the external service.
+     */
+    public function test_delete_admin_external() {
+        global $CFG;
+        require_once($CFG->dirroot . '/comment/lib.php');
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+
+        list($newcomment, $student, $args) = $this->create_student_comment_and_freeze_course($course);
+
+        // Check that the admin user cannot delete the comment.
+        $admincomment = new comment($args);
+        $this->assertFalse($admincomment->can_delete($newcomment->id));
+        $this->assertFalse($admincomment->can_post());
+        $this->expectException(comment_exception::class);
+        $this->expectExceptionMessage(get_string('nopermissiontodelentry', 'error'));
+        core_comment_external::delete_comments([$newcomment->id]);
+    }
+
+    /**
+     * Test that an admin cannot delete comments in frozen contexts.
+     */
+    public function test_delete_admin() {
+        global $CFG;
+        require_once($CFG->dirroot . '/comment/lib.php');
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+
+        list($newcomment, $student, $args) = $this->create_student_comment_and_freeze_course($course);
+
+        // Check that the admin user cannot delete the comment.
+        $admincomment = new comment($args);
+        $this->assertFalse($admincomment->can_delete($newcomment->id));
+        $this->assertFalse($admincomment->can_post());
+        $this->expectException(comment_exception::class);
+        $this->expectExceptionMessage(get_string('nopermissiontocomment', 'error'));
+        $admincomment->delete($newcomment->id);
+    }
+}
index 6a72879..828a2f5 100644 (file)
@@ -440,7 +440,7 @@ $string['noparticipants'] = 'No participants found for this course';
 $string['noparticipatorycms'] = 'Sorry, but you have no participatory course modules to report on';
 $string['nopermissions'] = 'Sorry, but you do not currently have permissions to do that ({$a}).';
 $string['nopermissiontocomment'] = 'You can\'t add comments';
-$string['nopermissiontodelentry'] = 'You can\'t delete other people\'s entries!';
+$string['nopermissiontodelentry'] = 'You can\'t delete this comment!';
 $string['nopermissiontoeditcomment'] = 'You can\'t edit other people\'s comments!';
 $string['nopermissiontohide'] = 'No permission to hide!';
 $string['nopermissiontoimportact'] = 'You do not have the required permissions to import activities to this course';