diff --git a/mod/forum/classes/local/managers/capability.php b/mod/forum/classes/local/managers/capability.php index b1fd17f886f..42bcda25df2 100644 --- a/mod/forum/classes/local/managers/capability.php +++ b/mod/forum/classes/local/managers/capability.php @@ -36,6 +36,7 @@ use mod_forum\subscriptions; use context; use context_system; use stdClass; +use moodle_exception; require_once($CFG->dirroot . '/mod/forum/lib.php'); @@ -430,28 +431,63 @@ class capability { } /** - * Can the user delete the post in this discussion? + * Verifies is the given user can delete a post. * * @param stdClass $user The user to check * @param discussion_entity $discussion The discussion to check * @param post_entity $post The post the user wants to delete + * @param bool $hasreplies Whether the post has replies * @return bool + * @throws moodle_exception */ - public function can_delete_post(stdClass $user, discussion_entity $discussion, post_entity $post) : bool { + public function validate_delete_post(stdClass $user, discussion_entity $discussion, post_entity $post, + bool $hasreplies = false) : void { global $CFG; $forum = $this->get_forum(); if ($forum->get_type() == 'single' && $discussion->is_first_post($post)) { // Do not allow deleting of first post in single simple type. - return false; - } else { - $context = $this->get_context(); - $ownpost = $post->is_owned_by_user($user); - $ineditingtime = $post->get_age() < $CFG->maxeditingtime; + throw new moodle_exception('cannotdeletepost', 'forum'); + } - return ($ownpost && $ineditingtime && has_capability('mod/forum:deleteownpost', $context, $user)) || - has_capability('mod/forum:deleteanypost', $context, $user); + $context = $this->get_context(); + $ownpost = $post->is_owned_by_user($user); + $ineditingtime = $post->get_age() < $CFG->maxeditingtime; + + if (!($ownpost && $ineditingtime && has_capability('mod/forum:deleteownpost', $context, $user) || + has_capability('mod/forum:deleteanypost', $context, $user))) { + + throw new moodle_exception('cannotdeletepost', 'forum'); + } + + if ($post->get_total_score()) { + throw new moodle_exception('couldnotdeleteratings', 'rating'); + } + + if ($hasreplies && !has_capability('mod/forum:deleteanypost', $context, $user)) { + throw new moodle_exception('couldnotdeletereplies', 'forum'); + } + } + + + /** + * Can the user delete the post in this discussion? + * + * @param stdClass $user The user to check + * @param discussion_entity $discussion The discussion to check + * @param post_entity $post The post the user wants to delete + * @param bool $hasreplies Whether the post has replies + * @return bool + */ + public function can_delete_post(stdClass $user, discussion_entity $discussion, post_entity $post, + bool $hasreplies = false) : bool { + + try { + $this->validate_delete_post($user, $discussion, $post, $hasreplies); + return true; + } catch (moodle_exception $e) { + return false; } } diff --git a/mod/forum/lang/en/deprecated.txt b/mod/forum/lang/en/deprecated.txt index cddc5b16fd3..14723276ea8 100644 --- a/mod/forum/lang/en/deprecated.txt +++ b/mod/forum/lang/en/deprecated.txt @@ -1,3 +1,4 @@ +cannotdeletediscussioninsinglediscussion,mod_forum inpagereplysubject,mod_forum overviewnumpostssince,mod_forum overviewnumunread,mod_forum diff --git a/mod/forum/lang/en/forum.php b/mod/forum/lang/en/forum.php index da3424c2451..237c5c7f02a 100644 --- a/mod/forum/lang/en/forum.php +++ b/mod/forum/lang/en/forum.php @@ -71,7 +71,6 @@ $string['cannotaddteacherforumto'] = 'Could not add converted teacher forum inst $string['cannotcreatediscussion'] = 'Could not create new discussion'; $string['cannotcreateinstanceforteacher'] = 'Could not create new course module instance for the teacher forum'; $string['cannotdeletepost'] = 'You can\'t delete this post!'; -$string['cannotdeletediscussioninsinglediscussion'] = 'You cannot delete the first post in a single discussion'; $string['cannoteditposts'] = 'You can\'t edit other people\'s posts!'; $string['cannotexportforum'] = 'You cannot export this forum'; $string['cannotfinddiscussion'] = 'Could not find the discussion in this forum'; @@ -704,6 +703,7 @@ $string['forumsubjectdeleted'] = 'This forum post has been removed'; $string['forumbodydeleted'] = 'The content of this forum post has been removed and can no longer be accessed.'; // Deprecated since Moodle 3.8. +$string['cannotdeletediscussioninsinglediscussion'] = 'You cannot delete the first post in a single discussion'; $string['inpagereplysubject'] = 'Re: {$a}'; $string['overviewnumpostssince'] = '{$a} posts since last login'; $string['overviewnumunread'] = '{$a} total unread'; diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 0f8902c2089..546c5079189 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -3173,6 +3173,17 @@ function forum_delete_discussion($discussion, $fulldelete, $course, $cm, $forum) } } + $params = array( + 'objectid' => $discussion->id, + 'context' => context_module::instance($cm->id), + 'other' => array( + 'forumid' => $forum->id, + ) + ); + $event = \mod_forum\event\discussion_deleted::create($params); + $event->add_record_snapshot('forum_discussions', $discussion); + $event->trigger(); + return $result; } diff --git a/mod/forum/post.php b/mod/forum/post.php index 342326c44c1..2b9501392b5 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -369,94 +369,47 @@ if (!empty($forum)) { } $capabilitymanager = $managerfactory->get_capability_manager($forumentity); - $post = $postdatamapper->to_legacy_object($postentity); - $discussion = $discussiondatamapper->to_legacy_object($discussionentity); - $forum = $forumdatamapper->to_legacy_object($forumentity); $course = $forumentity->get_course_record(); + $cm = $forumentity->get_course_module_record(); $modcontext = $forumentity->get_context(); - $coursecontext = context_course::instance($course->id); - - if (!$cm = get_coursemodule_from_instance("forum", $forum->id, $course->id)) { - print_error('invalidcoursemodule'); - } require_login($course, false, $cm); - if (!$capabilitymanager->can_delete_post($USER, $discussionentity, $postentity)) { - redirect( - $urlfactory->get_discussion_view_url_from_discussion($discussionentity), - get_string('cannotdeletepost', 'forum'), - null, - \core\output\notification::NOTIFY_ERROR - ); - } - $replycount = $postvault->get_reply_count_for_post_id_in_discussion_id( $USER, $postentity->get_id(), $discussionentity->get_id(), true); if (!empty($confirm) && confirm_sesskey()) { - // User has confirmed the delete. - // Check user capability to delete post. - $timepassed = time() - $post->created; - if ($post->totalscore) { - redirect( - $urlfactory->get_discussion_view_url_from_discussion($discussionentity), - get_string('couldnotdeleteratings', 'rating'), - null, - \core\output\notification::NOTIFY_ERROR - ); - } else if ($replycount && !has_capability('mod/forum:deleteanypost', $modcontext)) { - redirect( - $urlfactory->get_discussion_view_url_from_discussion($discussionentity), - get_string('couldnotdeletereplies', 'forum'), - null, - \core\output\notification::NOTIFY_ERROR - ); - } else { + // Do further checks and delete the post. + $hasreplies = $replycount > 0; + + try { + $capabilitymanager->validate_delete_post($USER, $discussionentity, $postentity, $hasreplies); + if (!$postentity->has_parent()) { - // Post is a discussion topic as well, so delete discussion. - if ($forum->type == 'single') { - redirect( - $urlfactory->get_discussion_view_url_from_discussion($discussionentity), - get_string('cannotdeletediscussioninsinglediscussion', 'forum'), - null, - \core\output\notification::NOTIFY_ERROR - ); - } - forum_delete_discussion($discussion, false, $course, $cm, $forum); - - $params = array( - 'objectid' => $discussion->id, - 'context' => $modcontext, - 'other' => array( - 'forumid' => $forum->id, - ) + forum_delete_discussion( + $discussiondatamapper->to_legacy_object($discussionentity), + false, + $forumentity->get_course_record(), + $forumentity->get_course_module_record(), + $forumdatamapper->to_legacy_object($forumentity) ); - $event = \mod_forum\event\discussion_deleted::create($params); - $event->add_record_snapshot('forum_discussions', $discussion); - $event->trigger(); - redirect( $urlfactory->get_forum_view_url_from_forum($forumentity), get_string('eventdiscussiondeleted', 'forum'), null, \core\output\notification::NOTIFY_SUCCESS ); - } else { - $deleted = forum_delete_post($post, has_capability('mod/forum:deleteanypost', $modcontext), $course, $cm, $forum); + forum_delete_post( + $postdatamapper->to_legacy_object($postentity), + has_capability('mod/forum:deleteanypost', $modcontext), + $forumentity->get_course_record(), + $forumentity->get_course_module_record(), + $forumdatamapper->to_legacy_object($forumentity) + ); - if (!$deleted) { - redirect( - $urlfactory->get_discussion_view_url_from_post($postentity), - get_string('errorwhiledelete', 'forum'), - null, - \core\output\notification::NOTIFY_ERROR - ); - } - - if ($forum->type == 'single') { + if ($forumentity->get_type() == 'single') { // Single discussion forums are an exception. // We show the forum itself since it only has one discussion thread. $discussionurl = $urlfactory->get_forum_view_url_from_forum($forumentity); @@ -471,10 +424,29 @@ if (!empty($forum)) { \core\output\notification::NOTIFY_SUCCESS ); } + } catch (Exception $e) { + redirect( + $urlfactory->get_discussion_view_url_from_discussion($discussionentity), + $e->getMessage(), + null, + \core\output\notification::NOTIFY_ERROR + ); } - } else { + + if (!$capabilitymanager->can_delete_post($USER, $discussionentity, $postentity)) { + redirect( + $urlfactory->get_discussion_view_url_from_discussion($discussionentity), + get_string('cannotdeletepost', 'forum'), + null, + \core\output\notification::NOTIFY_ERROR + ); + } + + $post = $postdatamapper->to_legacy_object($postentity); + $forum = $forumdatamapper->to_legacy_object($forumentity); + // User just asked to delete something. forum_set_return(); $PAGE->navbar->add(get_string('delete', 'forum')); diff --git a/mod/forum/tests/events_test.php b/mod/forum/tests/events_test.php index 9bbd159c7e0..b4ee7ed8f39 100644 --- a/mod/forum/tests/events_test.php +++ b/mod/forum/tests/events_test.php @@ -1626,22 +1626,35 @@ class mod_forum_events_testcase extends advanced_testcase { $sink = $this->redirectEvents(); forum_delete_discussion($discussion, true, $course, $cm, $forum); $events = $sink->get_events(); - // We will have 3 events. One for the discussion (creating a discussion creates a post), and two for the posts. - $this->assertCount(3, $events); + // We will have 4 events. One for the discussion, another one for the discussion topic post, and two for the posts. + $this->assertCount(4, $events); // Loop through the events and check they are valid. foreach ($events as $event) { - $post = $posts[$event->objectid]; - // Check that the event contains the expected values. - $this->assertInstanceOf('\mod_forum\event\post_deleted', $event); - $this->assertEquals(context_module::instance($forum->cmid), $event->get_context()); - $expected = array($course->id, 'forum', 'delete post', "discuss.php?d={$discussion->id}", $post->id, $forum->cmid); - $this->assertEventLegacyLogData($expected, $event); - $url = new \moodle_url('/mod/forum/discuss.php', array('d' => $discussion->id)); - $this->assertEquals($url, $event->get_url()); - $this->assertEventContextNotUsed($event); - $this->assertNotEmpty($event->get_name()); + if ($event->objectid == $discussion->id) { + // Check that the event contains the expected values. + $this->assertInstanceOf('\mod_forum\event\discussion_deleted', $event); + $this->assertEquals(context_module::instance($forum->cmid), $event->get_context()); + $expected = array($course->id, 'forum', 'delete discussion', "view.php?id={$forum->cmid}", + $forum->id, $forum->cmid); + $this->assertEventLegacyLogData($expected, $event); + $url = new \moodle_url('/mod/forum/view.php', array('id' => $forum->cmid)); + $this->assertEquals($url, $event->get_url()); + $this->assertEventContextNotUsed($event); + $this->assertNotEmpty($event->get_name()); + } else { + $post = $posts[$event->objectid]; + // Check that the event contains the expected values. + $this->assertInstanceOf('\mod_forum\event\post_deleted', $event); + $this->assertEquals(context_module::instance($forum->cmid), $event->get_context()); + $expected = array($course->id, 'forum', 'delete post', "discuss.php?d={$discussion->id}", $post->id, $forum->cmid); + $this->assertEventLegacyLogData($expected, $event); + $url = new \moodle_url('/mod/forum/discuss.php', array('d' => $discussion->id)); + $this->assertEquals($url, $event->get_url()); + $this->assertEventContextNotUsed($event); + $this->assertNotEmpty($event->get_name()); + } } } diff --git a/mod/forum/tests/managers_capability_test.php b/mod/forum/tests/managers_capability_test.php index cd7d66cc707..3c1e68e4607 100644 --- a/mod/forum/tests/managers_capability_test.php +++ b/mod/forum/tests/managers_capability_test.php @@ -1187,4 +1187,98 @@ class mod_forum_managers_capability_testcase extends advanced_testcase { $this->prevent_capability('mod/forum:readprivatereplies'); $this->assertFalse($capabilitymanager->can_view_any_private_reply($this->user)); } + + + /** + * Test delete a post with ratings. + */ + public function test_validate_delete_post_with_ratings() { + global $DB; + $this->resetAfterTest(true); + + // Setup test data. + $course = $this->getDataGenerator()->create_course(); + $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id)); + $user = $this->getDataGenerator()->create_user(); + $role = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + self::getDataGenerator()->enrol_user($user->id, $course->id, $role->id); + + // Add a discussion. + $record = new stdClass(); + $record->course = $course->id; + $record->userid = $user->id; + $record->forum = $forum->id; + $record->created = + $discussion = $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + // Add rating. + $post = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); + $post->totalscore = 80; + $DB->update_record('forum_posts', $post); + + $vaultfactory = mod_forum\local\container::get_vault_factory(); + $forumvault = $vaultfactory->get_forum_vault(); + $discussionvault = $vaultfactory->get_discussion_vault(); + $postvault = $vaultfactory->get_post_vault(); + + $postentity = $postvault->get_from_id($post->id); + $discussionentity = $discussionvault->get_from_id($postentity->get_discussion_id()); + $forumentity = $forumvault->get_from_id($discussionentity->get_forum_id()); + $capabilitymanager = $this->managerfactory->get_capability_manager($forumentity); + + $this->setUser($user); + $this->expectExceptionMessage(get_string('couldnotdeleteratings', 'rating')); + $capabilitymanager->validate_delete_post($user, $discussionentity, $postentity, false); + } + + /** + * Test delete a post with replies. + */ + public function test_validate_delete_post_with_replies() { + global $DB; + $this->resetAfterTest(true); + + // Setup test data. + $course = $this->getDataGenerator()->create_course(); + $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id)); + $user = $this->getDataGenerator()->create_user(); + $role = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + self::getDataGenerator()->enrol_user($user->id, $course->id, $role->id); + + // Add a discussion. + $record = new stdClass(); + $record->course = $course->id; + $record->userid = $user->id; + $record->forum = $forum->id; + $record->created = + $discussion = $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + $parentpost = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); + // Add a post. + $record = new stdClass(); + $record->course = $course->id; + $record->userid = $user->id; + $record->forum = $forum->id; + $record->discussion = $discussion->id; + $record->parent = $parentpost->id; + $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); + + $vaultfactory = mod_forum\local\container::get_vault_factory(); + $forumvault = $vaultfactory->get_forum_vault(); + $discussionvault = $vaultfactory->get_discussion_vault(); + $postvault = $vaultfactory->get_post_vault(); + + $postentity = $postvault->get_from_id($parentpost->id); + $discussionentity = $discussionvault->get_from_id($postentity->get_discussion_id()); + $forumentity = $forumvault->get_from_id($discussionentity->get_forum_id()); + $capabilitymanager = $this->managerfactory->get_capability_manager($forumentity); + + $this->setUser($user); + // Get reply count. + $replycount = $postvault->get_reply_count_for_post_id_in_discussion_id( + $user, $postentity->get_id(), $discussionentity->get_id(), true); + $this->expectExceptionMessage(get_string('couldnotdeletereplies', 'forum')); + $capabilitymanager->validate_delete_post($user, $discussionentity, $postentity, $replycount); + } + }