From 21e93fdbe3406fb7678bb81b9dc99cc1179921cb Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 24 Apr 2018 08:07:06 +0800 Subject: [PATCH] MDL-61309 mod_forum: Implement privacy deletion --- .../classes/message/inbound/reply_handler.php | 1 + mod/forum/classes/privacy/provider.php | 37 ++- mod/forum/db/install.xml | 3 +- mod/forum/db/upgrade.php | 18 ++ mod/forum/discuss.php | 2 +- mod/forum/externallib.php | 26 ++- mod/forum/lib.php | 130 +++++++++-- mod/forum/post.php | 1 + mod/forum/rsslib.php | 19 +- mod/forum/tests/externallib_test.php | 86 ++++++- mod/forum/tests/privacy_provider_test.php | 217 ++++++++++++++++-- mod/forum/version.php | 2 +- 12 files changed, 467 insertions(+), 75 deletions(-) diff --git a/mod/forum/classes/message/inbound/reply_handler.php b/mod/forum/classes/message/inbound/reply_handler.php index cd369e52bdc..039f42ba319 100644 --- a/mod/forum/classes/message/inbound/reply_handler.php +++ b/mod/forum/classes/message/inbound/reply_handler.php @@ -162,6 +162,7 @@ class reply_handler extends \core\message\inbound\handler { $addpost->subject = $subject; $addpost->parent = $post->id; $addpost->itemid = file_get_unused_draft_itemid(); + $addpost->deleted = 0; list ($message, $format) = self::remove_quoted_text($messagedata); $addpost->message = $message; diff --git a/mod/forum/classes/privacy/provider.php b/mod/forum/classes/privacy/provider.php index 30e26b8136d..668ec48cf53 100644 --- a/mod/forum/classes/privacy/provider.php +++ b/mod/forum/classes/privacy/provider.php @@ -305,8 +305,6 @@ class provider implements static::export_digest_data($userid, $forum); static::export_subscription_data($userid, $forum); static::export_tracking_data($userid, $forum); - - } $forums->close(); @@ -373,6 +371,7 @@ class provider implements 'pinned' => transform::yesno((bool) $discussion->pinned), 'timemodified' => transform::datetime($discussion->timemodified), 'usermodified' => transform::datetime($discussion->usermodified), + 'creator_was_you' => transform::yesno($discussion->userid == $userid), ]; // Store the discussion content. @@ -546,7 +545,7 @@ class provider implements 'subject' => format_string($post->subject, true), 'created' => transform::datetime($post->created), 'modified' => transform::datetime($post->modified), - 'author' => transform::user($post->userid), + 'author_was_you' => transform::yesno($post->userid == $userid), ]; $postdata->message = writer::with_context($context) @@ -787,13 +786,10 @@ class provider implements $fs->delete_area_files($context->id, 'mod_forum', 'post'); // Delete all ratings in the context. - $rm = new \rating_manager(); - $rm->delete_ratings((object) [ - 'contextid' => $context->id, - ]); + \core_rating\privacy\provider::delete_ratings($context, 'mod_forum', 'post'); // Delete all Tags. - \core_tag_tag::delete_instances('mod_forum', 'post', $context->id); + \core_tag\privacy\provider::delete_item_tags($context, 'mod_forum', 'forum_posts'); } /** @@ -803,6 +799,8 @@ class provider implements */ public static function delete_data_for_user(approved_contextlist $contextlist) { global $DB; + $user = $contextlist->get_user(); + $userid = $user->id; foreach ($contextlist as $context) { // Get the course module. $cm = $DB->get_record('course_modules', ['id' => $context->instanceid]); @@ -824,7 +822,7 @@ class provider implements // Delete all discussion items. $DB->delete_records_select( 'forum_queue', - "userid AND discussionid IN (SELECT id FROM {forum_discussions} WHERE forum = :forum)", + "userid = :userid AND discussionid IN (SELECT id FROM {forum_discussions} WHERE forum = :forum)", [ 'userid' => $userid, 'forum' => $forum->id, @@ -845,6 +843,7 @@ class provider implements // Do not delete discussion or forum posts. // Instead update them to reflect that the content has been deleted. $postsql = "userid = :userid AND discussion IN (SELECT id FROM {forum_discussions} WHERE forum = :forum)"; + $postidsql = "SELECT fp.id FROM {forum_posts} fp WHERE {$postsql}"; $postparams = [ 'forum' => $forum->id, 'userid' => $userid, @@ -852,29 +851,23 @@ class provider implements // Update the subject. $DB->set_field_select('forum_posts', 'subject', '', $postsql, $postparams); - 'subject', - get_string('privacy:request:delete:post:subject', 'mod_forum'), - $postsql, - $postparams); // Update the subject and its format. $DB->set_field_select('forum_posts', 'message', '', $postsql, $postparams); - 'forum_posts', - 'message', - get_string('privacy:request:delete:post:message', 'mod_forum'), - $postsql, - $postparams); $DB->set_field_select('forum_posts', 'messageformat', FORMAT_PLAIN, $postsql, $postparams); - $discussion->name = get_string('privacy:request:delete:discussion:name', 'mod_forum', null, $lang); - $DB->update_record('forum_discussions', $discussion); + // Mark the post as deleted. + $DB->set_field_select('forum_posts', 'deleted', 1, $postsql, $postparams); - // Note: Do _not_ delete ratings. + // Note: Do _not_ delete ratings of other users. Only delete ratings on the users own posts. // Ratings are aggregate fields and deleting the rating of this post will have an effect on the rating // of any post. + \core_rating\privacy\provider::delete_ratings_select($context, 'mod_forum', 'post', + "IN ($postidsql)", $postparams); // Delete all Tags. - \core_tag_tag::delete_instances('mod_forum', 'post', $context->id); + \core_tag\privacy\provider::delete_item_tags_select($context, 'mod_forum', 'forum_posts', + "IN ($postidsql)", $postparams); } $uniquediscussions->close(); diff --git a/mod/forum/db/install.xml b/mod/forum/db/install.xml index cbd19068a57..9c0c99bf7ff 100644 --- a/mod/forum/db/install.xml +++ b/mod/forum/db/install.xml @@ -1,5 +1,5 @@ - @@ -80,6 +80,7 @@ + diff --git a/mod/forum/db/upgrade.php b/mod/forum/db/upgrade.php index 015d453eb5d..44f158173b4 100644 --- a/mod/forum/db/upgrade.php +++ b/mod/forum/db/upgrade.php @@ -228,5 +228,23 @@ function xmldb_forum_upgrade($oldversion) { upgrade_mod_savepoint(true, 2017051501, 'forum'); } + // Automatically generated Moodle v3.4.0 release upgrade line. + // Put any upgrade step following this. + + if ($oldversion < 2017051502) { + + // Define field deleted to be added to forum_posts. + $table = new xmldb_table('forum_posts'); + $field = new xmldb_field('deleted', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'mailnow'); + + // Conditionally launch add field deleted. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Forum savepoint reached. + upgrade_mod_savepoint(true, 2017051502, 'forum'); + } + return true; } diff --git a/mod/forum/discuss.php b/mod/forum/discuss.php index bccea71c822..73792f05ebd 100644 --- a/mod/forum/discuss.php +++ b/mod/forum/discuss.php @@ -219,7 +219,7 @@ if (! $post = forum_get_post_full($parent)) { print_error("notexists", 'forum', "$CFG->wwwroot/mod/forum/view.php?f=$forum->id"); } -if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { +if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm, false)) { print_error('noviewdiscussionspermission', 'forum', "$CFG->wwwroot/mod/forum/view.php?id=$forum->id"); } diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 12c0e1278b1..a3a4ef2ca12 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -249,8 +249,7 @@ class mod_forum_external extends external_api { $allposts = forum_get_all_discussion_posts($discussion->id, $sort, $forumtracked); foreach ($allposts as $post) { - - if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { + if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm, false)) { $warning = array(); $warning['item'] = 'post'; $warning['itemid'] = $post->id; @@ -275,6 +274,23 @@ class mod_forum_external extends external_api { $post->children = array(); } + if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { + // The post is available, but has been marked as deleted. + // It will still be available but filled with a placeholder. + $post->userid = null; + $post->userfullname = null; + $post->userpictureurl = null; + + $post->subject = get_string('privacy:request:delete:post:subject', 'mod_forum'); + $post->message = get_string('privacy:request:delete:post:message', 'mod_forum'); + + $post->deleted = true; + $posts[] = $post; + + continue; + } + $post->deleted = false; + if (forum_is_author_hidden($post, $forum)) { $post->userid = null; $post->userfullname = null; @@ -345,7 +361,8 @@ class mod_forum_external extends external_api { 'canreply' => new external_value(PARAM_BOOL, 'The user can reply to posts?'), 'postread' => new external_value(PARAM_BOOL, 'The post was read'), 'userfullname' => new external_value(PARAM_TEXT, 'Post author full name'), - 'userpictureurl' => new external_value(PARAM_URL, 'Post author picture.', VALUE_OPTIONAL) + 'userpictureurl' => new external_value(PARAM_URL, 'Post author picture.', VALUE_OPTIONAL), + 'deleted' => new external_value(PARAM_BOOL, 'This post has been removed.'), ), 'post' ) ), @@ -848,7 +865,8 @@ class mod_forum_external extends external_api { $post->messageformat = FORMAT_HTML; // Force formatting for now. $post->messagetrust = trusttext_trusted($context); $post->itemid = $options['inlineattachmentsid']; - $post->attachments = $options['attachmentsid']; + $post->attachments = $options['attachmentsid']; + $post->deleted = 0; $fakemform = $post->attachments; if ($postid = forum_add_new_post($post, $fakemform)) { diff --git a/mod/forum/lib.php b/mod/forum/lib.php index c34eaa7716e..344a4756450 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -1442,6 +1442,7 @@ function forum_print_overview($courses,&$htmlarray) { .'FROM {forum_discussions} d ' .'JOIN {forum_posts} p ON p.discussion = d.id ' ."WHERE ($coursessql) " + .'AND p.deleted <> 1 ' .'AND p.userid != ? ' .'AND (d.timestart <= ? AND (d.timeend = 0 OR d.timeend > ?)) ' .'GROUP BY d.id, d.forum, d.course, d.groupid ' @@ -1469,7 +1470,7 @@ function forum_print_overview($courses,&$htmlarray) { $sql = 'SELECT d.forum,d.course,COUNT(p.id) AS count '. ' FROM {forum_posts} p '. ' JOIN {forum_discussions} d ON p.discussion = d.id '. - ' LEFT JOIN {forum_read} r ON r.postid = p.id AND r.userid = ? WHERE ('; + ' LEFT JOIN {forum_read} r ON r.postid = p.id AND r.userid = ? WHERE p.deleted <> 1 AND ('; $params = array($USER->id); foreach ($trackingforums as $track) { @@ -1573,7 +1574,7 @@ function forum_print_recent_activity($course, $viewfullnames, $timestart) { JOIN {forum_discussions} d ON d.id = p.discussion JOIN {forum} f ON f.id = d.forum JOIN {user} u ON u.id = p.userid - WHERE p.created > ? AND f.course = ? + WHERE p.created > ? AND f.course = ? AND p.deleted <> 1 ORDER BY p.id ASC", array($timestart, $course->id))) { // order by initial posting date return false; } @@ -3182,7 +3183,8 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa $postisread = forum_tp_is_post_read($USER->id, $post); } - if (!forum_user_can_see_post($forum, $discussion, $post, NULL, $cm)) { + if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm, false)) { + // Do _not_ check the deleted flag - we need to display a different UI. $output = ''; if (!$dummyifcantsee) { if ($return) { @@ -3221,6 +3223,62 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa return; } + if (!empty($post->deleted)) { + // Note: Posts marked as deleted are still returned by the above forum_user_can_post because it is required for + // nesting of posts. + $output = ''; + if (!$dummyifcantsee) { + if ($return) { + return $output; + } + echo $output; + return; + } + $output .= html_writer::tag('a', '', [ + 'id' => "p{$post->id}", + ]); + $output .= html_writer::start_tag('div', [ + 'class' => 'forumpost clearfix', + 'role' => 'region', + 'aria-label' => get_string('forumbodydeleted', 'forum'), + ]); + + $output .= html_writer::start_tag('div', array('class' => 'row header')); + $output .= html_writer::tag('div', '', array('class' => 'left picture')); + + $classes = ['topic']; + if (!empty($post->parent)) { + $classes[] = 'starter'; + } + $output .= html_writer::start_tag('div', ['class' => implode(' ', $classes)]); + + // Subject. + $output .= html_writer::tag('div', get_string('forumsubjectdeleted', 'forum'), [ + 'class' => 'subject', + 'role' => 'header', + ]); + + // Author. + $output .= html_writer::tag('div', '', [ + 'class' => 'author', + 'role' => 'header', + ]); + + $output .= html_writer::end_tag('div'); + $output .= html_writer::end_tag('div'); // End row. + $output .= html_writer::start_tag('div', ['class' => 'row']); + $output .= html_writer::tag('div', ' ', ['class' => 'left side']); // Groups. + $output .= html_writer::tag('div', get_string('forumbodydeleted', 'forum'), ['class' => 'content']); // Content. + $output .= html_writer::end_tag('div'); // End row. + $output .= html_writer::end_tag('div'); // End forumpost. + + if ($return) { + return $output; + } + echo $output; + return; + } + if (empty($str)) { $str = new stdClass; $str->edit = get_string('edit', 'forum'); @@ -3487,7 +3545,7 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa } // Output ratings - if (!empty($post->rating)) { + if (!empty($post->deleted) && !empty($post->rating)) { $output .= html_writer::tag('div', $OUTPUT->render($post->rating), array('class'=>'forum-post-rating')); } @@ -3714,6 +3772,7 @@ function mod_forum_rating_can_see_item_ratings($params) { if (!forum_user_can_see_post($forum, $discussion, $post, $USER, $cm)) { return false; } + return true; } @@ -4735,6 +4794,7 @@ function forum_delete_post($post, $children, $course, $cm, $forum, $skipcompleti 'forumtype' => $forum->type, ) ); + $post->deleted = 1; if ($post->userid !== $USER->id) { $params['relateduserid'] = $post->userid; } @@ -5251,21 +5311,19 @@ function forum_user_can_see_discussion($forum, $discussion, $context, $user=NULL } /** - * @global object - * @global object - * @param object $forum - * @param object $discussion - * @param object $post - * @param object $user - * @param object $cm - * @return bool + * Check whether a user can see the specified post. + * + * @param \stdClass $forum The forum to chcek + * @param \stdClass $discussion The discussion the post is in + * @param \stdClass $post The post in question + * @param \stdClass $user The user to test - if not specified, the current user is checked. + * @param \stdClass $cm The Course Module that the forum is in (required). + * @param bool $checkdeleted Whether to check the deleted flag on the post. + * @return bool */ -function forum_user_can_see_post($forum, $discussion, $post, $user=NULL, $cm=NULL) { +function forum_user_can_see_post($forum, $discussion, $post, $user = null, $cm = null, $checkdeleted = true) { global $CFG, $USER, $DB; - // Context used throughout function. - $modcontext = context_module::instance($cm->id); - // retrieve objects (yuk) if (is_numeric($forum)) { debugging('missing full forum', DEBUG_DEVELOPER); @@ -5291,6 +5349,10 @@ function forum_user_can_see_post($forum, $discussion, $post, $user=NULL, $cm=NUL $post->id = $post->parent; } + if ($checkdeleted && !empty($post->deleted)) { + return false; + } + if (!$cm) { debugging('missing cm', DEBUG_DEVELOPER); if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) { @@ -5298,6 +5360,9 @@ function forum_user_can_see_post($forum, $discussion, $post, $user=NULL, $cm=NUL } } + // Context used throughout function. + $modcontext = context_module::instance($cm->id); + if (empty($user) || empty($user->id)) { $user = $USER; } @@ -5822,13 +5887,23 @@ function forum_print_posts_threaded($course, &$cm, $forum, $discussion, $parent, forum_print_post($post, $discussion, $forum, $cm, $course, $ownpost, $reply, $link, '', '', $postread, true, $forumtracked); } else { - if (!forum_user_can_see_post($forum, $discussion, $post, NULL, $cm)) { - echo "\n"; - continue; + if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm, true)) { + if (forum_user_can_see_post($forum, $discussion, $post, null, $cm, false)) { + // This post has been deleted but still exists and may have children. + $subject = get_string('privacy:request:delete:post:subject', 'mod_forum'); + $byline = ''; + } else { + // The user can't see this post at all. + echo "\n"; + continue; + } + } else { + $by = new stdClass(); + $by->name = fullname($post, $canviewfullnames); + $by->date = userdate($post->modified); + $byline = ' ' . get_string("bynameondate", "forum", $by); + $subject = format_string($post->subject, true); } - $by = new stdClass(); - $by->name = fullname($post, $canviewfullnames); - $by->date = userdate($post->modified); if ($forumtracked) { if (!empty($post->postread)) { @@ -5839,9 +5914,14 @@ function forum_print_posts_threaded($course, &$cm, $forum, $discussion, $parent, } else { $style = ''; } - echo $style."id\">". - "discussion&parent=$post->id\">".format_string($post->subject,true)." "; - print_string("bynameondate", "forum", $by); + + echo $style; + echo ""; + echo html_writer::link(new moodle_url('/mod/forum/discuss.php', [ + 'd' => $post->discussion, + 'parent' => $post->id, + ]), $subject); + echo $byline; echo ""; } diff --git a/mod/forum/post.php b/mod/forum/post.php index 88acf308fd5..df0a726d408 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -791,6 +791,7 @@ if ($mform_post->is_cancelled()) { $addpost = $fromform; $addpost->forum=$forum->id; if ($fromform->id = forum_add_new_post($addpost, $mform_post)) { + $fromform->deleted = 0; $subscribemessage = forum_post_subscription($fromform, $forum, $discussion); if (!empty($fromform->mailnow)) { diff --git a/mod/forum/rsslib.php b/mod/forum/rsslib.php index 4f413976d93..f360627dbcf 100644 --- a/mod/forum/rsslib.php +++ b/mod/forum/rsslib.php @@ -189,7 +189,7 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) { FROM {forum_discussions} d JOIN {forum_posts} p ON p.discussion = d.id JOIN {user} u ON p.userid = u.id - WHERE d.forum = {$forum->id} AND p.parent = 0 + WHERE d.forum = {$forum->id} AND p.parent = 0 AND p.deleted <> 0 $timelimit $groupselect $newsince ORDER BY $forumsort"; return array($sql, $params); @@ -243,7 +243,7 @@ function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) { {forum_posts} p, {user} u WHERE d.forum = {$forum->id} AND - p.discussion = d.id AND + p.discussion = d.id AND p.deleted <> 0 AND u.id = p.userid $newsince $groupselect ORDER BY p.created desc"; @@ -339,10 +339,17 @@ function forum_rss_feed_contents($forum, $sql, $params, $context) { $message = get_string('forumbodyhidden', 'forum'); $item->author = get_string('forumauthorhidden', 'forum'); } else if (!$isdiscussion && !forum_user_can_see_post($forum, $discussion, $post, $USER, $cm)) { - // This is a post which the user has no permission to view - $item->title = get_string('forumsubjecthidden', 'forum'); - $message = get_string('forumbodyhidden', 'forum'); - $item->author = get_string('forumauthorhidden', 'forum'); + if (forum_user_can_see_post($forum, $discussion, $post, $USER, $cm, false)) { + // This is a post which the user has no permission to view. + $item->title = get_string('forumsubjecthidden', 'forum'); + $message = get_string('forumbodyhidden', 'forum'); + $item->author = get_string('forumauthorhidden', 'forum'); + } else { + // This is a post which has been deleted. + $item->title = get_string('privacy:request:delete:post:subject', 'mod_forum'); + $message = get_string('privacy:request:delete:post:subject', 'mod_forum'); + $item->author = get_string('forumauthorhidden', 'forum'); + } } else { // The user must have permission to view if ($isdiscussion && !empty($rec->discussionname)) { diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index e3ee2b66a23..b4fd959c5bb 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -299,7 +299,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { 'canreply' => true, 'postread' => false, 'userfullname' => fullname($user3), - 'userpictureurl' => '' + 'userpictureurl' => '', + 'deleted' => false, ); $expectedposts['posts'][] = array( @@ -334,7 +335,8 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { 'canreply' => true, 'postread' => false, 'userfullname' => fullname($user2), - 'userpictureurl' => '' + 'userpictureurl' => '', + 'deleted' => false, ); // Test a discussion with two additional posts (total 3 posts). @@ -407,6 +409,86 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { } } + /** + * Test get forum posts + */ + public function test_mod_forum_get_forum_discussion_posts_deleted() { + global $CFG, $PAGE; + + $this->resetAfterTest(true); + $generator = self::getDataGenerator()->get_plugin_generator('mod_forum'); + + // Create a course and enrol some users in it. + $course1 = self::getDataGenerator()->create_course(); + + // Create users. + $user1 = self::getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user1->id, $course1->id); + $user2 = self::getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user2->id, $course1->id); + + // Set the first created user to the test user. + self::setUser($user1); + + // Create test data. + $forum1 = self::getDataGenerator()->create_module('forum', (object) [ + 'course' => $course1->id, + ]); + $forum1context = context_module::instance($forum1->cmid); + + // Add discussions to the forum. + $discussion = $generator->create_discussion((object) [ + 'course' => $course1->id, + 'userid' => $user1->id, + 'forum' => $forum1->id, + ]); + + $discussion2 = $generator->create_discussion((object) [ + 'course' => $course1->id, + 'userid' => $user2->id, + 'forum' => $forum1->id, + ]); + + // Add replies to the discussion. + $discussionreply1 = $generator->create_post((object) [ + 'discussion' => $discussion->id, + 'parent' => $discussion->firstpost, + 'userid' => $user2->id, + ]); + $discussionreply2 = $generator->create_post((object) [ + 'discussion' => $discussion->id, + 'parent' => $discussionreply1->id, + 'userid' => $user2->id, + 'subject' => '', + 'message' => '', + 'messageformat' => FORMAT_PLAIN, + 'deleted' => 1, + ]); + $discussionreply3 = $generator->create_post((object) [ + 'discussion' => $discussion->id, + 'parent' => $discussion->firstpost, + 'userid' => $user2->id, + ]); + + // Test where some posts have been marked as deleted. + $posts = mod_forum_external::get_forum_discussion_posts($discussion->id, 'modified', 'DESC'); + $posts = external_api::clean_returnvalue(mod_forum_external::get_forum_discussion_posts_returns(), $posts); + $deletedsubject = get_string('privacy:request:delete:post:subject', 'mod_forum'); + $deletedmessage = get_string('privacy:request:delete:post:message', 'mod_forum'); + + foreach ($posts['posts'] as $post) { + if ($post['id'] == $discussionreply2->id) { + $this->assertTrue($post['deleted']); + $this->assertEquals($deletedsubject, $post['subject']); + $this->assertEquals($deletedmessage, $post['message']); + } else { + $this->assertFalse($post['deleted']); + $this->assertNotEquals($deletedsubject, $post['subject']); + $this->assertNotEquals($deletedmessage, $post['message']); + } + } + } + /** * Test get forum posts (qanda forum) */ diff --git a/mod/forum/tests/privacy_provider_test.php b/mod/forum/tests/privacy_provider_test.php index 85d24abb55f..fc6ac263ae4 100644 --- a/mod/forum/tests/privacy_provider_test.php +++ b/mod/forum/tests/privacy_provider_test.php @@ -29,6 +29,8 @@ global $CFG; require_once(__DIR__ . '/helper.php'); require_once($CFG->dirroot . '/rating/lib.php'); +use \mod_forum\privacy\provider; + /** * Tests for the forum implementation of the Privacy Provider API. * @@ -888,22 +890,22 @@ class mod_forum_privacy_provider_testcase extends \core_privacy\tests\provider_t } } - // Mark all posts as read by user1. - $user1 = reset($users); + // Mark all posts as read by user. + $user = reset($users); $ratedposts = []; foreach ($posts as $post) { $discussion = $discussions[$post->discussion]; $forum = $forums[$discussion->forum]; $context = $contexts[$forum->id]; - // Mark the post as being read by user1. - forum_tp_add_read_record($user1->id, $post->id); + // Mark the post as being read by user. + forum_tp_add_read_record($user->id, $post->id); // Tag the post. \core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $post->id, $context, ['example', 'tag']); // Rate the other users content. - if ($post->userid != $user1->id) { + if ($post->userid != $user->id) { $ratedposts[$post->id] = $post; $rm = new rating_manager(); $ratingoptions = (object) [ @@ -918,7 +920,6 @@ class mod_forum_privacy_provider_testcase extends \core_privacy\tests\provider_t $rating = new \rating($ratingoptions); $rating->update_rating(75); } - $posttags = \core_tag_tag::get_item_tags('mod_forum', 'forum_posts', $post->id); } // Run as the user under test. @@ -941,7 +942,7 @@ class mod_forum_privacy_provider_testcase extends \core_privacy\tests\provider_t // Delete for the first forum. $forum = reset($forums); $context = $contexts[$forum->id]; - $this->delete_data_for_all_users_in_context('mod_forum', $context); + provider::delete_data_for_all_users_in_context($context); // Determine what should have been deleted. $discussionsinforum = array_filter($discussions, function($discussion) use ($forum) { @@ -976,9 +977,8 @@ class mod_forum_privacy_provider_testcase extends \core_privacy\tests\provider_t } // All tags should have been deleted. - $posttags = \core_tag_tag::get_items_tags('mod_forum', 'post', array_keys($postsinforum)); - foreach ($posttags as $tags) { - $this->assertEmpty($tags); + foreach (array_keys($postsinforum) as $postid) { + $this->assertCount(0, \core_tag_tag::get_item_tags('mod_forum', 'forum_posts', $postid)); } // Check the other forum too. It should remain intact. @@ -1023,9 +1023,200 @@ class mod_forum_privacy_provider_testcase extends \core_privacy\tests\provider_t } // All tags should remain. - $posttags = \core_tag_tag::get_items_tags('mod_forum', 'forum_posts', array_keys($postsinforum)); - foreach ($posttags as $tags) { - $this->assertNotEmpty($tags); + foreach (array_keys($postsinforum) as $postid) { + $this->assertNotEmpty(\core_tag_tag::get_item_tags('mod_forum', 'forum_posts', $postid)); } } + + /** + * Ensure that all user data is deleted for a specific context. + */ + public function test_delete_data_for_user() { + global $DB; + + $fs = get_file_storage(); + $course = $this->getDataGenerator()->create_course(); + $users = $this->helper_create_users($course, 5); + + $forums = []; + $contexts = []; + for ($i = 0; $i < 2; $i++) { + $forum = $this->getDataGenerator()->create_module('forum', [ + 'course' => $course->id, + 'scale' => 100, + ]); + $cm = get_coursemodule_from_instance('forum', $forum->id); + $context = \context_module::instance($cm->id); + $forums[$forum->id] = $forum; + $contexts[$forum->id] = $context; + } + + $discussions = []; + $posts = []; + $postsbyforum = []; + foreach ($users as $user) { + $postsbyforum[$user->id] = []; + foreach ($forums as $forum) { + $context = $contexts[$forum->id]; + + // Create a new discussion + post in the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $user); + $discussion = $DB->get_record('forum_discussions', ['id' => $discussion->id]); + $discussions[$discussion->id] = $discussion; + $postsbyforum[$user->id][$context->id] = []; + + // Add a number of replies. + $posts[$post->id] = $post; + $thisforumposts[$post->id] = $post; + $postsbyforum[$user->id][$context->id][$post->id] = $post; + + $reply = $this->helper_reply_to_post($post, $user); + $posts[$reply->id] = $reply; + $postsbyforum[$user->id][$context->id][$reply->id] = $reply; + + $reply = $this->helper_reply_to_post($post, $user); + $posts[$reply->id] = $reply; + $postsbyforum[$user->id][$context->id][$reply->id] = $reply; + + $reply = $this->helper_reply_to_post($reply, $user); + $posts[$reply->id] = $reply; + $postsbyforum[$user->id][$context->id][$reply->id] = $reply; + + // Add a fake inline image to the original post. + $fs->create_file_from_string([ + 'contextid' => $context->id, + 'component' => 'mod_forum', + 'filearea' => 'post', + 'itemid' => $post->id, + 'filepath' => '/', + 'filename' => 'example.jpg', + ], 'image contents (not really)'); + } + } + + // Mark all posts as read by user1. + $user1 = reset($users); + foreach ($posts as $post) { + $discussion = $discussions[$post->discussion]; + $forum = $forums[$discussion->forum]; + $context = $contexts[$forum->id]; + + // Mark the post as being read by user1. + forum_tp_add_read_record($user1->id, $post->id); + } + + // Rate and tag all posts. + $ratedposts = []; + foreach ($users as $user) { + foreach ($posts as $post) { + $discussion = $discussions[$post->discussion]; + $forum = $forums[$discussion->forum]; + $context = $contexts[$forum->id]; + + // Tag the post. + \core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $post->id, $context, ['example', 'tag']); + + // Rate the other users content. + if ($post->userid != $user->id) { + $ratedposts[$post->id] = $post; + $rm = new rating_manager(); + $ratingoptions = (object) [ + 'context' => $context, + 'component' => 'mod_forum', + 'ratingarea' => 'post', + 'itemid' => $post->id, + 'scaleid' => $forum->scale, + 'userid' => $user->id, + ]; + + $rating = new \rating($ratingoptions); + $rating->update_rating(75); + } + } + } + + // Delete for one of the forums for the first user. + $firstcontext = reset($contexts); + list($postinsql, $postinparams) = $DB->get_in_or_equal( + array_keys($postsbyforum[$user1->id][$firstcontext->id]), SQL_PARAMS_NAMED); + + $othercontext = next($contexts); + list($otherpostinsql, $otherpostinparams) = $DB->get_in_or_equal( + array_keys($postsbyforum[$user1->id][$othercontext->id]), SQL_PARAMS_NAMED); + + $approvedcontextlist = new \core_privacy\tests\request\approved_contextlist( + \core_user::get_user($user1->id), + 'mod_forum', + [$firstcontext->id] + ); + provider::delete_data_for_user($approvedcontextlist); + + // All posts should remain. + $this->assertCount(40, $DB->get_records('forum_posts')); + + // There should be 8 posts belonging to user1. + $this->assertCount(8, $DB->get_records('forum_posts', [ + 'userid' => $user1->id, + ])); + + // Four of those posts should have been marked as deleted. + // That means that the deleted flag is set, and both the subject and message are empty. + $this->assertCount(4, $DB->get_records_select('forum_posts', "userid = :userid AND deleted = :deleted" + . " AND " . $DB->sql_compare_text('subject') . " = " . $DB->sql_compare_text(':subject') + . " AND " . $DB->sql_compare_text('message') . " = " . $DB->sql_compare_text(':message') + , [ + 'userid' => $user1->id, + 'deleted' => 1, + 'subject' => '', + 'message' => '', + ])); + + // Only user1's posts should have been marked this way. + $this->assertCount(4, $DB->get_records('forum_posts', [ + 'deleted' => 1, + ])); + $this->assertCount(4, $DB->get_records_select('forum_posts', + $DB->sql_compare_text('subject') . " = " . $DB->sql_compare_text(':subject'), [ + 'subject' => '', + ])); + $this->assertCount(4, $DB->get_records_select('forum_posts', + $DB->sql_compare_text('message') . " = " . $DB->sql_compare_text(':message'), [ + 'message' => '', + ])); + + // Only the posts in the first discussion should have been marked this way. + $this->assertCount(4, $DB->get_records_select('forum_posts', + "deleted = :deleted AND id {$postinsql}", + array_merge($postinparams, [ + 'deleted' => 1, + ]) + )); + + // Ratings should have been removed from the affected posts. + $this->assertCount(0, $DB->get_records_select('rating', "itemid {$postinsql}", $postinparams)); + + // Ratings should remain on posts in the other context. + $this->assertCount(16, $DB->get_records_select('rating', "itemid {$otherpostinsql}", $otherpostinparams)); + + // Ratings should remain where the user has rated another person's post. + $this->assertCount(32, $DB->get_records('rating', ['userid' => $user1->id])); + + // Tags for the affected posts should be removed. + $this->assertCount(8, $DB->get_records_select('tag_instance', "itemid {$otherpostinsql}", $otherpostinparams)); + + // Tags should remain for the other posts by this user. + $this->assertCount(0, $DB->get_records_select('tag_instance', "itemid {$postinsql}", $postinparams)); + + // Tags should remain for others. + // Original total: 5 users * 2 forums * 4 posts * 2 tags + // Deleted posts: 8 + // New total: 72. + $this->assertCount(72, $DB->get_records('tag_instance')); + + // Files for the affected posts should be removed. + $this->assertCount(0, $DB->get_records_select('files', "itemid {$postinsql}", $postinparams)); + + // Files for the other posts should remain. + $this->assertCount(2, $DB->get_records_select('files', "itemid {$otherpostinsql}", $otherpostinparams)); + } } diff --git a/mod/forum/version.php b/mod/forum/version.php index 1644f544cc8..f425d08b152 100644 --- a/mod/forum/version.php +++ b/mod/forum/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2017051501; // The current module version (Date: YYYYMMDDXX) +$plugin->version = 2017051502; // The current module version (Date: YYYYMMDDXX) $plugin->requires = 2017050500; // Requires this Moodle version $plugin->component = 'mod_forum'; // Full name of the plugin (used for diagnostics)