Merge branch 'MDL-62601-master' of git://github.com/andrewnicols/moodle

moodle · Oct 31, 2018 · 57c2400 · 57c2400
2 parents df9813c + 5efa9f4
commit 57c2400
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 13 deletions.
diff --git a/lib/editor/atto/classes/privacy/provider.php b/lib/editor/atto/classes/privacy/provider.php
@@ -72,15 +72,16 @@ public static function get_contexts_for_userid(int $userid) : \core_privacy\loca
         // This block doesn't know who information is stored against unless it
         // is at the user context.
         $contextlist = new \core_privacy\local\request\contextlist();
-        $contextuser = \context_user::instance($userid);
 
-        $sql = "SELECT contextid FROM {editor_atto_autosave} WHERE userid = :userid OR contextid = :contextid";
-        $params = [
-            'userid' => $userid,
-            'contextid' => $contextuser->id,
-        ];
+        $sql = "SELECT
+                    c.id
+                  FROM {editor_atto_autosave} eas
+                  JOIN {context} c ON c.id = eas.contextid
+                 WHERE contextlevel = :contextuser AND c.instanceid = :userid";
+        $contextlist->add_from_sql($sql, ['contextuser' => CONTEXT_USER, 'userid' => $userid]);
 
-        $contextlist->add_from_sql($sql, $params);
+        $sql = "SELECT contextid FROM {editor_atto_autosave} WHERE userid = :userid";
+        $contextlist->add_from_sql($sql, ['userid' => $userid]);
 
         return $contextlist;
     }
@@ -95,20 +96,34 @@ public static function export_user_data(approved_contextlist $contextlist) {
 
         $user = $contextlist->get_user();
 
+        $sql = "SELECT *
+                  FROM {editor_atto_autosave}
+                 WHERE userid = :userid AND contextid {$contextsql}";
+
         list($contextsql, $contextparams) = $DB->get_in_or_equal($contextlist->get_contextids(), SQL_PARAMS_NAMED);
         $contextparams['userid'] = $contextlist->get_user()->id;
+        $autosaves = $DB->get_recordset_sql($sql, $contextparams);
+        self::export_autosaves($user, $autosaves);
 
         $sql = "SELECT *
                   FROM {editor_atto_autosave}
-                 WHERE
-                    (userid = :userid AND contextid {$contextsql})
-                    OR
-                    (contextid = :usercontext)";
+                  JOIN {context} c ON c.id = eas.contextid
+                 WHERE c.id {$contextsql} AND contextlevel = :contextuser AND c.instanceid = :userid";
 
-        $usercontext = \context_user::instance($user->id);
-        $contextparams['usercontext'] = $usercontext->id;
+        list($contextsql, $contextparams) = $DB->get_in_or_equal($contextlist->get_contextids(), SQL_PARAMS_NAMED);
+        $contextparams['userid'] = $contextlist->get_user()->id;
+        $contextparams['contextuser'] = CONTEXT_USER;
         $autosaves = $DB->get_recordset_sql($sql, $contextparams);
+        self::export_autosaves($user, $autosaves);
+    }
 
+    /**
+     * Export all autosave records in the recordset, and close the recordset when finished.
+     *
+     * @param   \stdClass   $user The user whose data is to be exported
+     * @param   \moodle_recordset $autosaves The recordset containing the data to export
+     */
+    protected static function export_autosaves(\stdClass $user, \moodle_recordset $autosaves) {
         foreach ($autosaves as $autosave) {
             $context = \context::instance_by_id($autosave->contextid);
             $subcontext = [

diff --git a/privacy/tests/provider_test.php b/privacy/tests/provider_test.php
@@ -178,6 +178,29 @@ public function test_all_providers_compliant($component, $classname) {
         $this->assertTrue($manager->component_is_compliant($component));
     }
 
+    /**
+     * Ensure that providers do not throw an error when processing a deleted user.
+     *
+     * @dataProvider    is_user_data_provider
+     * @param   string  $component
+     */
+    public function test_component_understands_deleted_users($component) {
+        $this->resetAfterTest();
+
+        // Create a user.
+        $user = $this->getDataGenerator()->create_user();
+
+        // Delete the user and their context.
+        delete_user($user);
+        $usercontext = \context_user::instance($user->id);
+        $usercontext->delete();
+
+        $contextlist = manager::component_class_callback($component, \core_privacy\local\request\core_user_data_provider::class,
+                'get_contexts_for_userid', [$user->id]);
+
+        $this->assertInstanceOf(\core_privacy\local\request\contextlist::class, $contextlist);
+    }
+
     /**
      * Data provider for the metadata\provider tests.
      *
@@ -192,6 +215,20 @@ public function metadata_provider_provider() {
         });
     }
 
+    /**
+     * List of providers which implement the core_user_data_provider.
+     *
+     * @return array
+     */
+    public function is_user_data_provider() {
+        return array_filter($this->get_component_list(), function($component) {
+                return static::component_implements(
+                    $component['classname'],
+                    \core_privacy\local\request\core_user_data_provider::class
+                );
+        });
+    }
+
     /**
      * Checks whether the component's provider class implements the specified interface, either directly or as a grandchild.
      *