From 60bf7bf6c26474c6ecaac0f99af7ff9490906bf4 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 28 Feb 2018 15:42:53 +0800 Subject: [PATCH] MDL-61475 mod_choice: Update core_privacy implementation --- mod/choice/classes/privacy/provider.php | 125 +++++++++++---------- mod/choice/tests/privacy_provider_test.php | 41 ++++--- 2 files changed, 91 insertions(+), 75 deletions(-) diff --git a/mod/choice/classes/privacy/provider.php b/mod/choice/classes/privacy/provider.php index 4dce12679e8..270ce689723 100644 --- a/mod/choice/classes/privacy/provider.php +++ b/mod/choice/classes/privacy/provider.php @@ -24,14 +24,12 @@ namespace mod_choice\privacy; -use context_module; -use core_privacy\metadata\item_collection; -use core_privacy\metadata\provider as core_provider; -use core_privacy\request\approved_contextlist; -use core_privacy\request\contextlist; -use core_privacy\request\deletion_criteria; -use core_privacy\request\plugin\provider as plugin_provider; -use core_privacy\request\writer; +use core_privacy\local\metadata\collection; +use core_privacy\local\request\approved_contextlist; +use core_privacy\local\request\contextlist; +use core_privacy\local\request\deletion_criteria; +use core_privacy\local\request\helper; +use core_privacy\local\request\writer; defined('MOODLE_INTERNAL') || die(); @@ -41,11 +39,19 @@ defined('MOODLE_INTERNAL') || die(); * @copyright 2018 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements core_provider, plugin_provider { +class provider implements + // This plugin stores personal data. + \core_privacy\local\metadata\provider, + + // This plugin is a core_user_data_provider. + \core_privacy\local\request\plugin\provider { /** - * @inheritdoc + * Return the fields which contain personal data. + * + * @param collection $items a reference to the collection to use to store the metadata. + * @return collection the updated collection of metadata items. */ - public static function get_metadata(item_collection $items) : item_collection { + public static function get_metadata(collection $items) { $items->add_database_table( 'choice_answers', [ @@ -61,17 +67,20 @@ class provider implements core_provider, plugin_provider { } /** - * @inheritdoc + * Get the list of contexts that contain user information for the specified user. + * + * @param int $userid the userid. + * @return contextlist the list of contexts containing user info for the user. */ - public static function get_contexts_for_userid(int $userid) : contextlist { + public static function get_contexts_for_userid($userid) { // Fetch all choice answers. $sql = "SELECT c.id FROM {context} c INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname INNER JOIN {choice} ch ON ch.id = cm.instance - LEFT JOIN {choice_options} co ON co.choiceid = ch.id - LEFT JOIN {choice_answers} ca ON ca.optionid = co.id AND ca.choiceid = ch.id + INNER JOIN {choice_options} co ON co.choiceid = ch.id + INNER JOIN {choice_answers} ca ON ca.optionid = co.id AND ca.choiceid = ch.id WHERE ca.userid = :userid"; $params = [ @@ -86,7 +95,9 @@ class provider implements core_provider, plugin_provider { } /** - * @inheritdoc + * Export personal data for the given approved_contextlist. User and context information is contained within the contextlist. + * + * @param approved_contextlist $contextlist a list of contexts approved for export. */ public static function export_user_data(approved_contextlist $contextlist) { global $DB; @@ -95,75 +106,65 @@ class provider implements core_provider, plugin_provider { return; } - $userid = $contextlist->get_user()->id; + $user = $contextlist->get_user(); list($contextsql, $contextparams) = $DB->get_in_or_equal($contextlist->get_contextids(), SQL_PARAMS_NAMED); - $sql = "SELECT ca.id, - cm.id AS cmid, - ch.id as choiceid, - ch.name as choicename, - ch.intro, - ch.introformat, - co.text as answer, + $sql = "SELECT cm.id AS cmid, + co.text as answer, ca.timemodified FROM {context} c INNER JOIN {course_modules} cm ON cm.id = c.instanceid INNER JOIN {choice} ch ON ch.id = cm.instance - LEFT JOIN {choice_options} co ON co.choiceid = ch.id - LEFT JOIN {choice_answers} ca ON ca.optionid = co.id AND ca.choiceid = ch.id + INNER JOIN {choice_options} co ON co.choiceid = ch.id + INNER JOIN {choice_answers} ca ON ca.optionid = co.id AND ca.choiceid = ch.id WHERE c.id {$contextsql} AND ca.userid = :userid"; - $params = [ - 'userid' => $userid - ]; - $params += $contextparams; + $params = ['userid' => $user->id] + $contextparams; + // Create an array of the user's choice instances, supporting multiple answers per choice instance. + $choiceinstances = []; $choiceanswers = $DB->get_recordset_sql($sql, $params); - // Group choice answers per activity. (We might fetch multiple choice answers that belong to a single choice activity). - $answergroups = []; foreach ($choiceanswers as $choiceanswer) { - $cmid = $choiceanswer->cmid; - if (empty($answergroups[$cmid])) { - $context = context_module::instance($cmid); - $data = (object)[ - 'id' => $choiceanswer->id, - 'choiceid' => $choiceanswer->choiceid, - 'choicename' => $choiceanswer->choicename, - 'answer' => $choiceanswer->answer, - 'timemodified' => $choiceanswer->timemodified, + if (empty($choiceinstances[$choiceanswer->cmid])) { + $data = (object) [ + 'answer' => [], + 'timemodified' => \core_privacy\local\request\transform::datetime($choiceanswer->timemodified), ]; - - $data->intro = writer::with_context($context) - ->rewrite_pluginfile_urls([], 'mod_choice', 'intro', $choiceanswer->choiceid, $choiceanswer->intro); - $data->answer = [$choiceanswer->answer]; - $answergroups[$choiceanswer->cmid] = $data; - } else { - $answergroups[$choiceanswer->cmid]->answer[] = $choiceanswer->answer; + $choiceinstances[$choiceanswer->cmid] = $data; } + + // Instance exists, just add the additional answer. + $choiceinstances[$choiceanswer->cmid]->answer[] = $choiceanswer->answer; } $choiceanswers->close(); - // Export the data. - foreach ($answergroups as $cmid => $answergroup) { - $context = context_module::instance($cmid); - writer::with_context($context) - // Export the choice answer. - ->export_data([], $answergroup) + // Now export the data. + foreach ($choiceinstances as $cmid => $choicedata) { + $context = \context_module::instance($cmid); - // Export the associated files. - ->export_area_files([], 'mod_choice', 'intro', $answergroup->choiceid); + // Fetch the generic module data for the choice. + $contextdata = helper::get_context_data($context, $user); + + // Merge with choice data and write it. + $contextdata = (object) array_merge((array) $contextdata, (array) $choicedata); + writer::with_context($context) + ->export_data([], $contextdata); + + // Write generic module intro files. + helper::export_context_files($context, $user); } } /** - * @inheritdoc + * Delete all data for all users in the specified context. + * + * @param \context $context the context to delete in. */ - public static function delete_for_context(deletion_criteria $criteria) { + public static function delete_data_for_all_users_in_context(\context $context) { global $DB; - $context = $criteria->get_context(); if (empty($context)) { return; } @@ -172,9 +173,11 @@ class provider implements core_provider, plugin_provider { } /** - * @inheritdoc + * Delete all user data for the specified user, in the specified contexts. + * + * @param approved_contextlist $contextlist a list of contexts approved for deletion. */ - public static function delete_user_data(approved_contextlist $contextlist) { + public static function delete_data_for_user(approved_contextlist $contextlist) { global $DB; if (empty($contextlist->count())) { diff --git a/mod/choice/tests/privacy_provider_test.php b/mod/choice/tests/privacy_provider_test.php index 112a80d3299..44a7e62ab1a 100644 --- a/mod/choice/tests/privacy_provider_test.php +++ b/mod/choice/tests/privacy_provider_test.php @@ -22,8 +22,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -use core_privacy\metadata\item_collection; -use core_privacy\request\deletion_criteria; +use core_privacy\local\metadata\collection; +use core_privacy\local\request\deletion_criteria; use mod_choice\privacy\provider; defined('MOODLE_INTERNAL') || die(); @@ -35,7 +35,7 @@ defined('MOODLE_INTERNAL') || die(); * @copyright 2018 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class mod_choice_privacy_provider_testcase extends advanced_testcase { +class mod_choice_privacy_provider_testcase extends \core_privacy\tests\provider_testcase { /** @var stdClass The student object. */ protected $student; @@ -46,7 +46,7 @@ class mod_choice_privacy_provider_testcase extends advanced_testcase { protected $course; /** - * @inheritdoc + * {@inheritdoc} */ protected function setUp() { $this->resetAfterTest(); @@ -87,9 +87,9 @@ class mod_choice_privacy_provider_testcase extends advanced_testcase { * Test for provider::get_metadata(). */ public function test_get_metadata() { - $collection = new item_collection('mod_choice'); + $collection = new collection('mod_choice'); $newcollection = provider::get_metadata($collection); - $itemcollection = $newcollection->get_item_collection(); + $itemcollection = $newcollection->get_collection(); $this->assertCount(1, $itemcollection); $table = reset($itemcollection); @@ -118,9 +118,22 @@ class mod_choice_privacy_provider_testcase extends advanced_testcase { } /** - * Test for provider::delete_for_context(). + * Test for provider::export_user_data(). */ - public function test_delete_for_context() { + public function test_export_for_context() { + $cm = get_coursemodule_from_instance('choice', $this->choice->id); + $cmcontext = context_module::instance($cm->id); + + // Export all of the data for the context. + $this->export_context_data_for_user($this->student->id, $cmcontext, 'mod_choice'); + $writer = \core_privacy\local\request\writer::with_context($cmcontext); + $this->assertTrue($writer->has_any_data()); + } + + /** + * Test for provider::delete_data_for_all_users_in_context(). + */ + public function test_delete_data_for_all_users_in_context() { global $DB; $choice = $this->choice; @@ -143,8 +156,7 @@ class mod_choice_privacy_provider_testcase extends advanced_testcase { // Delete data based on context. $cmcontext = context_module::instance($cm->id); - $criteria = new deletion_criteria($cmcontext); - provider::delete_for_context($criteria); + provider::delete_data_for_all_users_in_context($cmcontext); // After deletion, the choice answers for that choice activity should have been deleted. $count = $DB->count_records('choice_answers', ['choiceid' => $choice->id]); @@ -152,9 +164,9 @@ class mod_choice_privacy_provider_testcase extends advanced_testcase { } /** - * Test for provider::delete_user_data(). + * Test for provider::delete_data_for_user(). */ - public function test_delete_user_data() { + public function test_delete_data_for_user_() { global $DB; $choice = $this->choice; @@ -195,8 +207,9 @@ class mod_choice_privacy_provider_testcase extends advanced_testcase { $context1 = context_module::instance($cm1->id); $context2 = context_module::instance($cm2->id); - $contextlist = new \core_privacy\request\approved_contextlist($this->student, [$context1->id, $context2->id]); - provider::delete_user_data($contextlist); + $contextlist = new \core_privacy\local\request\approved_contextlist($this->student, 'choice', + [$context1->id, $context2->id]); + provider::delete_data_for_user($contextlist); // After deletion, the choice answers for the first student should have been deleted. $count = $DB->count_records('choice_answers', ['choiceid' => $choice->id, 'userid' => $this->student->id]);