From c8e4922194281607971ba7338adfcd6583dd4ab9 Mon Sep 17 00:00:00 2001
From: Adrian Greeve <adrian@moodle.com>
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.17.1