From 61fafe8127bf19c1911c994a909e7939b820396b Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 16 Nov 2018 12:18:22 +0800 Subject: [PATCH 1/4] MDL-63969 core_favourites: Add support for removal of context users --- favourites/classes/privacy/provider.php | 110 ++++++++++++- favourites/tests/privacy_test.php | 199 ++++++++++++++++++++++-- 2 files changed, 291 insertions(+), 18 deletions(-) diff --git a/favourites/classes/privacy/provider.php b/favourites/classes/privacy/provider.php index 0d947394566..44a076f6862 100644 --- a/favourites/classes/privacy/provider.php +++ b/favourites/classes/privacy/provider.php @@ -29,6 +29,7 @@ defined('MOODLE_INTERNAL') || die(); use \core_privacy\local\metadata\collection; use \core_privacy\local\request\context; use \core_privacy\local\request\approved_contextlist; +use \core_privacy\local\request\transform; /** * Privacy class for requesting user data. @@ -36,7 +37,10 @@ use \core_privacy\local\request\approved_contextlist; * @copyright 2018 Jake Dallimore * @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\plugin_provider { +class provider implements + \core_privacy\local\metadata\provider, + \core_privacy\local\request\subsystem\plugin_provider, + \core_privacy\local\request\shared_userlist_provider { /** * Returns metadata about this system. @@ -86,13 +90,82 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l FROM {favourite} f WHERE userid = :userid AND component = :component"; + + $params = ['userid' => $userid, 'component' => $component]; + if (!is_null($itemtype)) { $sql .= " AND itemtype = :itemtype"; + $params['itemtype'] = $itemtype; } - $params = ['userid' => $userid, 'component' => $component, 'itemtype' => $itemtype]; + $contextlist->add_from_sql($sql, $params); } + /** + * Add users to a userlist who have favourites within the specified context. + * + * @param \core_privacy\local\request\userlist $userlist The userlist to add the users to. + * @param string $itemtype the type of the favourited items. + * @return void + */ + public static function add_userids_for_context(\core_privacy\local\request\userlist $userlist, + string $itemtype = null) { + if (empty($userlist)) { + return; + } + + $params = [ + 'contextid' => $userlist->get_context()->id, + 'component' => $userlist->get_component() + ]; + + $sql = "SELECT userid + FROM {favourite} + WHERE contextid = :contextid + AND component = :component"; + + if (!is_null($itemtype)) { + $sql .= " AND itemtype = :itemtype"; + $params['itemtype'] = $itemtype; + } + + $userlist->add_from_sql('userid', $sql, $params); + } + + /** + * Get favourites data for the specified user in the specified component, item type and item ID. + * + * @param int $userid The id of the user in scope. + * @param \context $context The context to which data is scoped. + * @param string $component The favourite's component name. + * @param string $itemtype The favourite's item type. + * @param int $itemid The favourite's item ID. + * @return array|null + */ + public static function get_favourites_info_for_user(int $userid, \context $context, + string $component, string $itemtype, int $itemid) { + global $DB; + + $params = [ + 'userid' => $userid, + 'component' => $component, + 'itemtype' => $itemtype, + 'itemid' => $itemid, + 'contextid' => $context->id + ]; + + if (!$favourited = $DB->get_record('favourite', $params)) { + return; + } + + return [ + 'starred' => transform::yesno(true), + 'ordering' => $favourited->ordering, + 'timecreated' => transform::datetime($favourited->timecreated), + 'timemodified' => transform::datetime($favourited->timemodified) + ]; + } + /** * Delete all favourites for all users in the specified contexts, and component area. * @@ -114,6 +187,39 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l $DB->delete_records_select('favourite', $select, $params); } + /** + * Delete all favourites for the specified users in the specified context, component area and item type. + * + * @param \core_privacy\local\request\approved_userlist $userlist The approved contexts and user information + * to delete information for. + * @param string $itemtype The favourite's itemtype. + * @throws \dml_exception if any errors are encountered during deletion. + */ + public static function delete_favourites_for_userlist(\core_privacy\local\request\approved_userlist $userlist, + string $itemtype) { + global $DB; + + $userids = $userlist->get_userids(); + + if (empty($userids)) { + return; + } + + $context = $userlist->get_context(); + list($usersql, $userparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED); + + $params = [ + 'component' => $userlist->get_component(), + 'itemtype' => $itemtype, + 'contextid' => $context->id + ]; + + $params += $userparams; + $select = "component = :component AND itemtype = :itemtype AND contextid = :contextid AND userid $usersql"; + + $DB->delete_records_select('favourite', $select, $params); + } + /** * Delete all favourites for the specified user, in the specified contexts. * diff --git a/favourites/tests/privacy_test.php b/favourites/tests/privacy_test.php index 409282bae80..75076890935 100644 --- a/favourites/tests/privacy_test.php +++ b/favourites/tests/privacy_test.php @@ -27,6 +27,7 @@ defined('MOODLE_INTERNAL') || die(); use \core_privacy\tests\provider_testcase; use \core_favourites\privacy\provider; +use \core_privacy\local\request\transform; /** * Unit tests for favourites/classes/privacy/provider @@ -65,25 +66,25 @@ class privacy_test extends provider_testcase { $ufservice1 = \core_favourites\service_factory::get_service_for_user_context($user1context); $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2context); $systemcontext = context_system::instance(); - $ufservice1->create_favourite('core_course', 'course', $course1context->instanceid, $systemcontext); - $ufservice1->create_favourite('core_course', 'course', $course2context->instanceid, $systemcontext); - $ufservice2->create_favourite('core_course', 'course', $course2context->instanceid, $systemcontext); - $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'course')); - $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'course')); + $ufservice1->create_favourite('core_course', 'courses', $course1context->instanceid, $systemcontext); + $ufservice1->create_favourite('core_course', 'courses', $course2context->instanceid, $systemcontext); + $ufservice2->create_favourite('core_course', 'courses', $course2context->instanceid, $systemcontext); + $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'courses')); + $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'courses')); // Now, just for variety, let's assume you can favourite a course at user context, and do so for user1. - $ufservice1->create_favourite('core_course', 'course', $course1context->instanceid, $user1context); + $ufservice1->create_favourite('core_course', 'courses', $course1context->instanceid, $user1context); // Now, ask the favourites privacy api to export contexts for favourites of the type we just created, for user1. $contextlist = new \core_privacy\local\request\contextlist(); - \core_favourites\privacy\provider::add_contexts_for_userid($contextlist, $user1->id, 'core_course', 'course'); + \core_favourites\privacy\provider::add_contexts_for_userid($contextlist, $user1->id, 'core_course', 'courses'); // Verify we have two contexts in the list for user1. $this->assertCount(2, $contextlist->get_contextids()); // And verify we only have the system context returned for user2. $contextlist = new \core_privacy\local\request\contextlist(); - \core_favourites\privacy\provider::add_contexts_for_userid($contextlist, $user2->id, 'core_course', 'course'); + \core_favourites\privacy\provider::add_contexts_for_userid($contextlist, $user2->id, 'core_course', 'courses'); $this->assertCount(1, $contextlist->get_contextids()); } @@ -96,19 +97,19 @@ class privacy_test extends provider_testcase { // Favourite 2 courses for user1 and 1 course for user2, all at the user context. $ufservice1 = \core_favourites\service_factory::get_service_for_user_context($user1context); $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2context); - $ufservice1->create_favourite('core_course', 'course', $course1context->instanceid, $user1context); - $ufservice1->create_favourite('core_course', 'course', $course2context->instanceid, $user1context); - $ufservice2->create_favourite('core_course', 'course', $course2context->instanceid, $user2context); - $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'course')); - $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'course')); + $ufservice1->create_favourite('core_course', 'courses', $course1context->instanceid, $user1context); + $ufservice1->create_favourite('core_course', 'courses', $course2context->instanceid, $user1context); + $ufservice2->create_favourite('core_course', 'courses', $course2context->instanceid, $user2context); + $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'courses')); + $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'courses')); // Now, delete the favourites for user1 only. $approvedcontextlist = new \core_privacy\local\request\approved_contextlist($user1, 'core_course', [$user1context->id]); - provider::delete_favourites_for_user($approvedcontextlist, 'core_course', 'course'); + provider::delete_favourites_for_user($approvedcontextlist, 'core_course', 'courses'); // Verify that we have no favourite courses for user1 but that the records are in tact for user2. - $this->assertCount(0, $ufservice1->find_favourites_by_type('core_course', 'course')); - $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'course')); + $this->assertCount(0, $ufservice1->find_favourites_by_type('core_course', 'courses')); + $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'courses')); } public function test_delete_favourites_for_all_users() { @@ -134,4 +135,170 @@ class privacy_test extends provider_testcase { $this->assertCount(0, $ufservice1->find_favourites_by_type('core_course', 'modules')); $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'modules')); } + + /** + * Test confirming that user ID's of favourited items can be added to the userlist. + */ + public function test_add_userids_for_context() { + list($user1, $user2, $user1context, $user2context, $course1context, $course2context) = $this->set_up_courses_and_users(); + + // Favourite 2 courses for user1 and 1 course for user2, all at the site context. + $ufservice1 = \core_favourites\service_factory::get_service_for_user_context($user1context); + $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2context); + $systemcontext = context_system::instance(); + $ufservice1->create_favourite('core_course', 'courses', $course1context->instanceid, $systemcontext); + $ufservice1->create_favourite('core_course', 'courses', $course2context->instanceid, $systemcontext); + $ufservice2->create_favourite('core_course', 'courses', $course2context->instanceid, $systemcontext); + $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'courses')); + $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'courses')); + + // Now, just for variety, let's assume you can favourite a course at user context, and do so for user1. + $ufservice1->create_favourite('core_course', 'courses', $course1context->instanceid, $user1context); + + // Now, ask the favourites privacy api to export userids for favourites of the type we just created, in the system context. + $userlist = new \core_privacy\local\request\userlist($systemcontext, 'core_course'); + provider::add_userids_for_context($userlist, 'courses'); + // Verify we have two userids in the list for system context. + $this->assertCount(2, $userlist->get_userids()); + $expected = [ + $user1->id, + $user2->id + ]; + $this->assertEquals($expected, $userlist->get_userids(), '', 0.0, 10, true); + + // Ask the favourites privacy api to export userids for favourites of the type we just created, in the user1 context. + $userlist = new \core_privacy\local\request\userlist($user1context, 'core_course'); + provider::add_userids_for_context($userlist, 'courses'); + // Verify we have one userid in the list for user1 context. + $this->assertCount(1, $userlist->get_userids()); + $expected = [$user1->id]; + $this->assertEquals($expected, $userlist->get_userids()); + + // Ask the favourites privacy api to export userids for favourites of the type we just created, in the user2 context. + $userlist = new \core_privacy\local\request\userlist($user2context, 'core_favourites'); + provider::add_userids_for_context($userlist, 'core_course', 'courses'); + // Verify we do not have any userids in the list for user2 context. + $this->assertCount(0, $userlist->get_userids()); + } + + /** + * Test deletion of user favourites based on an approved_userlist, component area and item type. + */ + public function test_delete_favourites_for_userlist() { + list($user1, $user2, $user1context, $user2context, $course1context, $course2context) = $this->set_up_courses_and_users(); + + // Favourite 2 courses for user1 and 1 course for user2. + $systemcontext = context_system::instance(); + $ufservice1 = \core_favourites\service_factory::get_service_for_user_context($user1context); + $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2context); + $ufservice1->create_favourite('core_course', 'courses', $course1context->instanceid, $systemcontext); + $ufservice1->create_favourite('core_course', 'courses', $course2context->instanceid, $user1context); + $ufservice2->create_favourite('core_course', 'courses', $course2context->instanceid, $systemcontext); + $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'courses')); + $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'courses')); + + // Ask the favourites privacy api to export userids for favourites of the type we just created, in the system context. + $userlist1 = new \core_privacy\local\request\userlist($systemcontext, 'core_course'); + provider::add_userids_for_context($userlist1, 'courses'); + // Verify we have two userids in the list for system context. + $this->assertCount(2, $userlist1->get_userids()); + + // Ask the favourites privacy api to export userids for favourites of the type we just created, in the user1 context. + $userlist2 = new \core_privacy\local\request\userlist($user1context, 'core_course'); + provider::add_userids_for_context($userlist2, 'courses'); + // Verify we have one userid in the list for user1 context. + $this->assertCount(1, $userlist2->get_userids()); + + // Now, delete the favourites for user1 only in the system context. + $approveduserlist = new \core_privacy\local\request\approved_userlist($systemcontext, 'core_course', + [$user1->id]); + provider::delete_favourites_for_userlist($approveduserlist, 'courses'); + // Ensure user1's data was deleted and user2 is still returned for system context. + $userlist1 = new \core_privacy\local\request\userlist($systemcontext, 'core_course'); + provider::add_userids_for_context($userlist1, 'courses'); + $this->assertCount(1, $userlist1->get_userids()); + // Verify that user2 is still in the list for system context. + $expected = [$user2->id]; + $this->assertEquals($expected, $userlist1->get_userids()); + // Verify that the data of user1 was not deleted in the user1context. + $userlist2 = new \core_privacy\local\request\userlist($user1context, 'core_course'); + provider::add_userids_for_context($userlist2, 'courses'); + $expected = [$user1->id]; + $this->assertEquals($expected, $userlist2->get_userids()); + + // Now, delete the favourites for user2 only in the user1 context. + // Make sure favourites are only being deleted in the right context. + $approveduserlist = new \core_privacy\local\request\approved_userlist($user1context, 'core_course', + [$user2->id]); + provider::delete_favourites_for_userlist($approveduserlist, 'courses'); + // Verify we have one userid in the list for system context. + $userlist2 = new \core_privacy\local\request\userlist($systemcontext, 'core_course'); + provider::add_userids_for_context($userlist2, 'courses'); + $this->assertCount(1, $userlist2->get_userids()); + // Verify that user2 is still in the list for system context. + $expected = [$user2->id]; + $this->assertEquals($expected, $userlist2->get_userids()); + + // Verify that user1 is still present in the list for user1 context. + $userlist3 = new \core_privacy\local\request\userlist($user1context, 'core_course'); + provider::add_userids_for_context($userlist3, 'courses'); + $this->assertCount(1, $userlist3->get_userids()); + // Verify that user1 is still in the list for user1 context. + $expected = [$user1->id]; + $this->assertEquals($expected, $userlist3->get_userids()); + } + + /** + * Test fetching the favourites data for a specified user in a specified component, item type and item ID. + */ + public function test_get_favourites_info_for_user() { + list($user1, $user2, $user1context, $user2context, $course1context, $course2context) = $this->set_up_courses_and_users(); + + // Favourite 2 courses for user1 and 1 course for user2. + $ufservice1 = \core_favourites\service_factory::get_service_for_user_context($user1context); + $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2context); + $coursefavourite1 = $ufservice1->create_favourite('core_course', 'courses', + $course1context->instanceid, $course1context); + $this->waitForSecond(); + $coursefavourite2 = $ufservice1->create_favourite('core_course', 'courses', + $course2context->instanceid, $course2context); + $this->waitForSecond(); + $coursefavourite3 = $ufservice2->create_favourite('core_course', 'courses', + $course2context->instanceid, $course2context); + $this->assertCount(2, $ufservice1->find_favourites_by_type('core_course', 'courses')); + $this->assertCount(1, $ufservice2->find_favourites_by_type('core_course', 'courses')); + + // Get the favourites info for user1 in the course1 context. + $favouriteinfo1 = (object) provider::get_favourites_info_for_user($user1->id, $course1context, + 'core_course', 'courses', $course1context->instanceid); + // Ensure the correct data has been returned. + $this->assertEquals(transform::yesno(true), $favouriteinfo1->starred); + $this->assertEquals('', $favouriteinfo1->ordering); + $this->assertEquals(transform::datetime($coursefavourite1->timecreated), $favouriteinfo1->timecreated); + $this->assertEquals(transform::datetime($coursefavourite1->timemodified), $favouriteinfo1->timemodified); + + // Get the favourites info for user1 in the course2 context. + $favouriteinfo2 = (object) provider::get_favourites_info_for_user($user1->id, $course2context, + 'core_course', 'courses', $course2context->instanceid); + // Ensure the correct data has been returned. + $this->assertEquals(transform::yesno(true), $favouriteinfo2->starred); + $this->assertEquals('', $favouriteinfo2->ordering); + $this->assertEquals(transform::datetime($coursefavourite2->timecreated), $favouriteinfo2->timecreated); + $this->assertEquals(transform::datetime($coursefavourite2->timemodified), $favouriteinfo2->timemodified); + + // Get the favourites info for user2 in the course2 context. + $favouriteinfo3 = (object) provider::get_favourites_info_for_user($user2->id, $course2context, + 'core_course', 'courses', $course2context->instanceid); + // Ensure the correct data has been returned. + $this->assertEquals(transform::yesno(true), $favouriteinfo3->starred); + $this->assertEquals('', $favouriteinfo3->ordering); + $this->assertEquals(transform::datetime($coursefavourite3->timecreated), $favouriteinfo3->timecreated); + $this->assertEquals(transform::datetime($coursefavourite3->timemodified), $favouriteinfo3->timemodified); + + // Get the favourites info for user2 in the course1 context (user2 has not favourited course1). + $favouriteinfo4 = provider::get_favourites_info_for_user($user2->id, $course1context, + 'core_course', 'courses', $course1context->instanceid); + // Ensure that data has not been returned. + $this->assertEmpty($favouriteinfo4); + } } From 4f8c9232f14fe7ea6850cdd045d6a55f35d832aa Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 16 Nov 2018 12:18:58 +0800 Subject: [PATCH 2/4] MDL-63969 core_course: Include core_favourites subsystem in provider --- course/classes/privacy/provider.php | 42 ++++++++++++++++++++++------- lang/en/course.php | 2 ++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/course/classes/privacy/provider.php b/course/classes/privacy/provider.php index a0808c166f9..edeb50cb1e4 100644 --- a/course/classes/privacy/provider.php +++ b/course/classes/privacy/provider.php @@ -55,6 +55,7 @@ class provider implements */ public static function get_metadata(collection $collection) : collection { $collection->add_subsystem_link('core_completion', [], 'privacy:metadata:completionsummary'); + $collection->add_subsystem_link('core_favourites', [], 'privacy:metadata:favouritessummary'); $collection->add_user_preference('coursecat_management_perpage', 'privacy:perpage'); return $collection; } @@ -75,6 +76,9 @@ class provider implements $params['contextcourse'] = CONTEXT_COURSE; $contextlist = new contextlist(); $contextlist->add_from_sql($sql, $params); + + \core_favourites\privacy\provider::add_contexts_for_userid($contextlist, $userid, 'core_course', 'courses'); + return $contextlist; } @@ -86,11 +90,12 @@ class provider implements public static function get_users_in_context(userlist $userlist) { $context = $userlist->get_context(); - if (!is_a($context, \context_course::class)) { + if (!$context instanceof \context_course) { return; } \core_completion\privacy\provider::add_course_completion_users_to_userlist($userlist); + \core_favourites\privacy\provider::add_userids_for_context($userlist, 'courses'); } /** @@ -111,10 +116,18 @@ class provider implements WHERE ctx.id $select"; $courses = $DB->get_recordset_sql($sql, $params); + foreach ($courses as $course) { $coursecompletion = \core_completion\privacy\provider::get_course_completion_info($contextlist->get_user(), $course); writer::with_context(\context_course::instance($course->id))->export_data( [get_string('privacy:completionpath', 'course')], (object) $coursecompletion); + // Get user's favourites information for the particular course. + $coursefavourite = \core_favourites\privacy\provider::get_favourites_info_for_user($contextlist->get_user()->id, + \context_course::instance($course->id), 'core_course', 'courses', $course->id); + if ($coursefavourite) { // If the course has been favourited by the user, include it in the export. + writer::with_context(\context_course::instance($course->id))->export_data( + [get_string('privacy:favouritespath', 'course')], (object) $coursefavourite); + } } $courses->close(); } @@ -217,10 +230,14 @@ class provider implements */ public static function delete_data_for_all_users_in_context(\context $context) { // Check what context we've been delivered. - if ($context->contextlevel == CONTEXT_COURSE) { - // Delete course completion data. - \core_completion\privacy\provider::delete_completion(null, $context->instanceid); + if (!$context instanceof \context_course) { + return; } + // Delete course completion data. + \core_completion\privacy\provider::delete_completion(null, $context->instanceid); + // Delete course favourite data. + \core_favourites\privacy\provider::delete_favourites_for_all_users($context, 'core_course', + 'courses'); } /** @@ -230,9 +247,13 @@ class provider implements */ public static function delete_data_for_user(approved_contextlist $contextlist) { foreach ($contextlist as $context) { - if ($context->contextlevel == CONTEXT_COURSE) { + // Check what context we've been delivered. + if ($context instanceof \context_course) { // Delete course completion data. \core_completion\privacy\provider::delete_completion($contextlist->get_user(), $context->instanceid); + // Delete course favourite data. + \core_favourites\privacy\provider::delete_favourites_for_user($contextlist, 'core_course', + 'courses'); } } } @@ -243,12 +264,15 @@ class provider implements * @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) { - // Delete course completion data. - \core_completion\privacy\provider::delete_completion_by_approved_userlist($userlist, $context->instanceid); + // Check what context we've been delivered. + if (!$context instanceof \context_course) { + return; } + // Delete course completion data. + \core_completion\privacy\provider::delete_completion_by_approved_userlist($userlist, $context->instanceid); + // Delete course favourite data. + \core_favourites\privacy\provider::delete_favourites_for_userlist($userlist, 'courses'); } } diff --git a/lang/en/course.php b/lang/en/course.php index bf8d76e62d0..3e32f80962b 100644 --- a/lang/en/course.php +++ b/lang/en/course.php @@ -28,4 +28,6 @@ $string['aria:favourite'] = 'Course is starred'; $string['favourite'] = 'Starred course'; $string['privacy:perpage'] = 'The number of courses to show per page.'; $string['privacy:completionpath'] = 'Course completion'; +$string['privacy:favouritespath'] = 'Course starred information'; $string['privacy:metadata:completionsummary'] = 'The course contains completion information about the user.'; +$string['privacy:metadata:favouritessummary'] = 'The course contains information relating to the course being starred by the user.'; From 214696b04f2b79c04c289ff7d22a54a29e8bcab6 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 16 Nov 2018 12:19:46 +0800 Subject: [PATCH 3/4] MDL-63969 core_course: Update course privacy provider unit tests --- course/tests/privacy_test.php | 295 ++++++++++++++++++++++++++++++---- 1 file changed, 262 insertions(+), 33 deletions(-) diff --git a/course/tests/privacy_test.php b/course/tests/privacy_test.php index 25f60f490c4..512a8d852f7 100644 --- a/course/tests/privacy_test.php +++ b/course/tests/privacy_test.php @@ -27,6 +27,8 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->dirroot . '/completion/tests/fixtures/completion_creation.php'); +use \core_privacy\local\request\transform; + /** * Unit tests for course/classes/privacy/policy * @@ -43,11 +45,45 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase */ public function test_get_contexts_for_userid() { $this->resetAfterTest(); - $user = $this->getDataGenerator()->create_user(); + + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + + // Make sure contexts are not being returned for user1. + $contextlist = \core_course\privacy\provider::get_contexts_for_userid($user1->id); + $this->assertCount(0, $contextlist->get_contextids()); + + // Make sure contexts are not being returned for user2. + $contextlist = \core_course\privacy\provider::get_contexts_for_userid($user2->id); + $this->assertCount(0, $contextlist->get_contextids()); + + // Create course completion data for user1. $this->create_course_completion(); - $this->complete_course($user); - $contextlist = \core_course\privacy\provider::get_contexts_for_userid($user->id); - $this->assertEquals($this->coursecontext->id, $contextlist->current()->id); + $this->complete_course($user1); + + // Make sure the course context is being returned for user1. + $contextlist = \core_course\privacy\provider::get_contexts_for_userid($user1->id); + $expected = [$this->coursecontext->id]; + $actual = $contextlist->get_contextids(); + $this->assertCount(1, $actual); + $this->assertEquals($expected, $actual); + + // Make sure contexts are still not being returned for user2. + $contextlist = \core_course\privacy\provider::get_contexts_for_userid($user2->id); + $this->assertCount(0, $contextlist->get_contextids()); + + // User2 has a favourite course. + $user2context = \context_user::instance($user2->id); + $ufservice = \core_favourites\service_factory::get_service_for_user_context($user2context); + $ufservice->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + + // Make sure the course context is being returned for user2. + $contextlist = \core_course\privacy\provider::get_contexts_for_userid($user2->id); + $expected = [$this->coursecontext->id]; + $actual = $contextlist->get_contextids(); + $this->assertCount(1, $actual); + $this->assertEquals($expected, $actual); } /** @@ -60,6 +96,7 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase $user1 = $this->getDataGenerator()->create_user(); $user2 = $this->getDataGenerator()->create_user(); $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); // User1 and user2 complete course. $this->create_course_completion(); @@ -69,15 +106,32 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase // User3 is enrolled but has not completed course. $this->getDataGenerator()->enrol_user($user3->id, $this->course->id, 'student'); - // Ensure only users that have course completion are returned. + // User4 has a favourited course. + $systemcontext = \context_system::instance(); + $user4ctx = \context_user::instance($user4->id); + $ufservice = \core_favourites\service_factory::get_service_for_user_context($user4ctx); + $ufservice->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + + // Ensure only users that have course completion or favourites are returned. $userlist = new \core_privacy\local\request\userlist($this->coursecontext, $component); \core_course\privacy\provider::get_users_in_context($userlist); - $expected = [$user1->id, $user2->id]; + $expected = [ + $user1->id, + $user2->id, + $user4->id + ]; $actual = $userlist->get_userids(); sort($expected); sort($actual); - $this->assertCount(2, $actual); + $this->assertCount(3, $actual); $this->assertEquals($expected, $actual); + + // Ensure that users are not being returned in other contexts than the course context. + $userlist = new \core_privacy\local\request\userlist($systemcontext, $component); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $this->assertCount(0, $actual); } /** @@ -85,6 +139,7 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase */ public function test_export_user_data() { $this->resetAfterTest(); + $user = $this->getDataGenerator()->create_user(); $this->create_course_completion(); $this->complete_course($user); @@ -95,6 +150,22 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase $completiondata = $writer->get_data([get_string('privacy:completionpath', 'course')]); $this->assertEquals('In progress', $completiondata->status); $this->assertCount(2, $completiondata->criteria); + + // User has a favourite course. + $usercontext = \context_user::instance($user->id); + $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext); + $favourite = $ufservice->create_favourite('core_course', 'courses', + $this->coursecontext->instanceid, $this->coursecontext); + + // Ensure that user's favourites data in the course context is being exported. + $writer = \core_privacy\local\request\writer::with_context($this->coursecontext); + \core_course\privacy\provider::export_user_data($approvedlist); + $favouritedata = $writer->get_data([get_string('privacy:favouritespath', 'course')]); + + $this->assertEquals(transform::yesno(true), $favouritedata->starred); + $this->assertEquals('', $favouritedata->ordering); + $this->assertEquals(transform::datetime($favourite->timecreated), $favouritedata->timecreated); + $this->assertEquals(transform::datetime($favourite->timemodified), $favouritedata->timemodified); } /** @@ -165,52 +236,169 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase */ public function test_delete_data_for_all_users_in_context() { global $DB; + $this->resetAfterTest(); + $user1 = $this->getDataGenerator()->create_user(); $user2 = $this->getDataGenerator()->create_user(); $this->create_course_completion(); + + $systemcontext = \context_system::instance(); + $user1ctx = \context_user::instance($user1->id); + $user2ctx = \context_user::instance($user2->id); + // User1 and user2 have a favourite course. + $ufservice1 = \core_favourites\service_factory::get_service_for_user_context($user1ctx); + $ufservice1->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2ctx); + $ufservice2->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + + // Ensure only users that have course favourites are returned in the course context (user1 and user2). + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $this->assertCount(2, $actual); + + // Ensure the users does not have a course completion data. + $records = $DB->get_records('course_modules_completion'); + $this->assertCount(0, $records); + $records = $DB->get_records('course_completion_crit_compl'); + $this->assertCount(0, $records); + + // Create course completions for user1 and users. $this->complete_course($user1); $this->complete_course($user2); $records = $DB->get_records('course_modules_completion'); $this->assertCount(2, $records); $records = $DB->get_records('course_completion_crit_compl'); $this->assertCount(2, $records); + + // Delete data for all users in a context different than the course context (system context). + \core_course\privacy\provider::delete_data_for_all_users_in_context($systemcontext); + + // Ensure the data in the course context has not been deleted. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $this->assertCount(2, $actual); + + // Delete data for all users in the course context. \core_course\privacy\provider::delete_data_for_all_users_in_context($this->coursecontext); + + // Ensure the completion data has been removed in the course context. $records = $DB->get_records('course_modules_completion'); $this->assertCount(0, $records); $records = $DB->get_records('course_completion_crit_compl'); $this->assertCount(0, $records); + + // Ensure that users are not returned after the deletion in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $this->assertCount(0, $actual); } /** * Test deleting data for only one user. */ public function test_delete_data_for_user() { - global $DB; $this->resetAfterTest(); + $user1 = $this->getDataGenerator()->create_user(); $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + + // Create course completion for user1. $this->create_course_completion(); $this->complete_course($user1); - $this->complete_course($user2); - $records = $DB->get_records('course_modules_completion'); - $this->assertCount(2, $records); - $records = $DB->get_records('course_completion_crit_compl'); - $this->assertCount(2, $records); + + // Ensure user1 is returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [$user1->id]; + $this->assertCount(1, $actual); + $this->assertEquals($expected, $actual); + + // User2 and user3 have a favourite course. + $systemcontext = \context_system::instance(); + $user2ctx = \context_user::instance($user2->id); + $user3ctx = \context_user::instance($user3->id); + $ufservice2 = \core_favourites\service_factory::get_service_for_user_context($user2ctx); + $ufservice2->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + $ufservice3 = \core_favourites\service_factory::get_service_for_user_context($user3ctx); + $ufservice3->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + + // Ensure user1, user2 and user3 are returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user1->id, + $user2->id, + $user3->id + ]; + sort($expected); + sort($actual); + $this->assertCount(3, $actual); + $this->assertEquals($expected, $actual); + + // Delete user1's data in the course context. $approvedlist = new \core_privacy\local\request\approved_contextlist($user1, 'core_course', [$this->coursecontext->id]); \core_course\privacy\provider::delete_data_for_user($approvedlist); - $records = $DB->get_records('course_modules_completion'); - $this->assertCount(1, $records); - $records = $DB->get_records('course_completion_crit_compl'); - $this->assertCount(1, $records); + + // Ensure user1's data is deleted and only user2 and user3 are returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user2->id, + $user3->id + ]; + sort($expected); + sort($actual); + $this->assertEquals($expected, $actual); + + // Delete user2's data in a context different than the course context (system context). + $approvedlist = new \core_privacy\local\request\approved_contextlist($user2, 'core_course', + [$systemcontext->id]); + \core_course\privacy\provider::delete_data_for_user($approvedlist); + + // Ensure user2 and user3 are still returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user2->id, + $user3->id + ]; + sort($expected); + sort($actual); + $this->assertEquals($expected, $actual); + + // Delete user2's data in the course context. + $approvedlist = new \core_privacy\local\request\approved_contextlist($user2, 'core_course', + [$this->coursecontext->id]); + \core_course\privacy\provider::delete_data_for_user($approvedlist); + + // Ensure user2's is deleted and user3 is still returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user3->id + ]; + $this->assertEquals($expected, $actual); } /** * Test deleting data within a context for an approved userlist. */ public function test_delete_data_for_users() { - global $DB; $this->resetAfterTest(); $component = 'core_course'; @@ -221,26 +409,67 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase $this->create_course_completion(); $this->complete_course($user1); $this->complete_course($user2); - $this->complete_course($user3); - // Ensure records exist for all users before delete. - $records = $DB->get_records('course_modules_completion'); - $this->assertCount(3, $records); - $records = $DB->get_records('course_completion_crit_compl'); - $this->assertCount(3, $records); + // Ensure user1, user2 are returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user1->id, + $user2->id + ]; + sort($expected); + sort($actual); + $this->assertCount(2, $actual); + $this->assertEquals($expected, $actual); + $systemcontext = \context_system::instance(); + // User3 has a favourite course. + $user3ctx = \context_user::instance($user3->id); + $ufservice = \core_favourites\service_factory::get_service_for_user_context($user3ctx); + $ufservice->create_favourite('core_course', 'courses', $this->coursecontext->instanceid, + $this->coursecontext); + + // Ensure user1, user2 and user3 are now returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user1->id, + $user2->id, + $user3->id + ]; + sort($expected); + sort($actual); + $this->assertCount(3, $actual); + $this->assertEquals($expected, $actual); + + // Delete data for user1 and user3 in the course context. $approveduserids = [$user1->id, $user3->id]; $approvedlist = new \core_privacy\local\request\approved_userlist($this->coursecontext, $component, $approveduserids); \core_course\privacy\provider::delete_data_for_users($approvedlist); - // Ensure content is only deleted for approved userlist. - $records = $DB->get_records('course_modules_completion'); - $this->assertCount(1, $records); - $record = reset($records); - $this->assertEquals($user2->id, $record->userid); - $records = $DB->get_records('course_completion_crit_compl'); - $this->assertCount(1, $records); - $record = reset($records); - $this->assertEquals($user2->id, $record->userid); + // Ensure user1 and user3 are deleted and user2 is still returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [$user2->id]; + $this->assertCount(1, $actual); + $this->assertEquals($expected, $actual); + + // Try to delete user2's data in a context different than course (system context). + $approveduserids = [$user2->id]; + $approvedlist = new \core_privacy\local\request\approved_userlist($systemcontext, $component, $approveduserids); + \core_course\privacy\provider::delete_data_for_users($approvedlist); + + // Ensure user2 is still returned in the course context. + $userlist = new \core_privacy\local\request\userlist($this->coursecontext, 'core_course'); + \core_course\privacy\provider::get_users_in_context($userlist); + $actual = $userlist->get_userids(); + $expected = [ + $user2->id + ]; + $this->assertCount(1, $actual); + $this->assertEquals($expected, $actual); } } From 329f1f23ff028ed5a45ca861ef0108720aea52b2 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 16 Nov 2018 12:20:17 +0800 Subject: [PATCH 4/4] MDL-63969 core_course: Store favourite courses in the course context The course favourites were previously set in the system context which is not right as they should be stored in the course context. --- course/externallib.php | 9 ++++++--- lib/db/upgrade.php | 13 +++++++++++++ version.php | 2 +- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/course/externallib.php b/course/externallib.php index 8d5997663df..81e21ababe5 100644 --- a/course/externallib.php +++ b/course/externallib.php @@ -3823,12 +3823,14 @@ class core_course_external extends external_api { $warning = []; - $favouriteexists = $ufservice->favourite_exists('core_course', 'courses', $course['id'], \context_system::instance()); + $favouriteexists = $ufservice->favourite_exists('core_course', 'courses', $course['id'], + \context_course::instance($course['id'])); if ($course['favourite']) { if (!$favouriteexists) { try { - $ufservice->create_favourite('core_course', 'courses', $course['id'], \context_system::instance()); + $ufservice->create_favourite('core_course', 'courses', $course['id'], + \context_course::instance($course['id'])); } catch (Exception $e) { $warning['courseid'] = $course['id']; if ($e instanceof moodle_exception) { @@ -3849,7 +3851,8 @@ class core_course_external extends external_api { } else { if ($favouriteexists) { try { - $ufservice->delete_favourite('core_course', 'courses', $course['id'], \context_system::instance()); + $ufservice->delete_favourite('core_course', 'courses', $course['id'], + \context_course::instance($course['id'])); } catch (Exception $e) { $warning['courseid'] = $course['id']; if ($e instanceof moodle_exception) { diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 9b9fb47a21e..350bf04a61b 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2771,5 +2771,18 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2018110700.01); } + if ($oldversion < 2018111300.01) { + // Update favourited courses, so they are saved in the particular course context instead of the system. + $favouritedcourses = $DB->get_records('favourite', ['component' => 'core_course', 'itemtype' => 'courses']); + + foreach ($favouritedcourses as $fc) { + $coursecontext = \context_course::instance($fc->itemid); + $fc->contextid = $coursecontext->id; + $DB->update_record('favourite', $fc); + } + + upgrade_main_savepoint(true, 2018111300.01); + } + return true; } diff --git a/version.php b/version.php index 6a8ab5b3d7b..d60bf4f7f7b 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018111300.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2018111300.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.