Skip to content

Commit

Permalink
MDL-62029 core_course: Fixes to context-aware provider implementation.
Browse files Browse the repository at this point in the history
  • Loading branch information
@snake
snake committed May 9, 2018
1 parent 5701891 commit 791824e
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 43 deletions.
48 changes: 34 additions & 14 deletions course/classes/privacy/provider.php
View file
Expand Up @@ -35,7 +35,6 @@
/**
* Privacy class for requesting user data.
*
* @package core_course
* @copyright 2018 Adrian Greeve <adrian@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
Expand Down Expand Up @@ -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]);
}
}
}
Expand All @@ -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)
Expand Down
72 changes: 53 additions & 19 deletions course/tests/privacy_test.php
View file
Expand Up @@ -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);
}

/**
Expand Down
14 changes: 9 additions & 5 deletions privacy/classes/local/request/context_aware_provider.php
View file
Expand Up @@ -15,8 +15,7 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* 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.
*
Expand All @@ -29,17 +28,22 @@
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 <adriangreeve.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
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);
}
8 changes: 3 additions & 5 deletions privacy/classes/manager.php
View file
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 791824e

Please sign in to comment.