From cb69e0ca88eaff97c6f7deeeae329d055ddc3d85 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Wed, 31 Oct 2018 11:08:49 +0800 Subject: [PATCH] MDL-63624 tool_cohortroles: Add support for removal of context users This issue is part of the MDL-62560 Epic. --- .../cohortroles/classes/privacy/provider.php | 55 ++++++++++- admin/tool/cohortroles/tests/privacy_test.php | 96 ++++++++++++++++++- 2 files changed, 148 insertions(+), 3 deletions(-) diff --git a/admin/tool/cohortroles/classes/privacy/provider.php b/admin/tool/cohortroles/classes/privacy/provider.php index bc38e7c4ba7..2cdfca6f2ca 100644 --- a/admin/tool/cohortroles/classes/privacy/provider.php +++ b/admin/tool/cohortroles/classes/privacy/provider.php @@ -30,6 +30,8 @@ use core_privacy\local\request\context; use core_privacy\local\request\contextlist; use core_privacy\local\request\transform; use core_privacy\local\request\writer; +use core_privacy\local\request\userlist; +use \core_privacy\local\request\approved_userlist; defined('MOODLE_INTERNAL') || die(); @@ -39,7 +41,10 @@ defined('MOODLE_INTERNAL') || die(); * @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\plugin\provider { +class provider implements + \core_privacy\local\metadata\provider, + \core_privacy\local\request\core_userlist_provider, + \core_privacy\local\request\plugin\provider { /** * Returns meta data about this system. @@ -91,6 +96,35 @@ 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(); + + // We should process user data from the system context. + // When we process user deletions and expiries, we always delete from the user context. + // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses + // as roles may change and data may be removed earlier than it should be. + if (!$context instanceof \context_system) { + return; + } + + $params = [ + 'contextid' => $context->id + ]; + + $sql = "SELECT tc.userid as userid + FROM {tool_cohortroles} tc + JOIN {cohort} c + ON tc.cohortid = c.id + WHERE c.contextid = :contextid"; + + $userlist->add_from_sql('userid', $sql, $params); + } + /** * Export all user data for the specified user, in the specified contexts. * @@ -172,6 +206,25 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l $DB->delete_records('tool_cohortroles', ['userid' => $userid]); } + /** + * 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(); + + // We should process user data from the system context. + // When we process user deletions and expiries, we always delete from the user context. + // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses + // as roles may change and data may be removed earlier than it should be. + if ($context instanceof \context_system) { + $DB->delete_records_list('tool_cohortroles', 'userid', $userlist->get_userids()); + } + } + /** * Delete all user data for the specified user, in the specified contexts. * diff --git a/admin/tool/cohortroles/tests/privacy_test.php b/admin/tool/cohortroles/tests/privacy_test.php index f118b74a099..26f99d30bee 100644 --- a/admin/tool/cohortroles/tests/privacy_test.php +++ b/admin/tool/cohortroles/tests/privacy_test.php @@ -30,6 +30,7 @@ use \core_privacy\local\request\writer; use \core_privacy\local\request\approved_contextlist; use \tool_cohortroles\api; use \tool_cohortroles\privacy\provider; +use core_privacy\local\request\approved_userlist; /** * Unit tests for the tool_cohortroles implementation of the privacy API. @@ -179,16 +180,107 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes $this->assertCount(0, $cohortroles); } + /** + * Test that only users within a course context are fetched. + */ + public function test_get_users_in_context() { + $component = 'tool_cohortroles'; + + // Create a user. + $user = $this->getDataGenerator()->create_user(); + $usercontext = context_user::instance($user->id); + + $systemcontext = context_system::instance(); + + $this->setAdminUser(); + + $userlist = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist); + $this->assertCount(0, $userlist); + + $nocohortroles = 3; + $this->setup_test_scenario_data($user->id, $nocohortroles); + + // The list of users within the system context should contain user. + provider::get_users_in_context($userlist); + $this->assertCount(1, $userlist); + $this->assertTrue(in_array($user->id, $userlist->get_userids())); + + // The list of users within the user context should be empty. + $userlist2 = new \core_privacy\local\request\userlist($usercontext, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(0, $userlist2); + } + + /** + * Test that data for users in approved userlist is deleted. + */ + public function test_delete_data_for_users() { + $component = 'tool_cohortroles'; + + // Create user1. + $user1 = $this->getDataGenerator()->create_user(); + // Create user2. + $user2 = $this->getDataGenerator()->create_user(); + // Create user3. + $user3 = $this->getDataGenerator()->create_user(); + $usercontext3 = context_user::instance($user3->id); + + $systemcontext = context_system::instance(); + + $this->setAdminUser(); + + $nocohortroles = 3; + $this->setup_test_scenario_data($user1->id, $nocohortroles); + $this->setup_test_scenario_data($user2->id, $nocohortroles, 'Sausage roll 2', + 'sausageroll2'); + $this->setup_test_scenario_data($user3->id, $nocohortroles, 'Sausage roll 3', + 'sausageroll3'); + + $userlist1 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(3, $userlist1); + $this->assertTrue(in_array($user1->id, $userlist1->get_userids())); + $this->assertTrue(in_array($user2->id, $userlist1->get_userids())); + $this->assertTrue(in_array($user3->id, $userlist1->get_userids())); + + // Convert $userlist1 into an approved_contextlist. + $approvedlist1 = new approved_userlist($systemcontext, $component, [$user1->id, $user2->id]); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist1); + + // Re-fetch users in systemcontext. + $userlist1 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist1); + // The user data of user1 and user2 in systemcontext should be deleted. + // The user data of user3 in systemcontext should be still present. + $this->assertCount(1, $userlist1); + $this->assertTrue(in_array($user3->id, $userlist1->get_userids())); + + // Convert $userlist1 into an approved_contextlist in the user context. + $approvedlist2 = new approved_userlist($usercontext3, $component, $userlist1->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist2); + // Re-fetch users in systemcontext. + $userlist1 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist1); + // The user data in systemcontext should not be deleted. + $this->assertCount(1, $userlist1); + } + /** * Helper function to setup tool_cohortroles records for testing a specific user. * * @param int $userid The ID of the user used for testing. * @param int $nocohortroles The number of tool_cohortroles to create for the user. + * @param string $rolename The name of the role to be created. + * @param string $roleshortname The short name of the role to be created. * @throws \core_competency\invalid_persistent_exception * @throws coding_exception */ - protected function setup_test_scenario_data($userid, $nocohortroles) { - $roleid = create_role('Sausage Roll', 'sausageroll', 'mmmm'); + protected function setup_test_scenario_data($userid, $nocohortroles, $rolename = 'Sausage Roll', + $roleshortname = 'sausageroll') { + $roleid = create_role($rolename, $roleshortname, 'mmmm'); for ($c = 0; $c < $nocohortroles; $c++) { $cohort = $this->getDataGenerator()->create_cohort();