Skip to content

Commit

Permalink
Merge branch 'MDL-62516-master' of git://github.com/andrewnicols/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
David Monllao committed May 21, 2018
2 parents 5c33427 + 630081d commit 30f61e4
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 20 deletions.
10 changes: 6 additions & 4 deletions mod/forum/classes/privacy/provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,7 @@ public static function delete_data_for_all_users_in_context(\context $context) {
// Delete all files from the posts.
$fs = get_file_storage();
$fs->delete_area_files($context->id, 'mod_forum', 'post');
$fs->delete_area_files($context->id, 'mod_forum', 'attachment');

// Delete all ratings in the context.
\core_rating\privacy\provider::delete_ratings($context, 'mod_forum', 'post');
Expand Down Expand Up @@ -879,13 +880,14 @@ public static function delete_data_for_user(approved_contextlist $contextlist) {
// Delete all Tags.
\core_tag\privacy\provider::delete_item_tags_select($context, 'mod_forum', 'forum_posts',
"IN ($postidsql)", $postparams);

// Delete all files from the posts.
$fs = get_file_storage();
$fs->delete_area_files_select($context->id, 'mod_forum', 'post', "IN ($postidsql)", $postparams);
$fs->delete_area_files_select($context->id, 'mod_forum', 'attachment', "IN ($postidsql)", $postparams);
}

$uniquediscussions->close();

// Delete all files from the posts.
$fs = get_file_storage();
$fs->delete_area_files($context->id, 'mod_forum', 'post');
}
}
}
62 changes: 46 additions & 16 deletions mod/forum/tests/privacy_provider_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,15 @@ public function test_all_users_deleted_from_context() {
'filepath' => '/',
'filename' => 'example.jpg',
], 'image contents (not really)');
// And an attachment.
$fs->create_file_from_string([
'contextid' => $context->id,
'component' => 'mod_forum',
'filearea' => 'attachment',
'itemid' => $post->id,
'filepath' => '/',
'filename' => 'example.jpg',
], 'image contents (not really)');
}
}

Expand Down Expand Up @@ -962,6 +971,7 @@ public function test_all_users_deleted_from_context() {
// All uploaded files relating to this context should have been deleted (post content).
foreach ($postsinforum as $post) {
$this->assertEmpty($fs->get_area_files($context->id, 'mod_forum', 'post', $post->id));
$this->assertEmpty($fs->get_area_files($context->id, 'mod_forum', 'attachment', $post->id));
}

// All ratings should have been deleted.
Expand Down Expand Up @@ -1093,6 +1103,15 @@ public function test_delete_data_for_user() {
'filepath' => '/',
'filename' => 'example.jpg',
], 'image contents (not really)');
// And a fake attachment.
$fs->create_file_from_string([
'contextid' => $context->id,
'component' => 'mod_forum',
'filearea' => 'attachment',
'itemid' => $post->id,
'filepath' => '/',
'filename' => 'example.jpg',
], 'image contents (not really)');
}
}

Expand Down Expand Up @@ -1139,12 +1158,26 @@ public function test_delete_data_for_user() {

// 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);
$deletedpostids = [];
$otherpostids = [];
foreach ($postsbyforum as $user => $contexts) {
foreach ($contexts as $thiscontextid => $theseposts) {
$thesepostids = array_map(function($post) {
return $post->id;
}, $theseposts);

if ($user == $user1->id && $thiscontextid == $firstcontext->id) {
// This post is in the deleted context and by the target user.
$deletedpostids = array_merge($deletedpostids, $thesepostids);
} else {
// This post is by another user, or in a non-target context.
$otherpostids = array_merge($otherpostids, $thesepostids);
}
}
}
list($postinsql, $postinparams) = $DB->get_in_or_equal($deletedpostids, SQL_PARAMS_NAMED);
list($otherpostinsql, $otherpostinparams) = $DB->get_in_or_equal($otherpostids, SQL_PARAMS_NAMED);

$approvedcontextlist = new \core_privacy\tests\request\approved_contextlist(
\core_user::get_user($user1->id),
Expand Down Expand Up @@ -1197,28 +1230,25 @@ public function test_delete_data_for_user() {
// 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 on posts in the other context, and posts not belonging to the affected user.
$this->assertCount(144, $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'));
// Tags should remain for the other posts by this user, and all posts by other users.
$this->assertCount(72, $DB->get_records_select('tag_instance', "itemid {$otherpostinsql}", $otherpostinparams));

// Files for the affected posts should be removed.
// 5 users * 2 forums * 1 file in each forum
// Original total: 10
// One post with file 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));
$this->assertCount(18, $DB->get_records_select('files', "filename <> '.' AND itemid {$otherpostinsql}", $otherpostinparams));
}
}

0 comments on commit 30f61e4

Please sign in to comment.