From 575584cb94760808317b88cb8cf4d3c73db55d8a Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Tue, 30 Oct 2018 16:18:24 +0800 Subject: [PATCH] MDL-63530 core_notes: Add support for removal of context users This issue is part of the MDL-62560 Epic. --- notes/classes/privacy/provider.php | 74 ++++++++++- notes/tests/privacy_test.php | 205 +++++++++++++++++++++++++++++ 2 files changed, 278 insertions(+), 1 deletion(-) diff --git a/notes/classes/privacy/provider.php b/notes/classes/privacy/provider.php index 479fb276286..1a661beb261 100644 --- a/notes/classes/privacy/provider.php +++ b/notes/classes/privacy/provider.php @@ -29,6 +29,8 @@ use core_privacy\local\request\approved_contextlist; 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(); @@ -41,7 +43,10 @@ require_once($CFG->dirroot . '/notes/lib.php'); * @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 { /** * Return the fields which contain personal data. @@ -112,6 +117,48 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l return $contextlist; } + /** + * Get the list of users who have data within a 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) { + global $DB; + + $context = $userlist->get_context(); + + if (!$context instanceof \context_course) { + return; + } + + $params = [ + 'instanceid' => $context->instanceid + ]; + + $sql = "SELECT usermodified as userid + FROM {post} + WHERE module = 'notes' + AND courseid = :instanceid"; + + $userlist->add_from_sql('userid', $sql, $params); + + $publishstates = [ + NOTES_STATE_PUBLIC, + NOTES_STATE_SITE + ]; + + list($publishstatesql, $publishstateparams) = $DB->get_in_or_equal($publishstates, SQL_PARAMS_NAMED); + $params += $publishstateparams; + + $sql = "SELECT userid + FROM {post} + WHERE module = 'notes' + AND courseid = :instanceid + AND publishstate {$publishstatesql}"; + + $userlist->add_from_sql('userid', $sql, $params); + } + /** * Export personal data for the given approved_contextlist. * User and context information is contained within the contextlist. @@ -191,6 +238,31 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l $DB->delete_records('post', ['module' => 'notes', 'courseid' => $context->instanceid]); } + /** + * 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->contextlevel != CONTEXT_COURSE) { + return; + } + + $userids = $userlist->get_userids(); + if (empty($userids)) { + return; + } + + list($usersql, $userparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED); + $select = "module = :module AND courseid = :courseid AND usermodified {$usersql}"; + $params = ['module' => 'notes', 'courseid' => $context->instanceid] + $userparams; + + $DB->delete_records_select('post', $select, $params); + } + /** * Delete all user data for the specified user, in the specified contexts. * diff --git a/notes/tests/privacy_test.php b/notes/tests/privacy_test.php index c3b7e94b60f..1e17b06a3a2 100644 --- a/notes/tests/privacy_test.php +++ b/notes/tests/privacy_test.php @@ -31,6 +31,7 @@ require_once($CFG->dirroot . "/notes/lib.php"); use \core_notes\privacy\provider; use \core_privacy\local\request\writer; use \core_privacy\local\request\approved_contextlist; +use \core_privacy\local\request\approved_userlist; /** * Unit tests for the core_notes implementation of the privacy API. @@ -357,6 +358,210 @@ class core_notes_privacy_testcase extends \core_privacy\tests\provider_testcase $this->assertCount(0, $notes); } + /** + * Test that only users within a course context are fetched. + */ + public function test_get_users_in_context() { + global $DB; + + $this->resetAfterTest(true); + + $component = 'core_notes'; + // Test setup. + $this->setAdminUser(); + set_config('enablenotes', true); + // Create a teacher. + $teacher1 = $this->getDataGenerator()->create_user(); + $this->setUser($teacher1); + $teacherrole = $DB->get_record('role', array('shortname' => 'teacher')); + // Create a student. + $student = $this->getDataGenerator()->create_user(); + // Create student2. + $student2 = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + + // Create courses, then enrol a teacher and a student. + $nocourses = 3; + for ($c = 1; $c <= $nocourses; $c++) { + ${'course' . $c} = $this->getDataGenerator()->create_course(); + ${'coursecontext' . $c} = context_course::instance(${'course' . $c}->id); + + role_assign($teacherrole->id, $teacher1->id, ${'coursecontext' . $c}->id); + role_assign($studentrole->id, $student->id, ${'coursecontext' . $c}->id); + role_assign($studentrole->id, $student2->id, ${'coursecontext' . $c}->id); + } + // The list of users in coursecontext1 should be empty (related data still have not been created). + $userlist1 = new \core_privacy\local\request\userlist($coursecontext1, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(0, $userlist1); + // The list of users in coursecontext2 should be empty (related data still have not been created). + $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(0, $userlist2); + // The list of users in coursecontext3 should be empty (related data still have not been created). + $userlist3 = new \core_privacy\local\request\userlist($coursecontext3, $component); + provider::get_users_in_context($userlist3); + $this->assertCount(0, $userlist3); + + // Create private user notes (i.e. NOTES_STATE_DRAFT) for student in course1 and course2 written by the teacher. + $this->help_create_user_note($student->id, NOTES_STATE_DRAFT, $course1->id, + "Test private user note about the student in Course 1 by the teacher"); + $this->help_create_user_note($student->id, NOTES_STATE_DRAFT, $course2->id, + "Test private user note about the student in Course 2 by the teacher"); + + // The list of users in coursecontext1 should return one user (teacher1). + provider::get_users_in_context($userlist1); + $this->assertCount(1, $userlist1); + $this->assertTrue(in_array($teacher1->id, $userlist1->get_userids())); + // The list of users in coursecontext2 should return one user (teacher1). + provider::get_users_in_context($userlist2); + $this->assertCount(1, $userlist2); + $this->assertTrue(in_array($teacher1->id, $userlist2->get_userids())); + // The list of users in coursecontext3 should not return any users. + provider::get_users_in_context($userlist3); + $this->assertCount(0, $userlist3); + + // Create public user note (i.e. NOTES_STATE_PUBLIC) for student in course3 written by the teacher. + $this->help_create_user_note($student->id, NOTES_STATE_PUBLIC, $course3->id, + "Test public user note about the student in Course 3 by the teacher"); + + // The list of users in coursecontext3 should return 2 users (teacher and student). + provider::get_users_in_context($userlist3); + $this->assertCount(2, $userlist3); + $this->assertTrue(in_array($teacher1->id, $userlist3->get_userids())); + $this->assertTrue(in_array($student->id, $userlist3->get_userids())); + + // Create site user note (i.e. NOTES_STATE_SITE) for student2 in course3 written by the teacher. + $this->help_create_user_note($student2->id, NOTES_STATE_SITE, $course3->id, + "Test site-wide user note about student2 in Course 3 by the teacher" + ); + + // The list of users in coursecontext3 should return 3 users (teacher, student and student2). + provider::get_users_in_context($userlist3); + $this->assertCount(3, $userlist3); + $this->assertTrue(in_array($teacher1->id, $userlist3->get_userids())); + $this->assertTrue(in_array($student->id, $userlist3->get_userids())); + $this->assertTrue(in_array($student2->id, $userlist3->get_userids())); + + // The list of users should not return any users in a different context than course context. + $contextsystem = context_system::instance(); + $userlist4 = new \core_privacy\local\request\userlist($contextsystem, $component); + provider::get_users_in_context($userlist4); + $this->assertCount(0, $userlist4); + } + + /** + * Test that data for users in approved userlist is deleted. + */ + public function test_delete_data_for_users() { + global $DB; + + $this->resetAfterTest(true); + + $component = 'core_notes'; + // Test setup. + $this->setAdminUser(); + set_config('enablenotes', true); + // Create a teacher. + $teacher1 = $this->getDataGenerator()->create_user(); + $this->setUser($teacher1); + $teacherrole = $DB->get_record('role', array('shortname' => 'teacher')); + // Create a student. + $student = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + + // Create Courses, then enrol a teacher and a student. + $nocourses = 3; + for ($c = 1; $c <= $nocourses; $c++) { + ${'course' . $c} = $this->getDataGenerator()->create_course(); + ${'coursecontext' . $c} = context_course::instance(${'course' . $c}->id); + + role_assign($teacherrole->id, $teacher1->id, ${'coursecontext' . $c}->id); + role_assign($studentrole->id, $student->id, ${'coursecontext' . $c}->id); + } + + // Create private notes for student in the course1 and course2 written by the teacher. + $this->help_create_user_note($student->id, NOTES_STATE_DRAFT, $course1->id, + "Test private user note about the student in Course 1 by the teacher"); + $this->help_create_user_note($student->id, NOTES_STATE_DRAFT, $course2->id, + "Test private user note about the student in Course 2 by the teacher"); + // Create public notes for student in the course3 written by the teacher. + $this->help_create_user_note($student->id, NOTES_STATE_PUBLIC, $course3->id, + "Test public user note about the student in Course 3 by the teacher"); + + // The list of users in coursecontext1 should return one user (teacher1). + $userlist1 = new \core_privacy\local\request\userlist($coursecontext1, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(1, $userlist1); + $this->assertTrue(in_array($teacher1->id, $userlist1->get_userids())); + // The list of users in coursecontext2 should return one user (teacher1). + $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(1, $userlist2); + $this->assertTrue(in_array($teacher1->id, $userlist2->get_userids())); + // The list of users in coursecontext3 should return two users (teacher1 and student). + $userlist3 = new \core_privacy\local\request\userlist($coursecontext3, $component); + provider::get_users_in_context($userlist3); + $this->assertCount(2, $userlist3); + $this->assertTrue(in_array($teacher1->id, $userlist3->get_userids())); + $this->assertTrue(in_array($student->id, $userlist3->get_userids())); + + $approvedlist = new approved_userlist($coursecontext3, $component, [$student->id]); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist); + // Re-fetch users in the coursecontext3. + $userlist3 = new \core_privacy\local\request\userlist($coursecontext3, $component); + // The user data in coursecontext3 should not be removed. + provider::get_users_in_context($userlist3); + $this->assertCount(2, $userlist3); + $this->assertTrue(in_array($teacher1->id, $userlist3->get_userids())); + $this->assertTrue(in_array($student->id, $userlist3->get_userids())); + + $approvedlist = new approved_userlist($coursecontext3, $component, [$teacher1->id]); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist); + // Re-fetch users in the coursecontext3. + $userlist3 = new \core_privacy\local\request\userlist($coursecontext3, $component); + // The user data in coursecontext3 should be removed. + provider::get_users_in_context($userlist3); + $this->assertCount(0, $userlist3); + + // Re-fetch users in the coursecontext1. + $userlist1 = new \core_privacy\local\request\userlist($coursecontext1, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(1, $userlist1); + + $approvedlist = new approved_userlist($coursecontext1, $component, [$student->id]); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist); + // Re-fetch users in the coursecontext1. + $userlist3 = new \core_privacy\local\request\userlist($coursecontext1, $component); + // The user data in coursecontext1 should not be removed. + provider::get_users_in_context($userlist3); + $this->assertCount(1, $userlist3); + $this->assertTrue(in_array($teacher1->id, $userlist3->get_userids())); + + $approvedlist = new approved_userlist($coursecontext1, $component, [$teacher1->id]); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist); + // Re-fetch users in the coursecontext1. + $userlist3 = new \core_privacy\local\request\userlist($coursecontext1, $component); + // The user data in coursecontext1 should be removed. + provider::get_users_in_context($userlist3); + $this->assertCount(0, $userlist3); + + // Re-fetch users in the coursecontext2. + $userlist2 = new \core_privacy\local\request\userlist($coursecontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(1, $userlist2); + + // The list of users should not return any users for contexts different than course context. + $systemcontext = context_system::instance(); + $userlist4 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist4); + $this->assertCount(0, $userlist4); + } + /** * Helper function to create user notes for testing. *