MDL-63680 report_stats: Add support for removal of context users
authorMihail Geshoski <mihail@moodle.com>
Tue, 23 Oct 2018 07:18:03 +0000 (15:18 +0800)
committerMihail Geshoski <mihail@moodle.com>
Fri, 26 Oct 2018 04:17:30 +0000 (12:17 +0800)
This issue is part of the MDL-62560 Epic.

report/stats/classes/privacy/provider.php
report/stats/tests/privacy_test.php

index 5db4f4f..9eb63ad 100644 (file)
@@ -29,6 +29,8 @@ defined('MOODLE_INTERNAL') || die();
 use \core_privacy\local\metadata\collection;
 use \core_privacy\local\request\contextlist;
 use \core_privacy\local\request\approved_contextlist;
+use \core_privacy\local\request\userlist;
+use \core_privacy\local\request\approved_userlist;
 
 /**
  * Privacy Subsystem for report_stats implementing provider.
@@ -36,7 +38,10 @@ use \core_privacy\local\request\approved_contextlist;
  * @copyright  2018 Zig Tan <zig@moodle.com>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class provider implements \core_privacy\local\metadata\provider, \core_privacy\local\request\subsystem\provider{
+class provider implements
+        \core_privacy\local\metadata\provider,
+        \core_privacy\local\request\core_userlist_provider,
+        \core_privacy\local\request\subsystem\provider{
 
     /**
      * Returns information about the user data stored in this component.
@@ -111,6 +116,51 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l
         return $contextlist;
     }
 
+    /**
+     * Get the list of users within a specific context.
+     *
+     * @param userlist $userlist The userlist containing the list of users who have data in this context/plugin combination.
+     */
+    public static function get_users_in_context(userlist $userlist) {
+        $context = $userlist->get_context();
+
+        if (!$context instanceof \context_course) {
+            return;
+        }
+
+        $params = [
+            'contextid' => $context->id,
+            'contextcourse' => CONTEXT_COURSE,
+        ];
+
+        $sql = "SELECT sud.userid
+                  FROM {stats_user_daily} sud
+                  JOIN {context} ctx
+                       ON ctx.instanceid = sud.courseid
+                       AND ctx.contextlevel = :contextcourse
+                 WHERE ctx.id = :contextid";
+
+        $userlist->add_from_sql('userid', $sql, $params);
+
+        $sql = "SELECT suw.userid
+                  FROM {stats_user_weekly} suw
+                  JOIN {context} ctx
+                       ON ctx.instanceid = suw.courseid
+                       AND ctx.contextlevel = :contextcourse
+                 WHERE ctx.id = :contextid";
+
+        $userlist->add_from_sql('userid', $sql, $params);
+
+        $sql = "SELECT sum.userid
+                  FROM {stats_user_monthly} sum
+                  JOIN {context} ctx
+                       ON ctx.instanceid = sum.courseid
+                       AND ctx.contextlevel = :contextcourse
+                 WHERE ctx.id = :contextid";
+
+        $userlist->add_from_sql('userid', $sql, $params);
+    }
+
     /**
      * Export all user data for the specified user, in the specified contexts.
      *
@@ -204,6 +254,27 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l
         }
     }
 
+    /**
+     * Delete multiple users within a single context.
+     *
+     * @param approved_userlist $userlist The approved context and user information to delete information for.
+     */
+    public static function delete_data_for_users(approved_userlist $userlist) {
+        global $DB;
+
+        $context = $userlist->get_context();
+
+        if ($context instanceof \context_course) {
+            list($usersql, $userparams) = $DB->get_in_or_equal($userlist->get_userids(), SQL_PARAMS_NAMED);
+            $select = "courseid = :courseid AND userid {$usersql}";
+            $params = ['courseid' => $context->instanceid] + $userparams;
+
+            $DB->delete_records_select('stats_user_daily', $select, $params);
+            $DB->delete_records_select('stats_user_weekly', $select, $params);
+            $DB->delete_records_select('stats_user_monthly', $select, $params);
+        }
+    }
+
     /**
      * Deletes stats for a given course.
      *
index 982030f..b79607c 100644 (file)
@@ -24,6 +24,9 @@
 
 defined('MOODLE_INTERNAL') || die();
 
+use \report_stats\privacy\provider;
+use \core_privacy\local\request\approved_userlist;
+
 /**
  * Class report_stats_privacy_testcase
  *
@@ -76,13 +79,13 @@ class report_stats_privacy_testcase extends advanced_testcase {
         $this->create_stats($course2->id, $user1->id, 'stats_user_monthly');
         $this->create_stats($course1->id, $user2->id, 'stats_user_weekly');
 
-        $contextlist = \report_stats\privacy\provider::get_contexts_for_userid($user1->id);
+        $contextlist = provider::get_contexts_for_userid($user1->id);
         $this->assertCount(2, $contextlist->get_contextids());
         foreach ($contextlist->get_contexts() as $context) {
             $this->assertEquals(CONTEXT_COURSE, $context->contextlevel);
             $this->assertNotEquals($context3, $context);
         }
-        $contextlist = \report_stats\privacy\provider::get_contexts_for_userid($user2->id);
+        $contextlist = provider::get_contexts_for_userid($user2->id);
         $this->assertCount(1, $contextlist->get_contextids());
         $this->assertEquals($context1, $contextlist->current());
     }
@@ -105,7 +108,7 @@ class report_stats_privacy_testcase extends advanced_testcase {
 
         $approvedlist = new \core_privacy\local\request\approved_contextlist($user, 'report_stats', [$context1->id, $context2->id]);
 
-        \report_stats\privacy\provider::export_user_data($approvedlist);
+        provider::export_user_data($approvedlist);
         $writer = \core_privacy\local\request\writer::with_context($context1);
         $dailystats = (array) $writer->get_data([get_string('privacy:dailypath', 'report_stats')]);
         $this->assertCount(2, $dailystats);
@@ -154,7 +157,7 @@ class report_stats_privacy_testcase extends advanced_testcase {
         $this->assertCount(2, $monthlyrecords);
 
         // Delete all user data for course 1.
-        \report_stats\privacy\provider::delete_data_for_all_users_in_context($context1);
+        provider::delete_data_for_all_users_in_context($context1);
         $dailyrecords = $DB->get_records('stats_user_daily');
         $this->assertCount(1, $dailyrecords);
         $weeklyrecords = $DB->get_records('stats_user_weekly');
@@ -194,7 +197,7 @@ class report_stats_privacy_testcase extends advanced_testcase {
 
         // Delete all user data for course 1.
         $approvedlist = new \core_privacy\local\request\approved_contextlist($user1, 'report_stats', [$context1->id]);
-        \report_stats\privacy\provider::delete_data_for_user($approvedlist);
+        provider::delete_data_for_user($approvedlist);
         $dailyrecords = $DB->get_records('stats_user_daily');
         $this->assertCount(1, $dailyrecords);
         $weeklyrecords = $DB->get_records('stats_user_weekly');
@@ -202,4 +205,116 @@ class report_stats_privacy_testcase extends advanced_testcase {
         $monthlyrecords = $DB->get_records('stats_user_monthly');
         $this->assertCount(1, $monthlyrecords);
     }
+
+    /**
+     * Test that only users within a course context are fetched.
+     */
+    public function test_get_users_in_context() {
+        $this->resetAfterTest();
+
+        $component = 'report_stats';
+
+        // Create user1.
+        $user1 = $this->getDataGenerator()->create_user();
+        // Create user2.
+        $user2 = $this->getDataGenerator()->create_user();
+        // Create course1.
+        $course1 = $this->getDataGenerator()->create_course();
+        $coursecontext1 = context_course::instance($course1->id);
+        // Create course2.
+        $course2 = $this->getDataGenerator()->create_course();
+        $coursecontext2 = context_course::instance($course2->id);
+
+        $userlist1 = new \core_privacy\local\request\userlist($coursecontext1, $component);
+        provider::get_users_in_context($userlist1);
+        $this->assertCount(0, $userlist1);
+
+        $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component);
+        provider::get_users_in_context($userlist2);
+        $this->assertCount(0, $userlist2);
+
+        $this->create_stats($course1->id, $user1->id, 'stats_user_daily');
+        $this->create_stats($course2->id, $user1->id, 'stats_user_monthly');
+        $this->create_stats($course1->id, $user2->id, 'stats_user_weekly');
+
+        // The list of users within the course context should contain users.
+        provider::get_users_in_context($userlist1);
+        $this->assertCount(2, $userlist1);
+        $this->assertTrue(in_array($user1->id, $userlist1->get_userids()));
+        $this->assertTrue(in_array($user2->id, $userlist1->get_userids()));
+
+        provider::get_users_in_context($userlist2);
+        $this->assertCount(1, $userlist2);
+        $this->assertTrue(in_array($user1->id, $userlist2->get_userids()));
+
+        // The list of users within other contexts than course should be empty.
+        $systemcontext = context_system::instance();
+        $userlist3 = new \core_privacy\local\request\userlist($systemcontext, $component);
+        provider::get_users_in_context($userlist3);
+        $this->assertCount(0, $userlist3);
+    }
+
+    /**
+     * Test that data for users in approved userlist is deleted.
+     */
+    public function test_delete_data_for_users() {
+        $this->resetAfterTest();
+
+        $component = 'report_stats';
+
+        // Create user1.
+        $user1 = $this->getDataGenerator()->create_user();
+        // Create user2.
+        $user2 = $this->getDataGenerator()->create_user();
+        // Create user3.
+        $user3 = $this->getDataGenerator()->create_user();
+        // Create course1.
+        $course1 = $this->getDataGenerator()->create_course();
+        $coursecontext1 = context_course::instance($course1->id);
+        // Create course2.
+        $course2 = $this->getDataGenerator()->create_course();
+        $coursecontext2 = context_course::instance($course2->id);
+
+        $this->create_stats($course1->id, $user1->id, 'stats_user_daily');
+        $this->create_stats($course2->id, $user1->id, 'stats_user_monthly');
+        $this->create_stats($course1->id, $user2->id, 'stats_user_weekly');
+        $this->create_stats($course1->id, $user3->id, 'stats_user_weekly');
+
+        $userlist1 = new \core_privacy\local\request\userlist($coursecontext1, $component);
+        provider::get_users_in_context($userlist1);
+        $this->assertCount(3, $userlist1);
+
+        $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component);
+        provider::get_users_in_context($userlist2);
+        $this->assertCount(1, $userlist2);
+
+        // Convert $userlist1 into an approved_contextlist.
+        $approvedlist1 = new approved_userlist($coursecontext1, $component, [$user1->id, $user2->id]);
+        // Delete using delete_data_for_user.
+        provider::delete_data_for_users($approvedlist1);
+
+        // Re-fetch users in coursecontext1.
+        $userlist1 = new \core_privacy\local\request\userlist($coursecontext1, $component);
+        provider::get_users_in_context($userlist1);
+        // The approved user data in coursecontext1 should be deleted.
+        // The user list should still return user3.
+        $this->assertCount(1, $userlist1);
+        $this->assertTrue(in_array($user3->id, $userlist1->get_userids()));
+        // Re-fetch users in coursecontext2.
+        $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component);
+        provider::get_users_in_context($userlist2);
+        // The user data in coursecontext2 should be still present.
+        $this->assertCount(1, $userlist2);
+
+        // Convert $userlist2 into an approved_contextlist in the system context.
+        $systemcontext = context_system::instance();
+        $approvedlist2 = new approved_userlist($systemcontext, $component, $userlist2->get_userids());
+        // Delete using delete_data_for_user.
+        provider::delete_data_for_users($approvedlist2);
+        // Re-fetch users in coursecontext2.
+        $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component);
+        provider::get_users_in_context($userlist2);
+        // The user data in systemcontext should not be deleted.
+        $this->assertCount(1, $userlist2);
+    }
 }