diff --git a/course/classes/privacy/provider.php b/course/classes/privacy/provider.php index a4c209d4eff24..b3f13603dd638 100644 --- a/course/classes/privacy/provider.php +++ b/course/classes/privacy/provider.php @@ -35,7 +35,6 @@ /** * Privacy class for requesting user data. * - * @package core_course * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -103,27 +102,47 @@ public static function export_user_data(approved_contextlist $contextlist) { } /** - * Exports course information based on the whole approved context list collection. + * Give the component a chance to include any contextual information deemed relevant to any child contexts which are + * exporting personal data. + * + * By giving the component access to the full list of contexts being exported across all components, it can determine whether a + * descendant context is being exported, and decide whether to add relevant contextual information about itself. Having access + * to the full list of contexts being exported is what makes this component a context aware provider. * - * @param \core_privacy\local\request\contextlist_collection $contextcollection The collection of approved context lists. + * E.g. + * If, during the core export process, a course module is included in the contextlist_collection but the course containing the + * module is not (perhaps there's no longer a user enrolment), then the course should include general contextual information in + * the export so we know basic details about which course the module belongs to. This method allows the course to make that + * decision, based on the existence of any decendant module contexts in the collection. + * + * @param \core_privacy\local\request\contextlist_collection $contextlistcollection */ - public static function export_complete_context_data(\core_privacy\local\request\contextlist_collection $completelist) { + public static function export_context_data(\core_privacy\local\request\contextlist_collection $contextlistcollection) { global $DB; - $coursecontextids = $DB->get_records('context', ['contextlevel' => CONTEXT_COURSE], '', 'id, instanceid'); - + $coursecontextids = $DB->get_records_menu('context', ['contextlevel' => CONTEXT_COURSE], '', 'id, instanceid'); $courseids = []; - foreach ($completelist as $component) { + foreach ($contextlistcollection as $component) { foreach ($component->get_contexts() as $context) { - if ($context->contextlevel == CONTEXT_USER - || $context->contextlevel == CONTEXT_SYSTEM - || $context->contextlevel == CONTEXT_COURSECAT) { - // Move onto the next context as these will not contain course contexts. + // All course contexts have been accounted for, so skip all checks. + if (empty($coursecontextids)) { + break; + } + // Move onto the next context as these will not contain course contexts. + if (in_array($context->contextlevel, [CONTEXT_USER, CONTEXT_SYSTEM, CONTEXT_COURSECAT])) { + continue; + } + // If the context is a course, then we just add it without the need to check context path. + if ($context->contextlevel == CONTEXT_COURSE) { + $courseids[$context->id] = $context->instanceid; + unset($coursecontextids[$context->id]); continue; } - foreach ($coursecontextids as $contextid => $record) { + // Otherwise, we need to check all the course context paths, to see if this context is a descendant. + foreach ($coursecontextids as $contextid => $instanceid) { if (stripos($context->path, '/' . $contextid . '/') !== false) { - $courseids[$contextid] = $record->instanceid; + $courseids[$contextid] = $instanceid; + unset($coursecontextids[$contextid]); } } } @@ -143,7 +162,8 @@ public static function export_complete_context_data(\core_privacy\local\request\ 'fullname' => $course->fullname, 'shortname' => $course->shortname, 'idnumber' => $course->idnumber, - 'summary' => writer::with_context($context)->rewrite_pluginfile_urls([], 'course', 'summary', 0, $course->summary), + 'summary' => writer::with_context($context)->rewrite_pluginfile_urls([], 'course', 'summary', 0, + format_string($course->summary)), 'format' => get_string('pluginname', 'format_' . $course->format), 'startdate' => transform::datetime($course->startdate), 'enddate' => transform::datetime($course->enddate) diff --git a/course/tests/privacy_test.php b/course/tests/privacy_test.php index c8b55b95ab309..b2d903363f0fd 100644 --- a/course/tests/privacy_test.php +++ b/course/tests/privacy_test.php @@ -67,33 +67,67 @@ public function test_export_user_data() { $this->assertCount(2, $completiondata->criteria); } - public function test_export_complete_context_data() { + /** + * Verify that if a module context is included in the contextlist_collection and its parent course is not, the + * export_context_data() call picks this up, and that the contextual course information is included. + */ + public function test_export_context_data_module_context_only() { $this->resetAfterTest(); - $user = $this->getDataGenerator()->create_user(); + + // Create a course and a single module. $course1 = $this->getDataGenerator()->create_course(['fullname' => 'Course 1', 'shortname' => 'C1']); $context1 = context_course::instance($course1->id); - $course2 = $this->getDataGenerator()->create_course(['fullname' => 'Course 2', 'shortname' => 'C2']); - $context2 = context_course::instance($course2->id); - $course3 = $this->getDataGenerator()->create_course(['fullname' => 'Course 3', 'shortname' => 'C3']); - - $this->setUser($user); - $modforum = $this->getDataGenerator()->create_module('forum', ['course' => $course1->id]); - $modresource = $this->getDataGenerator()->create_module('resource', ['course' => $course2->id]); - $modpage = $this->getDataGenerator()->create_module('page', ['course' => $course3->id]); - $forumcontext = context_module::instance($modforum->cmid); - $resourcecontext = context_module::instance($modresource->cmid); + $modassign = $this->getDataGenerator()->create_module('assign', ['course' => $course1->id, 'name' => 'assign test 1']); + $assigncontext = context_module::instance($modassign->cmid); + // Now, let's assume during user info export, only the coursemodule context is returned in the contextlist_collection. + $user = $this->getDataGenerator()->create_user(); $collection = new \core_privacy\local\request\contextlist_collection($user->id); - $approvedlist = new \core_privacy\local\request\approved_contextlist($user, 'mod_forum', [$forumcontext->id]); + $approvedlist = new \core_privacy\local\request\approved_contextlist($user, 'mod_assign', [$assigncontext->id]); $collection->add_contextlist($approvedlist); - $approvedlist = new \core_privacy\local\request\approved_contextlist($user, 'mod_resource', [$resourcecontext->id]); + + // Now, verify that core_course will detect this, and add relevant contextual information. + \core_course\privacy\provider::export_context_data($collection); + $writer = \core_privacy\local\request\writer::with_context($context1); + $this->assertTrue($writer->has_any_data()); + $writerdata = $writer->get_data(); + $this->assertObjectHasAttribute('fullname', $writerdata); + $this->assertObjectHasAttribute('shortname', $writerdata); + $this->assertObjectHasAttribute('idnumber', $writerdata); + $this->assertObjectHasAttribute('summary', $writerdata); + } + + /** + * Verify that if a module context and its parent course context are both included in the contextlist_collection, that course + * contextual information is present in the export. + */ + public function test_export_context_data_course_and_module_contexts() { + $this->resetAfterTest(); + + // Create a course and a single module. + $course1 = $this->getDataGenerator()->create_course(['fullname' => 'Course 1', 'shortname' => 'C1']); + $context1 = context_course::instance($course1->id); + $modassign = $this->getDataGenerator()->create_module('assign', ['course' => $course1->id, 'name' => 'assign test 1']); + $assigncontext = context_module::instance($modassign->cmid); + + // Now, assume during user info export, that both module and course contexts are returned in the contextlist_collection. + $user = $this->getDataGenerator()->create_user(); + $collection = new \core_privacy\local\request\contextlist_collection($user->id); + $approvedlist = new \core_privacy\local\request\approved_contextlist($user, 'mod_assign', [$assigncontext->id]); + $approvedlist2 = new \core_privacy\local\request\approved_contextlist($user, 'core_course', [$context1->id]); $collection->add_contextlist($approvedlist); + $collection->add_contextlist($approvedlist2); - $writer = \core_privacy\local\request\writer::with_context(context_system::instance()); - \core_course\privacy\provider::export_complete_context_data($collection); - $courses = $writer->get_data(); - print_object($courses); - // print_object($writer); + // Now, verify that core_course still adds relevant contextual information, even for courses which are explicitly listed in + // the contextlist_collection. + \core_course\privacy\provider::export_context_data($collection); + $writer = \core_privacy\local\request\writer::with_context($context1); + $this->assertTrue($writer->has_any_data()); + $writerdata = $writer->get_data(); + $this->assertObjectHasAttribute('fullname', $writerdata); + $this->assertObjectHasAttribute('shortname', $writerdata); + $this->assertObjectHasAttribute('idnumber', $writerdata); + $this->assertObjectHasAttribute('summary', $writerdata); } /** diff --git a/privacy/classes/local/request/context_aware_provider.php b/privacy/classes/local/request/context_aware_provider.php index bc5a4b33708d2..01baad9beca50 100644 --- a/privacy/classes/local/request/context_aware_provider.php +++ b/privacy/classes/local/request/context_aware_provider.php @@ -15,8 +15,7 @@ // along with Moodle. If not, see . /** - * This file contains the \core_privacy\local\request\plugin\provider interface to describe - * a class which provides data in some form for a plugin. + * File containing the provider interface for plugins needing access to all approved contexts to fill in relevant contextual data. * * Plugins should implement this if they need access to all approved contexts. * @@ -29,7 +28,7 @@ defined('MOODLE_INTERNAL') || die(); /** - * The provider interface for plugins which need access to all approved contexts to fill in user data. + * The provider interface for plugins which need access to all approved contexts to fill in relevant contextual data. * * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -37,9 +36,14 @@ interface context_aware_provider extends \core_privacy\local\request\core_data_provider { /** - * Export context information based on the whole approved context list collection. + * Give the component a chance to include any contextual information deemed relevant to any child contexts which are + * exporting personal data. + * + * By giving the component access to the full list of contexts being exported across all components, it can determine whether a + * descendant context is being exported, and decide whether to add relevant contextual information about itself. Having access + * to the full list of contexts being exported is what makes this component a context aware provider. * * @param \core_privacy\local\request\contextlist_collection $contextcollection The collection of approved context lists. */ - public static function export_complete_context_data(\core_privacy\local\request\contextlist_collection $contextcollection); + public static function export_context_data(\core_privacy\local\request\contextlist_collection $contextcollection); } diff --git a/privacy/classes/manager.php b/privacy/classes/manager.php index 14f20c7fda39d..0cd66f29bebf3 100644 --- a/privacy/classes/manager.php +++ b/privacy/classes/manager.php @@ -228,13 +228,11 @@ public function export_user_data(contextlist_collection $contextlistcollection) if ($this->component_implements($component, \core_privacy\local\request\user_preference_provider::class)) { $this->get_provider_classname($component)::export_user_preferences($contextlistcollection->get_userid()); } - } - // Check each component for context aware providers. - foreach ($this->get_component_list() as $component) { - // Core user preference providers. + // Contextual information providers. Give each component a chance to include context information based on the + // existence of a child context in the contextlist_collection. if ($this->component_implements($component, \core_privacy\local\request\context_aware_provider::class)) { - $this->get_provider_classname($component)::export_complete_context_data($contextlistcollection); + $this->get_provider_classname($component)::export_context_data($contextlistcollection); } }