From 2d64b4154cdea6a50ae83fc891560afdafca5c03 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Tue, 23 Oct 2018 15:18:03 +0800 Subject: [PATCH] MDL-63680 report_stats: Add support for removal of context users This issue is part of the MDL-62560 Epic. --- report/stats/classes/privacy/provider.php | 73 ++++++++++++- report/stats/tests/privacy_test.php | 125 +++++++++++++++++++++- 2 files changed, 192 insertions(+), 6 deletions(-) diff --git a/report/stats/classes/privacy/provider.php b/report/stats/classes/privacy/provider.php index 5db4f4f8bb1..9eb63adf232 100644 --- a/report/stats/classes/privacy/provider.php +++ b/report/stats/classes/privacy/provider.php @@ -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 * @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. * diff --git a/report/stats/tests/privacy_test.php b/report/stats/tests/privacy_test.php index 982030ff4f9..b79607c7315 100644 --- a/report/stats/tests/privacy_test.php +++ b/report/stats/tests/privacy_test.php @@ -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); + } }