MDL-63897 dataprivacy: Kill the preprocess stage
authorAndrew Nicols <andrew@nicols.co.uk>
Wed, 7 Nov 2018 01:37:06 +0000 (09:37 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Fri, 9 Nov 2018 03:08:07 +0000 (11:08 +0800)
22 files changed:
admin/tool/dataprivacy/classes/api.php
admin/tool/dataprivacy/classes/contextlist.php [deleted file]
admin/tool/dataprivacy/classes/contextlist_context.php [deleted file]
admin/tool/dataprivacy/classes/data_request.php
admin/tool/dataprivacy/classes/external/data_request_exporter.php
admin/tool/dataprivacy/classes/local/helper.php
admin/tool/dataprivacy/classes/request_contextlist.php [deleted file]
admin/tool/dataprivacy/classes/task/initiate_data_request_task.php [deleted file]
admin/tool/dataprivacy/classes/task/process_data_request_task.php
admin/tool/dataprivacy/db/install.xml
admin/tool/dataprivacy/db/upgrade.php
admin/tool/dataprivacy/lang/en/deprecated.txt [new file with mode: 0644]
admin/tool/dataprivacy/lang/en/tool_dataprivacy.php
admin/tool/dataprivacy/tests/api_test.php
admin/tool/dataprivacy/tests/behat/behat_tool_dataprivacy.php
admin/tool/dataprivacy/tests/behat/datadelete.feature
admin/tool/dataprivacy/tests/behat/dataexport.feature
admin/tool/dataprivacy/tests/behat/protecteddelete.feature [new file with mode: 0644]
admin/tool/dataprivacy/tests/expired_data_requests_test.php
admin/tool/dataprivacy/tests/external_test.php
admin/tool/dataprivacy/tests/task_test.php
admin/tool/dataprivacy/version.php

index 0fb74f2..a7e587f 100644 (file)
@@ -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;
 
@@ -249,7 +245,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,11 +256,6 @@ 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);
-
         return $datarequest;
     }
 
@@ -603,11 +594,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 +1052,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');
-
-            // 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);
+    public static function get_approved_contextlist_collection_for_collection(contextlist_collection $collection,
+            \stdClass $foruser, int $type) : contextlist_collection {
 
+        // 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();
-            }
-
-            // 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 = [];
+                $contextids[] = $context->id;
             }
 
-            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;
-                }
+            // 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)
+                    );
             }
-
-            $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 (file)
index ef25c11..0000000
+++ /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 (file)
index 0f4ca9f..0000000
+++ /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
-            ]
-        ];
-    }
-}
index a95d27a..22cf823 100644 (file)
@@ -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,
index b7d483c..2826fc8 100644 (file)
@@ -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.
index 25f5bf8..14d68f7 100644 (file)
@@ -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 (file)
index 5764b7e..0000000
+++ /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 (file)
index 1e02a2f..0000000
+++ /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);
-                }
-            }
-        }
-    }
-}
index 802f61a..0834263 100644 (file)
@@ -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());
index 2d4a01a..fc5e96b 100644 (file)
@@ -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"
 >
         <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"/>
index 306337d..2f52d48 100644 (file)
@@ -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 (file)
index 0000000..c400ae8
--- /dev/null
@@ -0,0 +1 @@
+statuspreprocessing,tool_dataprivacy
index 35595cf..c9f4ce1 100644 (file)
@@ -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';
index a1a5de9..8eed7cb 100644 (file)
@@ -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);
-        $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(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);
+        // 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(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();
-
-        $clcontext = new \tool_dataprivacy\contextlist_context(0, (object) [
-                'contextid' => $coursecontext->id,
-                'status' => contextlist_context::STATUS_APPROVED,
-                'contextlistid' => $contextlist->get('id'),
-            ]);
-        $clcontext->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');
 
-        $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();
-
-        $clcontext = new \tool_dataprivacy\contextlist_context(0, (object) [
-                'contextid' => $coursecontext->id,
-                'status' => contextlist_context::STATUS_APPROVED,
-                'contextlistid' => $contextlist->get('id'),
-            ]);
-        $clcontext->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');
 
-        $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.
      *
index 857bb33..2a004d9 100644 (file)
@@ -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'));
+    }
 }
index 99daa14..c4bc139 100644 (file)
@@ -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
index b2a8235..2751695 100644 (file)
@@ -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 (file)
index 0000000..b4e36dd
--- /dev/null
@@ -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"
index 662d9ab..8c5cb93 100644 (file)
@@ -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');
index 0db55d2..b17811a 100644 (file)
@@ -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();
index 1006364..0dc614a 100644 (file)
@@ -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]));
     }
index 2658ecb..35ab48c 100644 (file)
@@ -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';