From c8e4922194281607971ba7338adfcd6583dd4ab9 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Wed, 9 May 2018 10:16:39 +0800 Subject: [PATCH 1/1] MDL-62289 tool_dataprivacy: Ensure all user data deleted. We now do a comprehensive check and clean of user data when a user context expires. --- .../classes/expired_user_contexts.php | 27 +++++++++++++++++-- .../tests/expired_contexts_test.php | 19 +++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/admin/tool/dataprivacy/classes/expired_user_contexts.php b/admin/tool/dataprivacy/classes/expired_user_contexts.php index 62d9485e47d..e4b40e88a4d 100644 --- a/admin/tool/dataprivacy/classes/expired_user_contexts.php +++ b/admin/tool/dataprivacy/classes/expired_user_contexts.php @@ -115,12 +115,35 @@ class expired_user_contexts extends \tool_dataprivacy\expired_contexts_manager { * @return \context|false */ protected function delete_expired_context(\core_privacy\manager $privacymanager, \tool_dataprivacy\expired_context $expiredctx) { - if (!$context = parent::delete_expired_context($privacymanager, $expiredctx)) { + $context = \context::instance_by_id($expiredctx->get('contextid'), IGNORE_MISSING); + if (!$context) { + api::delete_expired_context($expiredctx->get('contextid')); return false; } - // Delete the user. + if (!PHPUNIT_TEST) { + mtrace('Deleting context ' . $context->id . ' - ' . + shorten_text($context->get_context_name(true, true))); + } + + // To ensure that all user data is deleted, instead of deleting by context, we run through and collect any stray + // contexts for the user that may still exist and call delete_data_for_user(). $user = \core_user::get_user($context->instanceid, '*', MUST_EXIST); + $approvedlistcollection = new \core_privacy\local\request\contextlist_collection($user->id); + $contextlistcollection = $privacymanager->get_contexts_for_userid($user->id); + + foreach ($contextlistcollection as $contextlist) { + $approvedlistcollection->add_contextlist(new \core_privacy\local\request\approved_contextlist( + $user, + $contextlist->get_component(), + $contextlist->get_contextids() + )); + } + + $privacymanager->delete_data_for_user($approvedlistcollection); + api::set_expired_context_status($expiredctx, expired_context::STATUS_CLEANED); + + // Delete the user. delete_user($user); return $context; diff --git a/admin/tool/dataprivacy/tests/expired_contexts_test.php b/admin/tool/dataprivacy/tests/expired_contexts_test.php index 7e62cc4b04f..1e434a7fbdd 100644 --- a/admin/tool/dataprivacy/tests/expired_contexts_test.php +++ b/admin/tool/dataprivacy/tests/expired_contexts_test.php @@ -84,6 +84,22 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase { $this->getDataGenerator()->enrol_user($user3->id, $course2->id, 'student'); $this->getDataGenerator()->enrol_user($user4->id, $course3->id, 'student'); + // Add an activity and some data for user 2. + $assignmod = $this->getDataGenerator()->create_module('assign', ['course' => $course2->id]); + $data = (object) [ + 'assignment' => $assignmod->id, + 'userid' => $user2->id, + 'timecreated' => time(), + 'timemodified' => time(), + 'status' => 'new', + 'groupid' => 0, + 'attemptnumber' => 0, + 'latest' => 1, + ]; + $DB->insert_record('assign_submission', $data); + // We should have one record in the assign submission table. + $this->assertEquals(1, $DB->count_records('assign_submission')); + // Users without lastaccess are skipped as well as users enroled in courses with no end date. $expired = new \tool_dataprivacy\expired_user_contexts(); $numexpired = $expired->flag_expired(); @@ -109,6 +125,9 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase { $deleted = $expired->delete(); $this->assertEquals(0, $deleted); + // No user data left in mod_assign. + $this->assertEquals(0, $DB->count_records('assign_submission')); + // The user is deleted. $deleteduser = \core_user::get_user($user2->id, 'id, deleted', IGNORE_MISSING); $this->assertEquals(1, $deleteduser->deleted); -- 2.43.0