diff --git a/admin/tool/dataprivacy/classes/api.php b/admin/tool/dataprivacy/classes/api.php index 0fb74f2ad48..695fef720b8 100644 --- a/admin/tool/dataprivacy/classes/api.php +++ b/admin/tool/dataprivacy/classes/api.php @@ -39,7 +39,6 @@ use required_capability_exception; use stdClass; use tool_dataprivacy\external\data_request_exporter; use tool_dataprivacy\local\helper; -use tool_dataprivacy\task\initiate_data_request_task; use tool_dataprivacy\task\process_data_request_task; use tool_dataprivacy\data_request; @@ -65,9 +64,6 @@ class api { /** Newly submitted and we haven't yet started finding out where they have data. */ const DATAREQUEST_STATUS_PENDING = 0; - /** Newly submitted and we have started to find the location of data. */ - const DATAREQUEST_STATUS_PREPROCESSING = 1; - /** Metadata ready and awaiting review and approval by the Data Protection officer. */ const DATAREQUEST_STATUS_AWAITING_APPROVAL = 2; @@ -229,14 +225,22 @@ class api { * @param int $type The request type. * @param string $comments Request comments. * @param int $creationmethod The creation method of the data request. + * @param bool $notify Notify DPOs of this pending request. * @return data_request * @throws invalid_persistent_exception * @throws coding_exception */ public static function create_data_request($foruser, $type, $comments = '', - $creationmethod = data_request::DATAREQUEST_CREATION_MANUAL) { + $creationmethod = data_request::DATAREQUEST_CREATION_MANUAL, + $notify = null + ) { global $USER, $ADMIN; + if (null === $notify && data_request::DATAREQUEST_CREATION_AUTO == $creationmethod) { + // If the request was automatically created, then do not notify unless explicitly set. + $notify = false; + } + $datarequest = new data_request(); // The user the request is being made for. $datarequest->set('userid', $foruser); @@ -249,7 +253,7 @@ class api { // The user making the request. $datarequest->set('requestedby', $requestinguser); // Set status. - $datarequest->set('status', self::DATAREQUEST_STATUS_PENDING); + $datarequest->set('status', self::DATAREQUEST_STATUS_AWAITING_APPROVAL); // Set request type. $datarequest->set('type', $type); // Set request comments. @@ -260,10 +264,15 @@ class api { // Store subject access request. $datarequest->create(); - // Fire an ad hoc task to initiate the data request process. - $task = new initiate_data_request_task(); - $task->set_custom_data(['requestid' => $datarequest->get('id')]); - manager::queue_adhoc_task($task, true); + if ($notify) { + // Get the list of the site Data Protection Officers. + $dpos = api::get_site_dpos(); + + // Email the data request to the Data Protection Officer(s)/Admin(s). + foreach ($dpos as $dpo) { + api::notify_dpo($dpo, $datarequest); + } + } return $datarequest; } @@ -603,11 +612,6 @@ class api { // Update the status and the DPO. $result = self::update_request_status($requestid, self::DATAREQUEST_STATUS_APPROVED, $USER->id); - // Approve all the contexts attached to the request. - // Currently, approving the request implicitly approves all associated contexts, but this may change in future, allowing - // users to selectively approve certain contexts only. - self::update_request_contexts_with_status($requestid, contextlist_context::STATUS_APPROVED); - // Fire an ad hoc task to initiate the data request process. $task = new process_data_request_task(); $task->set_custom_data(['requestid' => $requestid]); @@ -1066,147 +1070,39 @@ class api { } /** - * Adds the contexts from the contextlist_collection to the request with the status provided. + * Finds all contextlists having at least one approved context, and returns them as in a contextlist_collection. * - * @param contextlist_collection $clcollection a collection of contextlists for all components. - * @param int $requestid the id of the request. - * @param int $status the status to set the contexts to. + * @param contextlist_collection $collection The collection of unapproved contextlist objects. + * @param \stdClass $foruser The target user + * @param int $type The purpose of the collection + * @return contextlist_collection The collection of approved_contextlist objects. */ - public static function add_request_contexts_with_status(contextlist_collection $clcollection, int $requestid, int $status) { - $request = new data_request($requestid); - $user = \core_user::get_user($request->get('userid')); - foreach ($clcollection as $contextlist) { - // Convert the \core_privacy\local\request\contextlist into a contextlist persistent and store it. - $clp = \tool_dataprivacy\contextlist::from_contextlist($contextlist); - $clp->create(); - $contextlistid = $clp->get('id'); + public static function get_approved_contextlist_collection_for_collection(contextlist_collection $collection, + \stdClass $foruser, int $type) : contextlist_collection { - // Store the associated contexts in the contextlist. - foreach ($contextlist->get_contextids() as $contextid) { - if ($request->get('type') == static::DATAREQUEST_TYPE_DELETE) { - $context = \context::instance_by_id($contextid); - $purpose = static::get_effective_context_purpose($context); + // Create the approved contextlist collection object. + $approvedcollection = new contextlist_collection($collection->get_userid()); + foreach ($collection as $contextlist) { + $contextids = []; + foreach ($contextlist as $context) { + if (self::DATAREQUEST_TYPE_DELETE == $type) { // Data can only be deleted from it if the context is either expired, or unprotected. - if (!expired_contexts_manager::is_context_expired_or_unprotected_for_user($context, $user)) { + $purpose = static::get_effective_context_purpose($context); + if (!expired_contexts_manager::is_context_expired_or_unprotected_for_user($context, $foruser)) { continue; } } - $context = new contextlist_context(); - $context->set('contextid', $contextid) - ->set('contextlistid', $contextlistid) - ->set('status', $status) - ->create(); + $contextids[] = $context->id; } - // Create the relation to the request. - $requestcontextlist = request_contextlist::create_relation($requestid, $contextlistid); - $requestcontextlist->create(); - } - } - - /** - * Sets the status of all contexts associated with the request. - * - * @param int $requestid the requestid to which the contexts belong. - * @param int $status the status to set to. - * @throws \dml_exception if the requestid is invalid. - * @throws \moodle_exception if the status is invalid. - */ - public static function update_request_contexts_with_status(int $requestid, int $status) { - // Validate contextlist_context status using the persistent's attribute validation. - $contextlistcontext = new contextlist_context(); - $contextlistcontext->set('status', $status); - if (array_key_exists('status', $contextlistcontext->get_errors())) { - throw new moodle_exception("Invalid contextlist_context status: $status"); - } - - // Validate requestid using the persistent's record validation. - // A dml_exception is thrown if the record is missing. - $datarequest = new data_request($requestid); - - // Bulk update the status of the request contexts. - global $DB; - - $select = "SELECT ctx.id as id - FROM {" . request_contextlist::TABLE . "} rcl - JOIN {" . contextlist::TABLE . "} cl ON rcl.contextlistid = cl.id - JOIN {" . contextlist_context::TABLE . "} ctx ON cl.id = ctx.contextlistid - WHERE rcl.requestid = ?"; - - // Fetch records IDs to be updated and update by chunks, if applicable (limit of 1000 records per update). - $limit = 1000; - $idstoupdate = $DB->get_fieldset_sql($select, [$requestid]); - $count = count($idstoupdate); - $idchunks = $idstoupdate; - if ($count > $limit) { - $idchunks = array_chunk($idstoupdate, $limit); - } - $transaction = $DB->start_delegated_transaction(); - $initialparams = [$status]; - foreach ($idchunks as $chunk) { - list($insql, $inparams) = $DB->get_in_or_equal($chunk); - $update = "UPDATE {" . contextlist_context::TABLE . "} - SET status = ? - WHERE id $insql"; - $params = array_merge($initialparams, $inparams); - $DB->execute($update, $params); - } - $transaction->allow_commit(); - } - - /** - * Finds all request contextlists having at least on approved context, and returns them as in a contextlist_collection. - * - * @param data_request $request the data request with which the contextlists are associated. - * @return contextlist_collection the collection of approved_contextlist objects. - */ - public static function get_approved_contextlist_collection_for_request(data_request $request) : contextlist_collection { - $foruser = core_user::get_user($request->get('userid')); - - // Fetch all approved contextlists and create the core_privacy\local\request\contextlist objects here. - global $DB; - $sql = "SELECT cl.component, ctx.contextid - FROM {" . request_contextlist::TABLE . "} rcl - JOIN {" . contextlist::TABLE . "} cl ON rcl.contextlistid = cl.id - JOIN {" . contextlist_context::TABLE . "} ctx ON cl.id = ctx.contextlistid - WHERE rcl.requestid = ? - AND ctx.status = ? - ORDER BY cl.component, ctx.contextid"; - - // Create the approved contextlist collection object. - $lastcomponent = null; - $approvedcollection = new contextlist_collection($foruser->id); - - $rs = $DB->get_recordset_sql($sql, [$request->get('id'), contextlist_context::STATUS_APPROVED]); - foreach ($rs as $record) { - // If we encounter a new component, and we've built up contexts for the last, then add the approved_contextlist for the - // last (the one we've just finished with) and reset the context array for the next one. - if ($lastcomponent != $record->component) { - if (!empty($contexts)) { - $approvedcollection->add_contextlist(new approved_contextlist($foruser, $lastcomponent, $contexts)); - } - $contexts = []; + // The data for the last component contextlist won't have been written yet, so write it now. + if (!empty($contextids)) { + $approvedcollection->add_contextlist( + new approved_contextlist($foruser, $contextlist->get_component(), $contextids) + ); } - - if ($request->get('type') == static::DATAREQUEST_TYPE_DELETE) { - $context = \context::instance_by_id($record->contextid); - $purpose = static::get_effective_context_purpose($context); - // Data can only be deleted from it if the context is either expired, or unprotected. - if (!expired_contexts_manager::is_context_expired_or_unprotected_for_user($context, $foruser)) { - continue; - } - } - - $contexts[] = $record->contextid; - $lastcomponent = $record->component; - } - $rs->close(); - - // The data for the last component contextlist won't have been written yet, so write it now. - if (!empty($contexts)) { - $approvedcollection->add_contextlist(new approved_contextlist($foruser, $lastcomponent, $contexts)); } return $approvedcollection; diff --git a/admin/tool/dataprivacy/classes/contextlist.php b/admin/tool/dataprivacy/classes/contextlist.php deleted file mode 100644 index ef25c11b9a3..00000000000 --- a/admin/tool/dataprivacy/classes/contextlist.php +++ /dev/null @@ -1,64 +0,0 @@ -<?php -// This file is part of Moodle - http://moodle.org/ -// -// Moodle is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Moodle is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Moodle. If not, see <http://www.gnu.org/licenses/>. - -/** - * Contains the contextlist persistent. - * - * @package tool_dataprivacy - * @copyright 2018 Jake Dallimore <jrhdallimore@gmail.com> - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -namespace tool_dataprivacy; - -defined('MOODLE_INTERNAL') || die(); - -use core\persistent; - -/** - * The contextlist persistent. - * - * @copyright 2018 Jake Dallimore <jrhdallimore@gmail.com> - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class contextlist extends persistent { - - /** The table name this persistent object maps to. */ - const TABLE = 'tool_dataprivacy_contextlist'; - - /** - * Return the definition of the properties of this model. - * - * @return array - */ - protected static function define_properties() { - return [ - 'component' => [ - 'type' => PARAM_TEXT - ] - ]; - } - - /** - * Create a new contextlist persistent from an instance of \core_privacy\local\request\contextlist. - * - * @param \core_privacy\local\request\contextlist $contextlist the core privacy contextlist. - * @return contextlist a contextlist persistent. - */ - public static function from_contextlist(\core_privacy\local\request\contextlist $contextlist) : contextlist { - $contextlistpersistent = new contextlist(); - return $contextlistpersistent->set('component', $contextlist->get_component()); - } -} diff --git a/admin/tool/dataprivacy/classes/contextlist_context.php b/admin/tool/dataprivacy/classes/contextlist_context.php deleted file mode 100644 index 0f4ca9fcff7..00000000000 --- a/admin/tool/dataprivacy/classes/contextlist_context.php +++ /dev/null @@ -1,74 +0,0 @@ -<?php -// This file is part of Moodle - http://moodle.org/ -// -// Moodle is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Moodle is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Moodle. If not, see <http://www.gnu.org/licenses/>. - -/** - * Contains the contextlist_context persistent. - * - * @package tool_dataprivacy - * @copyright 2018 Jake Dallimore <jrhdallimore@gmail.com> - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -namespace tool_dataprivacy; - -defined('MOODLE_INTERNAL') || die(); - -use core\persistent; - -/** - * The contextlist_context persistent. - * - * @copyright 2018 Jake Dallimore <jrhdallimore@gmail.com> - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class contextlist_context extends persistent { - - /** The table name this persistent object maps to. */ - const TABLE = 'tool_dataprivacy_ctxlst_ctx'; - - /** This context is pending approval. */ - const STATUS_PENDING = 0; - - /** This context has been approved. */ - const STATUS_APPROVED = 1; - - /** This context has been rejected. */ - const STATUS_REJECTED = 2; - - /** - * Return the definition of the properties of this model. - * - * @return array - */ - protected static function define_properties() { - return [ - 'contextid' => [ - 'type' => PARAM_INT - ], - 'contextlistid' => [ - 'type' => PARAM_INT - ], - 'status' => [ - 'choices' => [ - self::STATUS_PENDING, - self::STATUS_APPROVED, - self::STATUS_REJECTED, - ], - 'default' => self::STATUS_PENDING, - 'type' => PARAM_INT - ] - ]; - } -} diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index a95d27a9a00..39015632a5f 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -83,10 +83,9 @@ class data_request extends persistent { 'type' => PARAM_INT ], 'status' => [ - 'default' => api::DATAREQUEST_STATUS_PENDING, + 'default' => api::DATAREQUEST_STATUS_AWAITING_APPROVAL, 'choices' => [ api::DATAREQUEST_STATUS_PENDING, - api::DATAREQUEST_STATUS_PREPROCESSING, api::DATAREQUEST_STATUS_AWAITING_APPROVAL, api::DATAREQUEST_STATUS_APPROVED, api::DATAREQUEST_STATUS_PROCESSING, @@ -278,7 +277,14 @@ class data_request extends persistent { $currentdata = $this->to_record(); unset($currentdata->id); - $clone = api::create_data_request($this->get('userid'), $this->get('type')); + // Clone the original request, but do not notify. + $clone = api::create_data_request( + $this->get('userid'), + $this->get('type'), + $this->get('comments'), + $this->get('creationmethod'), + false + ); $clone->set('comments', $this->get('comments')); $clone->set('dpo', $this->get('dpo')); $clone->set('requestedby', $this->get('requestedby')); diff --git a/admin/tool/dataprivacy/classes/external/data_request_exporter.php b/admin/tool/dataprivacy/classes/external/data_request_exporter.php index b7d483ca64d..2826fc81d7c 100644 --- a/admin/tool/dataprivacy/classes/external/data_request_exporter.php +++ b/admin/tool/dataprivacy/classes/external/data_request_exporter.php @@ -164,9 +164,6 @@ class data_request_exporter extends persistent_exporter { // Request can be manually completed for general enquiry requests. $values['canmarkcomplete'] = $requesttype == api::DATAREQUEST_TYPE_OTHERS; break; - case api::DATAREQUEST_STATUS_PREPROCESSING: - $values['statuslabelclass'] = 'label-default'; - break; case api::DATAREQUEST_STATUS_AWAITING_APPROVAL: $values['statuslabelclass'] = 'label-info'; // DPO can review the request once it's ready. diff --git a/admin/tool/dataprivacy/classes/local/helper.php b/admin/tool/dataprivacy/classes/local/helper.php index 25f5bf819cf..14d68f72a1b 100644 --- a/admin/tool/dataprivacy/classes/local/helper.php +++ b/admin/tool/dataprivacy/classes/local/helper.php @@ -136,7 +136,6 @@ class helper { public static function get_request_statuses() { return [ api::DATAREQUEST_STATUS_PENDING => get_string('statuspending', 'tool_dataprivacy'), - api::DATAREQUEST_STATUS_PREPROCESSING => get_string('statuspreprocessing', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_AWAITING_APPROVAL => get_string('statusawaitingapproval', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_APPROVED => get_string('statusapproved', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_PROCESSING => get_string('statusprocessing', 'tool_dataprivacy'), diff --git a/admin/tool/dataprivacy/classes/request_contextlist.php b/admin/tool/dataprivacy/classes/request_contextlist.php deleted file mode 100644 index 5764b7e11f8..00000000000 --- a/admin/tool/dataprivacy/classes/request_contextlist.php +++ /dev/null @@ -1,70 +0,0 @@ -<?php -// This file is part of Moodle - http://moodle.org/ -// -// Moodle is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Moodle is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Moodle. If not, see <http://www.gnu.org/licenses/>. - -/** - * Contains the request_contextlist persistent. - * - * @package tool_dataprivacy - * @copyright 2018 Jake Dallimore <jrhdallimore@gmail.com> - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -namespace tool_dataprivacy; - -defined('MOODLE_INTERNAL') || die(); - -use core\persistent; - -/** - * The request_contextlist persistent. - * - * @copyright 2018 Jake Dallimore <jrhdallimore@gmail.com> - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class request_contextlist extends persistent { - - /** The table name this persistent object maps to. */ - const TABLE = 'tool_dataprivacy_rqst_ctxlst'; - - /** - * Return the definition of the properties of this model. - * - * @return array - */ - protected static function define_properties() { - return [ - 'requestid' => [ - 'type' => PARAM_INT - ], - 'contextlistid' => [ - 'type' => PARAM_INT - ] - ]; - } - - /** - * Creates a new relation, but does not persist it. - * - * @param int $requestid - * @param int $contextlistid - * @return $this - * @throws \coding_exception - */ - public static function create_relation($requestid, $contextlistid) { - $requestcontextlist = new request_contextlist(); - return $requestcontextlist->set('requestid', $requestid) - ->set('contextlistid', $contextlistid); - } -} diff --git a/admin/tool/dataprivacy/classes/task/initiate_data_request_task.php b/admin/tool/dataprivacy/classes/task/initiate_data_request_task.php deleted file mode 100644 index 1e02a2f8988..00000000000 --- a/admin/tool/dataprivacy/classes/task/initiate_data_request_task.php +++ /dev/null @@ -1,107 +0,0 @@ -<?php -// This file is part of Moodle - http://moodle.org/ -// -// Moodle is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Moodle is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Moodle. If not, see <http://www.gnu.org/licenses/>. - -/** - * Adhoc task that processes a data request and prepares the user's relevant contexts for review. - * - * @package tool_dataprivacy - * @copyright 2018 Jun Pataleta - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_dataprivacy\task; - -use coding_exception; -use core\task\adhoc_task; -use moodle_exception; -use tool_dataprivacy\api; -use tool_dataprivacy\contextlist_context; -use tool_dataprivacy\data_request; - -defined('MOODLE_INTERNAL') || die(); - -/** - * Class that processes a data request and prepares the user's relevant contexts for review. - * - * Custom data accepted: - * - requestid -> The ID of the data request to be processed. - * - * @package tool_dataprivacy - * @copyright 2018 Jun Pataleta - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class initiate_data_request_task extends adhoc_task { - - /** - * Run the task to initiate the data request process. - * - * @throws coding_exception - * @throws moodle_exception - */ - public function execute() { - global $CFG; - - require_once($CFG->dirroot . "/{$CFG->admin}/tool/dataprivacy/lib.php"); - - if (!isset($this->get_custom_data()->requestid)) { - throw new coding_exception('The custom data \'requestid\' is required.'); - } - $requestid = $this->get_custom_data()->requestid; - - $datarequest = new data_request($requestid); - - // Check if this request still needs to be processed. e.g. The user might have cancelled it before this task has run. - $status = $datarequest->get('status'); - if (!api::is_active($status)) { - mtrace('Request ' . $requestid . ' with status ' . $status . ' doesn\'t need to be processed. Skipping...'); - return; - } - - // Update the status of this request as pre-processing. - mtrace('Generating the contexts containing personal data for the user...'); - api::update_request_status($requestid, api::DATAREQUEST_STATUS_PREPROCESSING); - - // Add the list of relevant contexts to the request, and mark all as pending approval. - $privacymanager = new \core_privacy\manager(); - $privacymanager->set_observer(new \tool_dataprivacy\manager_observer()); - - $contextlistcollection = $privacymanager->get_contexts_for_userid($datarequest->get('userid')); - api::add_request_contexts_with_status($contextlistcollection, $requestid, contextlist_context::STATUS_PENDING); - - // When the preparation of the contexts finishes, update the request status to awaiting approval. - api::update_request_status($requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - mtrace('Context generation complete...'); - - // Get the list of the site Data Protection Officers. - $dpos = api::get_site_dpos(); - - // We should prevent DPO(s)/Admin(s) being flooded with notifications for each request when bulk delete - // data requests are being created. - // NOTE: This should be improved, we should not totally disable the notifications for automatically - // created requests. Possibly, we should create one notification for these such cases. - if ($datarequest->get('creationmethod') != data_request::DATAREQUEST_CREATION_AUTO) { - // Email the data request to the Data Protection Officer(s)/Admin(s). - foreach ($dpos as $dpo) { - $dponame = fullname($dpo); - if (api::notify_dpo($dpo, $datarequest)) { - mtrace('Message sent to DPO: ' . $dponame); - } else { - mtrace('A problem was encountered while sending the message to the DPO: ' . $dponame); - } - } - } - } -} diff --git a/admin/tool/dataprivacy/classes/task/process_data_request_task.php b/admin/tool/dataprivacy/classes/task/process_data_request_task.php index 802f61af70d..08342632d45 100644 --- a/admin/tool/dataprivacy/classes/task/process_data_request_task.php +++ b/admin/tool/dataprivacy/classes/task/process_data_request_task.php @@ -74,12 +74,24 @@ class process_data_request_task extends adhoc_task { return; } + // Grab the manager. + // We set an observer against it to handle failures. + $manager = new \core_privacy\manager(); + $manager->set_observer(new \tool_dataprivacy\manager_observer()); + // Get the user details now. We might not be able to retrieve it later if it's a deletion processing. $foruser = core_user::get_user($request->userid); // Update the status of this request as pre-processing. - mtrace('Processing request...'); + mtrace('Pre-processing request...'); api::update_request_status($requestid, api::DATAREQUEST_STATUS_PROCESSING); + $contextlistcollection = $manager->get_contexts_for_userid($requestpersistent->get('userid')); + + mtrace('Fetching approved contextlists from collection'); + $approvedclcollection = api::get_approved_contextlist_collection_for_collection( + $contextlistcollection, $foruser, $request->type); + + mtrace('Processing request...'); $completestatus = api::DATAREQUEST_STATUS_COMPLETE; if ($request->type == api::DATAREQUEST_TYPE_EXPORT) { @@ -91,13 +103,7 @@ class process_data_request_task extends adhoc_task { return; } - // Get the collection of approved_contextlist objects needed for core_privacy data export. - $approvedclcollection = api::get_approved_contextlist_collection_for_request($requestpersistent); - // Export the data. - $manager = new \core_privacy\manager(); - $manager->set_observer(new \tool_dataprivacy\manager_observer()); - $exportedcontent = $manager->export_user_data($approvedclcollection); $fs = get_file_storage(); @@ -115,9 +121,6 @@ class process_data_request_task extends adhoc_task { $thing = $fs->create_file_from_pathname($filerecord, $exportedcontent); $completestatus = api::DATAREQUEST_STATUS_DOWNLOAD_READY; } else if ($request->type == api::DATAREQUEST_TYPE_DELETE) { - // Get the collection of approved_contextlist objects needed for core_privacy data deletion. - $approvedclcollection = api::get_approved_contextlist_collection_for_request($requestpersistent); - // Delete the data. $manager = new \core_privacy\manager(); $manager->set_observer(new \tool_dataprivacy\manager_observer()); diff --git a/admin/tool/dataprivacy/db/install.xml b/admin/tool/dataprivacy/db/install.xml index 2d4a01a9591..fc5e96b3390 100644 --- a/admin/tool/dataprivacy/db/install.xml +++ b/admin/tool/dataprivacy/db/install.xml @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="UTF-8" ?> -<XMLDB PATH="admin/tool/dataprivacy/db" VERSION="20180904" COMMENT="XMLDB file for Moodle tool/dataprivacy" +<XMLDB PATH="admin/tool/dataprivacy/db" VERSION="20181107" COMMENT="XMLDB file for Moodle tool/dataprivacy" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../../lib/xmldb/xmldb.xsd" > @@ -112,44 +112,6 @@ <KEY NAME="contextid" TYPE="foreign-unique" FIELDS="contextid" REFTABLE="context" REFFIELDS="id"/> </KEYS> </TABLE> - <TABLE NAME="tool_dataprivacy_contextlist" COMMENT="List of contexts for a component"> - <FIELDS> - <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/> - <FIELD NAME="component" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false" COMMENT="Frankenstyle component name"/> - <FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/> - <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/> - </FIELDS> - <KEYS> - <KEY NAME="primary" TYPE="primary" FIELDS="id"/> - </KEYS> - </TABLE> - <TABLE NAME="tool_dataprivacy_ctxlst_ctx" COMMENT="A contextlist context item"> - <FIELDS> - <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/> - <FIELD NAME="contextid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/> - <FIELD NAME="contextlistid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/> - <FIELD NAME="status" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Approval status of the context item"/> - <FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/> - <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/> - </FIELDS> - <KEYS> - <KEY NAME="primary" TYPE="primary" FIELDS="id"/> - <KEY NAME="contextlistid" TYPE="foreign" FIELDS="contextlistid" REFTABLE="tool_dataprivacy_contextlist" REFFIELDS="id" COMMENT="Reference to the contextlist containing this context item"/> - </KEYS> - </TABLE> - <TABLE NAME="tool_dataprivacy_rqst_ctxlst" COMMENT="Association table joining requests and contextlists"> - <FIELDS> - <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/> - <FIELD NAME="requestid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/> - <FIELD NAME="contextlistid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/> - </FIELDS> - <KEYS> - <KEY NAME="primary" TYPE="primary" FIELDS="id"/> - <KEY NAME="requestid" TYPE="foreign" FIELDS="requestid" REFTABLE="tool_dataprivacy_request" REFFIELDS="id" COMMENT="Reference to the request"/> - <KEY NAME="contextlistid" TYPE="foreign" FIELDS="contextlistid" REFTABLE="tool_dataprivacy_contextlist" REFFIELDS="id" COMMENT="Reference to the contextlist"/> - <KEY NAME="request_contextlist" TYPE="unique" FIELDS="requestid, contextlistid" COMMENT="Uniqueness constraint on request and contextlist"/> - </KEYS> - </TABLE> <TABLE NAME="tool_dataprivacy_purposerole" COMMENT="Data purpose overrides for a specific role"> <FIELDS> <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/> diff --git a/admin/tool/dataprivacy/db/upgrade.php b/admin/tool/dataprivacy/db/upgrade.php index 306337d346c..2f52d4857c6 100644 --- a/admin/tool/dataprivacy/db/upgrade.php +++ b/admin/tool/dataprivacy/db/upgrade.php @@ -262,5 +262,42 @@ function xmldb_tool_dataprivacy_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2018100406, 'tool', 'dataprivacy'); } + + if ($oldversion < 2018110700) { + // Define table tool_dataprivacy_ctxlst_ctx to be dropped. + $table = new xmldb_table('tool_dataprivacy_ctxlst_ctx'); + + // Conditionally launch drop table for tool_dataprivacy_ctxlst_ctx. + if ($dbman->table_exists($table)) { + $dbman->drop_table($table); + } + + // Define table tool_dataprivacy_rqst_ctxlst to be dropped. + $table = new xmldb_table('tool_dataprivacy_rqst_ctxlst'); + + // Conditionally launch drop table for tool_dataprivacy_rqst_ctxlst. + if ($dbman->table_exists($table)) { + $dbman->drop_table($table); + } + + // Define table tool_dataprivacy_contextlist to be dropped. + $table = new xmldb_table('tool_dataprivacy_contextlist'); + + // Conditionally launch drop table for tool_dataprivacy_contextlist. + if ($dbman->table_exists($table)) { + $dbman->drop_table($table); + } + + // Update all requests which were in states Pending, or Pre-Processing, to Awaiting approval. + $DB->set_field('tool_dataprivacy_request', 'status', 2, ['status' => 0]); + $DB->set_field('tool_dataprivacy_request', 'status', 2, ['status' => 1]); + + // Remove the old initiate_data_request_task adhoc entries. + $DB->delete_records('task_adhoc', ['classname' => '\tool_dataprivacy\task\initiate_data_request_task']); + + // Dataprivacy savepoint reached. + upgrade_plugin_savepoint(true, 2018110700, 'tool', 'dataprivacy'); + } + return true; } diff --git a/admin/tool/dataprivacy/lang/en/deprecated.txt b/admin/tool/dataprivacy/lang/en/deprecated.txt new file mode 100644 index 00000000000..c400ae86878 --- /dev/null +++ b/admin/tool/dataprivacy/lang/en/deprecated.txt @@ -0,0 +1 @@ +statuspreprocessing,tool_dataprivacy diff --git a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php index 35595cf00ab..c9f4ce1309e 100644 --- a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php +++ b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php @@ -301,7 +301,6 @@ $string['statusready'] = 'Download ready'; $string['statusdeleted'] = 'Deleted'; $string['statusdetail'] = 'Status:'; $string['statusexpired'] = 'Expired'; -$string['statuspreprocessing'] = 'Pre-processing'; $string['statusprocessing'] = 'Processing'; $string['statuspending'] = 'Pending'; $string['statusrejected'] = 'Rejected'; @@ -332,3 +331,6 @@ $string['role_help'] = 'The role which the override should apply to.'; $string['duplicaterole'] = 'Role already specified'; $string['purposeoverview'] = 'A purpose describes the intended use and retention policy for stored data. The basis for storing and retaining that data is also described in the purpose.'; $string['roleoverrideoverview'] = 'The default retention policy can be overridden for specific user roles, allowing you to specify a longer, or a shorter, retention policy. A user is only expired when all of their roles have expired.'; + +// Deprecated since Moodle 3.6. +$string['statuspreprocessing'] = 'Pre-processing'; diff --git a/admin/tool/dataprivacy/tests/api_test.php b/admin/tool/dataprivacy/tests/api_test.php index a1a5de986d2..8eed7cbf443 100644 --- a/admin/tool/dataprivacy/tests/api_test.php +++ b/admin/tool/dataprivacy/tests/api_test.php @@ -24,7 +24,6 @@ use core\invalid_persistent_exception; use core\task\manager; -use tool_dataprivacy\contextlist_context; use tool_dataprivacy\context_instance; use tool_dataprivacy\api; use tool_dataprivacy\data_registry; @@ -33,7 +32,6 @@ use tool_dataprivacy\data_request; use tool_dataprivacy\purpose; use tool_dataprivacy\category; use tool_dataprivacy\local\helper; -use tool_dataprivacy\task\initiate_data_request_task; use tool_dataprivacy\task\process_data_request_task; defined('MOODLE_INTERNAL') || die(); @@ -288,40 +286,6 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertCount(1, $adhoctasks); } - /** - * Test for api::approve_data_request() with the request not yet waiting for approval. - */ - public function test_approve_data_request_not_yet_ready() { - global $DB; - - $this->resetAfterTest(); - - $generator = new testing_data_generator(); - $s1 = $generator->create_user(); - $u1 = $generator->create_user(); - - $context = context_system::instance(); - - // Manager role. - $managerroleid = $DB->get_field('role', 'id', array('shortname' => 'manager')); - // Give the manager role with the capability to manage data requests. - assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $managerroleid, $context->id, true); - // Assign u1 as a manager. - role_assign($managerroleid, $u1->id, $context->id); - - // Map the manager role to the DPO role. - set_config('dporoles', $managerroleid, 'tool_dataprivacy'); - - // Create the sample data request. - $this->setUser($s1); - $datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT); - $requestid = $datarequest->get('id'); - - $this->setUser($u1); - $this->expectException(moodle_exception::class); - api::approve_data_request($requestid); - } - /** * Test for api::approve_data_request() when called by a user who doesn't have the DPO role. */ @@ -581,12 +545,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertEquals($user->id, $datarequest->get('requestedby')); $this->assertEquals(0, $datarequest->get('dpo')); $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type')); - $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status')); + $this->assertEquals(api::DATAREQUEST_STATUS_AWAITING_APPROVAL, $datarequest->get('status')); $this->assertEquals($comment, $datarequest->get('comments')); - - // Test adhoc task creation. - $adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class); - $this->assertCount(1, $adhoctasks); } /** @@ -609,12 +569,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertEquals($user->id, $datarequest->get('userid')); $this->assertEquals($USER->id, $datarequest->get('requestedby')); $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type')); - $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status')); + $this->assertEquals(api::DATAREQUEST_STATUS_AWAITING_APPROVAL, $datarequest->get('status')); $this->assertEquals($comment, $datarequest->get('comments')); - - // Test adhoc task creation. - $adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class); - $this->assertCount(1, $adhoctasks); } /** @@ -648,12 +604,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertEquals($parent->id, $datarequest->get('requestedby')); $this->assertEquals(0, $datarequest->get('dpo')); $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type')); - $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status')); + $this->assertEquals(api::DATAREQUEST_STATUS_AWAITING_APPROVAL, $datarequest->get('status')); $this->assertEquals($comment, $datarequest->get('comments')); - - // Test adhoc task creation. - $adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class); - $this->assertCount(1, $adhoctasks); } /** @@ -827,8 +779,6 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { */ public function status_provider() { return [ - [api::DATAREQUEST_STATUS_PENDING, true], - [api::DATAREQUEST_STATUS_PREPROCESSING, true], [api::DATAREQUEST_STATUS_AWAITING_APPROVAL, true], [api::DATAREQUEST_STATUS_APPROVED, true], [api::DATAREQUEST_STATUS_PROCESSING, true], @@ -1481,38 +1431,10 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { ]; } - /** - * Test that delete requests filter out protected purpose contexts. - */ - public function test_add_request_contexts_with_status_delete() { - $this->resetAfterTest(); - - $data = $this->setup_test_add_request_contexts_with_status(api::DATAREQUEST_TYPE_DELETE); - $contextids = $data->list->get_contextids(); - - $this->assertCount(1, $contextids); - $this->assertEquals($data->contexts->unprotected, $contextids); - } - - /** - * Test that export requests don't filter out protected purpose contexts. - */ - public function test_add_request_contexts_with_status_export() { - $this->resetAfterTest(); - - $data = $this->setup_test_add_request_contexts_with_status(api::DATAREQUEST_TYPE_EXPORT); - $contextids = $data->list->get_contextids(); - - $this->assertCount(2, $contextids); - $this->assertEquals($data->contexts->used, $contextids, '', 0.0, 10, true); - } - /** * Test that delete requests do not filter out protected purpose contexts if they are already expired. */ - public function test_add_request_contexts_with_status_delete_course_expired_protected() { - global $DB; - + public function test_get_approved_contextlist_collection_for_collection_delete_course_expired_protected() { $this->resetAfterTest(); $purposes = $this->setup_basics('PT1H', 'PT1H', 'PT1H'); @@ -1524,120 +1446,17 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student'); - $collection = new \core_privacy\local\request\contextlist_collection($user->id); + // Create the initial contextlist. $contextlist = new \core_privacy\local\request\contextlist(); + $contextlist->add_from_sql('SELECT id FROM {context} WHERE id = :contextid', ['contextid' => $coursecontext->id]); $contextlist->set_component('tool_dataprivacy'); - $contextlist->add_from_sql('SELECT id FROM {context} WHERE id IN(:ctx1)', ['ctx1' => $coursecontext->id]); - $collection->add_contextlist($contextlist); - $request = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); + $initialcollection = new \core_privacy\local\request\contextlist_collection($user->id); + $initialcollection->add_contextlist($contextlist); $purposes->course->purpose->set('protected', 1)->save(); - api::add_request_contexts_with_status($collection, $request->get('id'), contextlist_context::STATUS_APPROVED); - - $requests = contextlist_context::get_records(); - $this->assertCount(1, $requests); - } - - /** - * Test that delete requests does filter out protected purpose contexts which are not expired. - */ - public function test_add_request_contexts_with_status_delete_course_unexpired_protected() { - global $DB; - - $this->resetAfterTest(); - - $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1Y'); - $purposes->course->purpose->set('protected', 1)->save(); - - $user = $this->getDataGenerator()->create_user(); - $course = $this->getDataGenerator()->create_course(['startdate' => time() - YEARSECS, 'enddate' => time()]); - $coursecontext = \context_course::instance($course->id); - - $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student'); - - $collection = new \core_privacy\local\request\contextlist_collection($user->id); - $contextlist = new \core_privacy\local\request\contextlist(); - $contextlist->set_component('tool_dataprivacy'); - $contextlist->add_from_sql('SELECT id FROM {context} WHERE id IN(:ctx1)', ['ctx1' => $coursecontext->id]); - $collection->add_contextlist($contextlist); - - $request = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); - - $purposes->course->purpose->set('protected', 1)->save(); - api::add_request_contexts_with_status($collection, $request->get('id'), contextlist_context::STATUS_APPROVED); - - $requests = contextlist_context::get_records(); - $this->assertCount(0, $requests); - } - - /** - * Test that delete requests do not filter out unexpired contexts if they are not protected. - */ - public function test_add_request_contexts_with_status_delete_course_unexpired_unprotected() { - global $DB; - - $this->resetAfterTest(); - - $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1Y'); - $purposes->course->purpose->set('protected', 1)->save(); - - $user = $this->getDataGenerator()->create_user(); - $course = $this->getDataGenerator()->create_course(['startdate' => time() - YEARSECS, 'enddate' => time()]); - $coursecontext = \context_course::instance($course->id); - - $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student'); - - $collection = new \core_privacy\local\request\contextlist_collection($user->id); - $contextlist = new \core_privacy\local\request\contextlist(); - $contextlist->set_component('tool_dataprivacy'); - $contextlist->add_from_sql('SELECT id FROM {context} WHERE id IN(:ctx1)', ['ctx1' => $coursecontext->id]); - $collection->add_contextlist($contextlist); - - $request = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); - - $purposes->course->purpose->set('protected', 0)->save(); - api::add_request_contexts_with_status($collection, $request->get('id'), contextlist_context::STATUS_APPROVED); - - $requests = contextlist_context::get_records(); - $this->assertCount(1, $requests); - } - - /** - * Test that delete requests do not filter out protected purpose contexts if they are already expired. - */ - public function test_get_approved_contextlist_collection_for_request_delete_course_expired_protected() { - $this->resetAfterTest(); - - $purposes = $this->setup_basics('PT1H', 'PT1H', 'PT1H'); - $purposes->course->purpose->set('protected', 1)->save(); - - $user = $this->getDataGenerator()->create_user(); - $course = $this->getDataGenerator()->create_course(['startdate' => time() - YEARSECS, 'enddate' => time() - YEARSECS]); - $coursecontext = \context_course::instance($course->id); - - $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student'); - - // Create the request, with its contextlist and context. - $request = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); - $contextlist = new \tool_dataprivacy\contextlist(0, (object) ['component' => 'tool_dataprivacy']); - $contextlist->save(); - - $clcontext = new \tool_dataprivacy\contextlist_context(0, (object) [ - 'contextid' => $coursecontext->id, - 'status' => contextlist_context::STATUS_APPROVED, - 'contextlistid' => $contextlist->get('id'), - ]); - $clcontext->save(); - - $rcl = new \tool_dataprivacy\request_contextlist(0, (object) [ - 'requestid' => $request->get('id'), - 'contextlistid' => $contextlist->get('id'), - ]); - $rcl->save(); - - $purposes->course->purpose->set('protected', 1)->save(); - $collection = api::get_approved_contextlist_collection_for_request($request); + $collection = api::get_approved_contextlist_collection_for_collection( + $initialcollection, $user, api::DATAREQUEST_TYPE_DELETE); $this->assertCount(1, $collection); @@ -1648,7 +1467,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { /** * Test that delete requests does filter out protected purpose contexts which are not expired. */ - public function test_get_approved_contextlist_collection_for_request_delete_course_unexpired_protected() { + public function test_get_approved_contextlist_collection_for_collection_delete_course_unexpired_protected() { $this->resetAfterTest(); $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1Y'); @@ -1660,26 +1479,17 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student'); - // Create the request, with its contextlist and context. - $request = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); - $contextlist = new \tool_dataprivacy\contextlist(0, (object) ['component' => 'tool_dataprivacy']); - $contextlist->save(); + // Create the initial contextlist. + $contextlist = new \core_privacy\local\request\contextlist(); + $contextlist->add_from_sql('SELECT id FROM {context} WHERE id = :contextid', ['contextid' => $coursecontext->id]); + $contextlist->set_component('tool_dataprivacy'); - $clcontext = new \tool_dataprivacy\contextlist_context(0, (object) [ - 'contextid' => $coursecontext->id, - 'status' => contextlist_context::STATUS_APPROVED, - 'contextlistid' => $contextlist->get('id'), - ]); - $clcontext->save(); - - $rcl = new \tool_dataprivacy\request_contextlist(0, (object) [ - 'requestid' => $request->get('id'), - 'contextlistid' => $contextlist->get('id'), - ]); - $rcl->save(); + $initialcollection = new \core_privacy\local\request\contextlist_collection($user->id); + $initialcollection->add_contextlist($contextlist); $purposes->course->purpose->set('protected', 1)->save(); - $collection = api::get_approved_contextlist_collection_for_request($request); + $collection = api::get_approved_contextlist_collection_for_collection( + $initialcollection, $user, api::DATAREQUEST_TYPE_DELETE); $this->assertCount(0, $collection); @@ -1690,7 +1500,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { /** * Test that delete requests do not filter out unexpired contexts if they are not protected. */ - public function test_get_approved_contextlist_collection_for_request_delete_course_unexpired_unprotected() { + public function test_get_approved_contextlist_collection_for_collection_delete_course_unexpired_unprotected() { $this->resetAfterTest(); $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1Y'); @@ -1702,26 +1512,17 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student'); - // Create the request, with its contextlist and context. - $request = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); - $contextlist = new \tool_dataprivacy\contextlist(0, (object) ['component' => 'tool_dataprivacy']); - $contextlist->save(); + // Create the initial contextlist. + $contextlist = new \core_privacy\local\request\contextlist(); + $contextlist->add_from_sql('SELECT id FROM {context} WHERE id = :contextid', ['contextid' => $coursecontext->id]); + $contextlist->set_component('tool_dataprivacy'); - $clcontext = new \tool_dataprivacy\contextlist_context(0, (object) [ - 'contextid' => $coursecontext->id, - 'status' => contextlist_context::STATUS_APPROVED, - 'contextlistid' => $contextlist->get('id'), - ]); - $clcontext->save(); - - $rcl = new \tool_dataprivacy\request_contextlist(0, (object) [ - 'requestid' => $request->get('id'), - 'contextlistid' => $contextlist->get('id'), - ]); - $rcl->save(); + $initialcollection = new \core_privacy\local\request\contextlist_collection($user->id); + $initialcollection->add_contextlist($contextlist); $purposes->course->purpose->set('protected', 0)->save(); - $collection = api::get_approved_contextlist_collection_for_request($request); + $collection = api::get_approved_contextlist_collection_for_collection( + $initialcollection, $user, api::DATAREQUEST_TYPE_DELETE); $this->assertCount(1, $collection); @@ -1887,108 +1688,6 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { } } - /** - * Perform setup for the test_add_request_contexts_with_status_xxxxx tests. - * - * @param int $type The type of request to create - * @return \stdClass - */ - protected function setup_test_add_request_contexts_with_status($type) { - $this->resetAfterTest(); - - $this->setAdminUser(); - - // User under test. - $s1 = $this->getDataGenerator()->create_user(); - - // Create three sample contexts. - // 1 which should not be returned; and - // 1 which will be returned and is not protected; and - // 1 which will be returned and is protected. - - $c1 = $this->getDataGenerator()->create_course(); - $c2 = $this->getDataGenerator()->create_course(); - $c3 = $this->getDataGenerator()->create_course(); - - $ctx1 = \context_course::instance($c1->id); - $ctx2 = \context_course::instance($c2->id); - $ctx3 = \context_course::instance($c3->id); - - $unprotected = api::create_purpose((object)[ - 'name' => 'Unprotected', 'retentionperiod' => 'PT1M', 'lawfulbases' => 'gdpr_art_6_1_a']); - $protected = api::create_purpose((object) [ - 'name' => 'Protected', 'retentionperiod' => 'PT1M', 'lawfulbases' => 'gdpr_art_6_1_a', 'protected' => true]); - - $cat1 = api::create_category((object)['name' => 'a']); - - // Set the defaults. - list($purposevar, $categoryvar) = data_registry::var_names_from_context( - \context_helper::get_class_for_level(CONTEXT_SYSTEM) - ); - set_config($purposevar, $unprotected->get('id'), 'tool_dataprivacy'); - set_config($categoryvar, $cat1->get('id'), 'tool_dataprivacy'); - - $contextinstance1 = api::set_context_instance((object) [ - 'contextid' => $ctx1->id, - 'purposeid' => $unprotected->get('id'), - 'categoryid' => $cat1->get('id'), - ]); - - $contextinstance2 = api::set_context_instance((object) [ - 'contextid' => $ctx2->id, - 'purposeid' => $unprotected->get('id'), - 'categoryid' => $cat1->get('id'), - ]); - - $contextinstance3 = api::set_context_instance((object) [ - 'contextid' => $ctx3->id, - 'purposeid' => $protected->get('id'), - 'categoryid' => $cat1->get('id'), - ]); - - $collection = new \core_privacy\local\request\contextlist_collection($s1->id); - $contextlist = new \core_privacy\local\request\contextlist(); - $contextlist->set_component('tool_dataprivacy'); - $contextlist->add_from_sql('SELECT id FROM {context} WHERE id IN(:ctx2, :ctx3)', [ - 'ctx2' => $ctx2->id, - 'ctx3' => $ctx3->id, - ]); - - $collection->add_contextlist($contextlist); - - // Create the sample data request. - $datarequest = api::create_data_request($s1->id, $type); - $requestid = $datarequest->get('id'); - - // Add the full collection with contexts 2, and 3. - api::add_request_contexts_with_status($collection, $requestid, \tool_dataprivacy\contextlist_context::STATUS_PENDING); - - // Mark it as approved. - api::update_request_contexts_with_status($requestid, \tool_dataprivacy\contextlist_context::STATUS_APPROVED); - - // Fetch the list. - $approvedcollection = api::get_approved_contextlist_collection_for_request($datarequest); - - return (object) [ - 'contexts' => (object) [ - 'unused' => [ - $ctx1->id, - ], - 'used' => [ - $ctx2->id, - $ctx3->id, - ], - 'unprotected' => [ - $ctx2->id, - ], - 'protected' => [ - $ctx3->id, - ], - ], - 'list' => $approvedcollection->get_contextlist_for_component('tool_dataprivacy'), - ]; - } - /** * Setup the basics with the specified retention period. * diff --git a/admin/tool/dataprivacy/tests/behat/behat_tool_dataprivacy.php b/admin/tool/dataprivacy/tests/behat/behat_tool_dataprivacy.php index 857bb336ba7..2a004d9d149 100644 --- a/admin/tool/dataprivacy/tests/behat/behat_tool_dataprivacy.php +++ b/admin/tool/dataprivacy/tests/behat/behat_tool_dataprivacy.php @@ -281,4 +281,54 @@ class behat_tool_dataprivacy extends behat_base { 'categoryid' => $category->get('id'), ]); } + + /** + * Create a dataprivacy request. + * + * @Given /^I create a dataprivacy "(?P<type_string>(?:[^"]|\\")*)" request for "(?P<user_string>(?:[^"]|\\")*)"$/ + * + * @param string $type The type of request to create (delete, or export) + * @param string $username The username to create for + */ + public function i_create_a_dataprivacy_request_for($type, $username) { + if ($type === 'delete') { + $type = \tool_dataprivacy\api::DATAREQUEST_TYPE_DELETE; + } else if ($type === 'export') { + $type = \tool_dataprivacy\api::DATAREQUEST_TYPE_EXPORT; + } else { + throw new \Behat\Behat\Tester\Exception\ExpectationException("Unknown request type '{$type}'"); + } + + $user = \core_user::get_user_by_username($username); + + \tool_dataprivacy\api::create_data_request($user->id, $type); + } + + /** + * Approve a dataprivacy request. + * + * @Given /^I approve a dataprivacy "(?P<type_string>(?:[^"]|\\")*)" request for "(?P<user_string>(?:[^"]|\\")*)"$/ + * + * @param string $type The type of request to create (delete, or export) + * @param string $username The username to create for + */ + public function i_approve_a_dataprivacy_request_for($type, $username) { + if ($type === 'delete') { + $type = \tool_dataprivacy\api::DATAREQUEST_TYPE_DELETE; + } else if ($type === 'export') { + $type = \tool_dataprivacy\api::DATAREQUEST_TYPE_EXPORT; + } else { + throw new \Behat\Behat\Tester\Exception\ExpectationException("Unknown request type '{$type}'"); + } + + $user = \core_user::get_user_by_username($username); + + $request = \tool_dataprivacy\data_request::get_record([ + 'userid' => $user->id, + 'type' => $type, + 'status' => \tool_dataprivacy\api::DATAREQUEST_STATUS_AWAITING_APPROVAL, + ]); + + \tool_dataprivacy\api::approve_data_request($request->get('id')); + } } diff --git a/admin/tool/dataprivacy/tests/behat/datadelete.feature b/admin/tool/dataprivacy/tests/behat/datadelete.feature index 99daa149851..c4bc1391b79 100644 --- a/admin/tool/dataprivacy/tests/behat/datadelete.feature +++ b/admin/tool/dataprivacy/tests/behat/datadelete.feature @@ -34,9 +34,6 @@ Feature: Data delete from the privacy API And I set the field "Type" to "Delete all of my personal data" And I press "Save changes" Then I should see "Victim User 1" - And I should see "Pending" in the "Victim User 1" "table_row" - And I run all adhoc tasks - And I reload the page And I should see "Awaiting approval" in the "Victim User 1" "table_row" And I open the action menu in "Victim User 1" "table_row" And I follow "Approve request" @@ -59,9 +56,6 @@ Feature: Data delete from the privacy API And I set the field "Type" to "Delete all of my personal data" And I press "Save changes" Then I should see "Delete all of my personal data" - And I should see "Pending" in the "Delete all of my personal data" "table_row" - And I run all adhoc tasks - And I reload the page And I should see "Awaiting approval" in the "Delete all of my personal data" "table_row" And I log out @@ -97,9 +91,6 @@ Feature: Data delete from the privacy API And I set the field "Type" to "Delete all of my personal data" And I press "Save changes" Then I should see "Victim User 1" - And I should see "Pending" in the "Victim User 1" "table_row" - And I run all adhoc tasks - And I reload the page And I should see "Awaiting approval" in the "Victim User 1" "table_row" And I log out diff --git a/admin/tool/dataprivacy/tests/behat/dataexport.feature b/admin/tool/dataprivacy/tests/behat/dataexport.feature index b2a82355967..2751695166c 100644 --- a/admin/tool/dataprivacy/tests/behat/dataexport.feature +++ b/admin/tool/dataprivacy/tests/behat/dataexport.feature @@ -30,9 +30,6 @@ Feature: Data export from the privacy API And I set the field "User" to "Victim User 1" And I press "Save changes" Then I should see "Victim User 1" - And I should see "Pending" in the "Victim User 1" "table_row" - And I run all adhoc tasks - And I reload the page And I should see "Awaiting approval" in the "Victim User 1" "table_row" And I open the action menu in "Victim User 1" "table_row" And I follow "Approve request" @@ -59,9 +56,6 @@ Feature: Data export from the privacy API And I follow "New request" And I press "Save changes" Then I should see "Export all of my personal data" - And I should see "Pending" in the "Export all of my personal data" "table_row" - And I run all adhoc tasks - And I reload the page And I should see "Awaiting approval" in the "Export all of my personal data" "table_row" And I log out @@ -99,9 +93,6 @@ Feature: Data export from the privacy API And I set the field "User" to "Victim User 1" And I press "Save changes" Then I should see "Victim User 1" - And I should see "Pending" in the "Victim User 1" "table_row" - And I run all adhoc tasks - And I reload the page And I should see "Awaiting approval" in the "Victim User 1" "table_row" And I log out diff --git a/admin/tool/dataprivacy/tests/behat/protecteddelete.feature b/admin/tool/dataprivacy/tests/behat/protecteddelete.feature new file mode 100644 index 00000000000..b4e36dde72d --- /dev/null +++ b/admin/tool/dataprivacy/tests/behat/protecteddelete.feature @@ -0,0 +1,84 @@ +@tool @tool_dataprivacy +Feature: Protected data should not be deleted + In order to delete data for users and meet legal requirements + As an privacy office + I need to be ensure that only expired or unprotected data is removed + + Background: + Given the following "users" exist: + | username | firstname | lastname | + | u1 | u1 | u1 | + And the following "courses" exist: + | fullname | shortname | startdate | enddate | + | C1 | C1 | ##1 year ago## | ##1 month ago## | + | C2 | C2 | ##1 year ago## | ##last day of next month## | + And the following "course enrolments" exist: + | user | course | role | + | u1 | C1 | student | + | u1 | C2 | student | + And the following "activities" exist: + | activity | name | intro | course | idnumber | + | forum | forump1 | Test forum description | C1 | forump1 | + | forum | forumu1 | Test forum description | C1 | forumu1 | + | forum | forump2 | Test forum description | C2 | forump2 | + | forum | forumu2 | Test forum description | C2 | forumu2 | + And the following data privacy "categories" exist: + | name | + | CAT | + And the following data privacy "purposes" exist: + | name | retentionperiod | protected | + | Site purpose | PT1H | 0 | + | prot | P1D | 1 | + | unprot | P1D | 0 | + And I set the category and purpose for the "forump1" "forum" in course "C1" to "CAT" and "prot" + And I set the category and purpose for the "forump2" "forum" in course "C2" to "CAT" and "prot" + And I set the category and purpose for the "forumu1" "forum" in course "C1" to "CAT" and "unprot" + And I set the category and purpose for the "forumu2" "forum" in course "C2" to "CAT" and "unprot" + And I set the site category and purpose to "CAT" and "Site purpose" + + @javascripta + Scenario: Unexpired and protected data is not removed + Given I log in as "u1" + And I am on "C1" course homepage + And I add a new discussion to "forump1" forum with: + | Subject | Discussion subject | + | Message | Test post in forump1 | + And I am on "C1" course homepage + And I add a new discussion to "forumu1" forum with: + | Subject | Discussion subject | + | Message | Test post in forumu1 | + And I am on "C2" course homepage + And I add a new discussion to "forump2" forum with: + | Subject | Discussion subject | + | Message | Test post in forump2 | + And I am on "C2" course homepage + And I add a new discussion to "forumu2" forum with: + | Subject | Discussion subject | + | Message | Test post in forumu2 | + And I log out + And I log in as "admin" + And I create a dataprivacy "delete" request for "u1" + And I approve a dataprivacy "delete" request for "u1" + And I run all adhoc tasks + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I should see "Deleted" in the "u1" "table_row" + + And I am on "C1" course homepage + And I follow "forump1" + And I follow "Discussion subject" + Then I should not see "Test post in forump1" + + When I am on "C1" course homepage + And I follow "forumu1" + And I follow "Discussion subject" + Then I should not see "Test post in forumu1" + + And I am on "C2" course homepage + And I follow "forump2" + And I follow "Discussion subject" + Then I should see "Test post in forump2" + + When I am on "C2" course homepage + And I follow "forumu2" + And I follow "Discussion subject" + Then I should not see "Test post in forumu2" diff --git a/admin/tool/dataprivacy/tests/data_request_test.php b/admin/tool/dataprivacy/tests/data_request_test.php index e10c12da7e0..f15bdd50ce9 100644 --- a/admin/tool/dataprivacy/tests/data_request_test.php +++ b/admin/tool/dataprivacy/tests/data_request_test.php @@ -189,7 +189,7 @@ class tool_dataprivacy_data_request_testcase extends data_privacy_testcase { $this->assertEquals('Foo', $newrequest->get('comments')); $this->assertEquals(42, $newrequest->get('requestedby')); $this->assertEquals(98, $newrequest->get('dpo')); - $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $newrequest->get('status')); + $this->assertEquals(api::DATAREQUEST_STATUS_AWAITING_APPROVAL, $newrequest->get('status')); $this->assertEquals(api::DATAREQUEST_TYPE_DELETE, $newrequest->get('type')); $this->assertEquals(api::DATAREQUEST_STATUS_REJECTED, $uut->get('status')); @@ -213,7 +213,7 @@ class tool_dataprivacy_data_request_testcase extends data_privacy_testcase { $this->assertEquals('Foo', $newrequest->get('comments')); $this->assertEquals(42, $newrequest->get('requestedby')); $this->assertEquals(98, $newrequest->get('dpo')); - $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $newrequest->get('status')); + $this->assertEquals(api::DATAREQUEST_STATUS_AWAITING_APPROVAL, $newrequest->get('status')); $this->assertEquals(api::DATAREQUEST_TYPE_DELETE, $newrequest->get('type')); $this->assertEquals(api::DATAREQUEST_STATUS_REJECTED, $uut->get('status')); diff --git a/admin/tool/dataprivacy/tests/expired_data_requests_test.php b/admin/tool/dataprivacy/tests/expired_data_requests_test.php index 662d9abd51f..8c5cb93037b 100644 --- a/admin/tool/dataprivacy/tests/expired_data_requests_test.php +++ b/admin/tool/dataprivacy/tests/expired_data_requests_test.php @@ -68,13 +68,11 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc // Create and approve data request. $this->setUser($studentuser->id); $datarequest = api::create_data_request($studentuser->id, api::DATAREQUEST_TYPE_EXPORT); - $this->setAdminUser(); - ob_start(); - $this->runAdhocTasks('\tool_dataprivacy\task\initiate_data_request_task'); $requestid = $datarequest->get('id'); - $this->setUser($dpouser->id); - api::approve_data_request($requestid); $this->setAdminUser(); + api::approve_data_request($requestid); + $this->setUser(); + ob_start(); $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task'); ob_end_clean(); @@ -114,7 +112,6 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc $this->assertEquals(0, $DB->count_records('files', $fileconditions)); } - /** * Test for \tool_dataprivacy\data_request::is_expired() * Tests for the expected request status to protect from false positive/negative, @@ -136,7 +133,6 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc // Approve the request. ob_start(); - $this->runAdhocTasks('\tool_dataprivacy\task\initiate_data_request_task'); $this->setAdminUser(); api::approve_data_request($requestid); $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task'); diff --git a/admin/tool/dataprivacy/tests/external_test.php b/admin/tool/dataprivacy/tests/external_test.php index 0db55d2bf31..b17811a0409 100644 --- a/admin/tool/dataprivacy/tests/external_test.php +++ b/admin/tool/dataprivacy/tests/external_test.php @@ -94,6 +94,7 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { // Test data request creation. $this->setUser($requester); $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + $datarequest->set('status', api::DATAREQUEST_STATUS_CANCELLED)->save(); // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); @@ -356,6 +357,7 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { $this->setUser($requester); $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + $datarequest->set('status', api::DATAREQUEST_STATUS_CANCELLED)->save(); // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); diff --git a/admin/tool/dataprivacy/tests/task_test.php b/admin/tool/dataprivacy/tests/task_test.php index 1006364b580..0dc614a43a0 100644 --- a/admin/tool/dataprivacy/tests/task_test.php +++ b/admin/tool/dataprivacy/tests/task_test.php @@ -175,9 +175,6 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { // After running the scheduled task, the user should have only one delete data request. $this->assertCount(1, api::get_data_requests($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); - // The user should not have a newly created delete data request. - $this->assertCount(0, api::get_data_requests($user->id, - [api::DATAREQUEST_STATUS_PENDING], [api::DATAREQUEST_TYPE_DELETE])); } /** @@ -212,7 +209,7 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { $user->deleted = 1; $DB->update_record('user', $user); - // The user should still have the existing finished delete data request. + // The user should still have the existing cancelled delete data request. $this->assertCount(1, \tool_dataprivacy\api::get_data_requests($user->id, [api::DATAREQUEST_STATUS_CANCELLED], [api::DATAREQUEST_TYPE_DELETE])); @@ -220,7 +217,7 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { // After running the scheduled task, the user should still have one delete data requests. $this->assertCount(1, api::get_data_requests($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); - // The user should still have the existing finished delete data request. + // The user should only have the existing cancelled delete data request. $this->assertCount(1, \tool_dataprivacy\api::get_data_requests($user->id, [api::DATAREQUEST_STATUS_CANCELLED], [api::DATAREQUEST_TYPE_DELETE])); } diff --git a/admin/tool/dataprivacy/version.php b/admin/tool/dataprivacy/version.php index 2658ecbcb6d..35ab48c7739 100644 --- a/admin/tool/dataprivacy/version.php +++ b/admin/tool/dataprivacy/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die; -$plugin->version = 2018100406; +$plugin->version = 2018110700; $plugin->requires = 2018050800; // Moodle 3.5dev (Build 2018031600) and upwards. $plugin->component = 'tool_dataprivacy';