Merge branch 'MDL-62016_master' of git://github.com/dmonllao/moodle
authorJake Dallimore <jake@moodle.com>
Mon, 14 May 2018 03:05:45 +0000 (11:05 +0800)
committerJake Dallimore <jake@moodle.com>
Mon, 14 May 2018 03:05:45 +0000 (11:05 +0800)
60 files changed:
admin/tool/dataprivacy/classes/api.php
admin/tool/dataprivacy/classes/external.php
admin/tool/dataprivacy/classes/local/helper.php
admin/tool/dataprivacy/classes/output/data_registry_page.php
admin/tool/dataprivacy/classes/output/my_data_requests_page.php
admin/tool/dataprivacy/classes/task/initiate_data_request_task.php
admin/tool/dataprivacy/classes/task/process_data_request_task.php
admin/tool/dataprivacy/createdatarequest_form.php
admin/tool/dataprivacy/lang/en/tool_dataprivacy.php
admin/tool/dataprivacy/tests/api_test.php
admin/tool/log/store/standard/classes/privacy/provider.php
admin/tool/monitor/classes/privacy/provider.php
admin/tool/phpunit/classes/privacy/provider.php [new file with mode: 0644]
admin/tool/policy/classes/output/page_agreedocs.php
admin/tool/policy/index.php
admin/tool/policy/tests/behat/acceptances.feature
auth/oauth2/classes/privacy/provider.php
backup/converter/convertlib.php
competency/classes/privacy/provider.php
completion/classes/privacy/provider.php
completion/tests/fixtures/completion_creation.php
completion/tests/privacy_test.php
course/classes/privacy/provider.php
course/tests/privacy_test.php
enrol/lti/classes/privacy/provider.php [new file with mode: 0644]
enrol/lti/lang/en/enrol_lti.php
enrol/lti/tests/privacy_provider_test.php [new file with mode: 0644]
grade/classes/privacy/provider.php
grade/grading/form/guide/classes/privacy/provider.php
lang/en/privacy.php
lib/classes/task/file_temp_cleanup_task.php
lib/editor/atto/plugins/accessibilitychecker/tests/behat/accessibilitychecker.feature
lib/editor/atto/plugins/image/tests/behat/image.feature
lib/editor/atto/plugins/media/tests/behat/media.feature
lib/editor/atto/tests/behat/autosave.feature
lib/tests/cronlib_test.php
message/tests/privacy_provider_test.php
mod/assign/classes/privacy/provider.php
mod/data/classes/privacy/provider.php
mod/lesson/classes/privacy/provider.php
mod/lesson/tests/behat/duplicate_lesson_page.feature
mod/lesson/tests/behat/questions_images.feature
mod/quiz/classes/privacy/provider.php
mod/quiz/tests/behat/manually_mark_question.feature
mod/workshop/tests/behat/embedded_images.feature
pix/e/insert_edit_video.svg
portfolio/boxnet/classes/privacy/provider.php
portfolio/flickr/classes/privacy/provider.php
portfolio/googledocs/classes/privacy/provider.php
portfolio/mahara/classes/privacy/provider.php
portfolio/picasa/classes/privacy/provider.php
privacy/classes/local/request/moodle_content_writer.php
privacy/classes/manager.php
privacy/classes/tests/request/content_writer.php
privacy/tests/moodle_content_writer_test.php
privacy/tests/provider_test.php
question/type/calculatedmulti/questiontype.php
rating/classes/privacy/provider.php
theme/boost/scss/moodle/forms.scss
version.php

index 9054612..7bffc9f 100644 (file)
@@ -37,6 +37,7 @@ use moodle_url;
 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;
 
@@ -186,8 +187,25 @@ class api {
         $datarequest = new data_request();
         // The user the request is being made for.
         $datarequest->set('userid', $foruser);
+
+        $requestinguser = $USER->id;
+        // Check when the user is making a request on behalf of another.
+        if ($requestinguser != $foruser) {
+            if (self::is_site_dpo($requestinguser)) {
+                // The user making the request is a DPO. Should be fine.
+                $datarequest->set('dpo', $requestinguser);
+            } else {
+                // If not a DPO, only users with the capability to make data requests for the user should be allowed.
+                // (e.g. users with the Parent role, etc).
+                if (!api::can_create_data_request_for_user($foruser)) {
+                    $forusercontext = \context_user::instance($foruser);
+                    throw new required_capability_exception($forusercontext,
+                            'tool/dataprivacy:makedatarequestsforchildren', 'nopermissions', '');
+                }
+            }
+        }
         // The user making the request.
-        $datarequest->set('requestedby', $USER->id);
+        $datarequest->set('requestedby', $requestinguser);
         // Set status.
         $datarequest->set('status', self::DATAREQUEST_STATUS_PENDING);
         // Set request type.
@@ -218,16 +236,29 @@ class api {
      * @throws dml_exception
      */
     public static function get_data_requests($userid = 0) {
-        global $USER;
+        global $DB, $USER;
         $results = [];
         $sort = 'status ASC, timemodified ASC';
         if ($userid) {
             // Get the data requests for the user or data requests made by the user.
-            $select = "userid = :userid OR requestedby = :requestedby";
+            $select = "(userid = :userid OR requestedby = :requestedby)";
             $params = [
                 'userid' => $userid,
                 'requestedby' => $userid
             ];
+
+            // Build a list of user IDs that the user is allowed to make data requests for.
+            // Of course, the user should be included in this list.
+            $alloweduserids = [$userid];
+            // Get any users that the user can make data requests for.
+            if ($children = helper::get_children_of_user($userid)) {
+                // Get the list of user IDs of the children and merge to the allowed user IDs.
+                $alloweduserids = array_merge($alloweduserids, array_keys($children));
+            }
+            list($insql, $inparams) = $DB->get_in_or_equal($alloweduserids, SQL_PARAMS_NAMED);
+            $select .= " AND userid $insql";
+            $params = array_merge($params, $inparams);
+
             $results = data_request::get_records_select($select, $params, $sort);
         } else {
             // If the current user is one of the site's Data Protection Officers, then fetch all data requests.
@@ -290,17 +321,19 @@ class api {
      * @param int $requestid The request identifier.
      * @param int $status The request status.
      * @param int $dpoid The user ID of the Data Protection Officer
+     * @param string $comment The comment about the status update.
      * @return bool
      * @throws invalid_persistent_exception
      * @throws coding_exception
      */
-    public static function update_request_status($requestid, $status, $dpoid = 0) {
+    public static function update_request_status($requestid, $status, $dpoid = 0, $comment = '') {
         // Update the request.
         $datarequest = new data_request($requestid);
         $datarequest->set('status', $status);
         if ($dpoid) {
             $datarequest->set('dpo', $dpoid);
         }
+        $datarequest->set('dpocomment', $comment);
         return $datarequest->update();
     }
 
@@ -454,6 +487,19 @@ class api {
         return message_send($message);
     }
 
+    /**
+     * Checks whether a non-DPO user can make a data request for another user.
+     *
+     * @param int $user The user ID of the target user.
+     * @param int $requester The user ID of the user making the request.
+     * @return bool
+     * @throws coding_exception
+     */
+    public static function can_create_data_request_for_user($user, $requester = null) {
+        $usercontext = \context_user::instance($user);
+        return has_capability('tool/dataprivacy:makedatarequestsforchildren', $usercontext, $requester);
+    }
+
     /**
      * Creates a new data purpose.
      *
index 63f8ecb..6587c40 100644 (file)
@@ -96,8 +96,8 @@ class external extends external_api {
         self::validate_context($context);
 
         // Ensure the request exists.
-        $select = 'id = :id AND requestedby = :requestedby';
-        $params = ['id' => $requestid, 'requestedby' => $USER->id];
+        $select = 'id = :id AND (userid = :userid OR requestedby = :requestedby)';
+        $params = ['id' => $requestid, 'userid' => $USER->id, 'requestedby' => $USER->id];
         $requestexists = data_request::record_exists_select($select, $params);
 
         $result = false;
index 77664b8..d7c3436 100644 (file)
@@ -108,4 +108,42 @@ class helper {
                 throw new moodle_exception('errorinvalidrequeststatus', 'tool_dataprivacy');
         }
     }
+
+    /**
+     * Get the users that a user can make data request for.
+     *
+     * E.g. User having a parent role and has the 'tool/dataprivacy:makedatarequestsforchildren' capability.
+     * @param int $userid The user's ID.
+     * @return array
+     */
+    public static function get_children_of_user($userid) {
+        global $DB;
+
+        // Get users that the user has role assignments to.
+        $allusernames = get_all_user_name_fields(true, 'u');
+        $sql = "SELECT u.id, $allusernames
+                  FROM {role_assignments} ra, {context} c, {user} u
+                 WHERE ra.userid = :userid
+                       AND ra.contextid = c.id
+                       AND c.instanceid = u.id
+                       AND c.contextlevel = :contextlevel";
+        $params = [
+            'userid' => $userid,
+            'contextlevel' => CONTEXT_USER
+        ];
+
+        // The final list of users that we will return;
+        $finalresults = [];
+
+        // Our prospective list of users.
+        if ($candidates = $DB->get_records_sql($sql, $params)) {
+            foreach ($candidates as $key => $child) {
+                $childcontext = \context_user::instance($child->id);
+                if (has_capability('tool/dataprivacy:makedatarequestsforchildren', $childcontext, $userid)) {
+                    $finalresults[$key] = $child;
+                }
+            }
+        }
+        return $finalresults;
+    }
 }
index 4a5348c..cfdad43 100644 (file)
@@ -316,29 +316,23 @@ class data_registry_page implements renderable, templatable {
 
         $branches = [];
 
-        $blockinstances = \core_block_external::get_course_blocks($coursecontext->instanceid);
-        if (empty($blockinstances['blocks'])) {
-            return $branches;
-        }
+        $children = $coursecontext->get_child_contexts();
+        foreach ($children as $childcontext) {
 
-        foreach ($blockinstances['blocks'] as $bi) {
-            if (function_exists('block_instance_by_id')) {
-                $blockinstance = block_instance_by_id($bi['instanceid']);
-            } else {
-                // TODO To be removed when MDL-61621 gets integrated.
-                $blockinstance = $DB->get_record('block_instances', ['id' => $bi['instanceid']]);
-                $blockinstance = block_instance($blockinstance->blockname, $blockinstance);
+            if ($childcontext->contextlevel !== CONTEXT_BLOCK) {
+                continue;
             }
-            $blockcontext = \context_block::instance($bi['instanceid']);
-            $displayname = shorten_text(format_string($blockinstance->get_title(), true, ['context' => $blockcontext->id]));
+
+            $blockinstance = block_instance_by_id($childcontext->instanceid);
+            $displayname = shorten_text(format_string($blockinstance->get_title(), true, ['context' => $childcontext]));
             $branches[] = self::complete([
                 'text' => $displayname,
-                'contextid' => $blockcontext->id,
+                'contextid' => $childcontext->id,
             ]);
+
         }
 
         return $branches;
-
     }
 
     /**
index 6b90669..25229f0 100644 (file)
@@ -67,6 +67,8 @@ class my_data_requests_page implements renderable, templatable {
      * @throws moodle_exception
      */
     public function export_for_template(renderer_base $output) {
+        global $USER;
+
         $data = new stdClass();
         $data->newdatarequesturl = new moodle_url('/admin/tool/dataprivacy/createdatarequest.php');
 
@@ -80,10 +82,24 @@ class my_data_requests_page implements renderable, templatable {
             $requestid = $request->get('id');
             $status = $request->get('status');
             $userid = $request->get('userid');
-            $usercontext = context_user::instance($userid);
-            $requestexporter = new data_request_exporter($request, ['context' => $usercontext]);
+
+            $usercontext = context_user::instance($userid, IGNORE_MISSING);
+            if (!$usercontext) {
+                // Use the context system.
+                $outputcontext = \context_system::instance();
+            } else {
+                $outputcontext = $usercontext;
+            }
+
+            $requestexporter = new data_request_exporter($request, ['context' => $outputcontext]);
             $item = $requestexporter->export($output);
 
+            if ($request->get('userid') != $USER->id) {
+                // Append user name if it differs from $USER.
+                $a = (object)['typename' => $item->typename, 'user' => $item->foruser->fullname];
+                $item->typename = get_string('requesttypeuser', 'tool_dataprivacy', $a);
+            }
+
             $candownload = false;
             $cancancel = true;
             switch ($status) {
@@ -107,7 +123,7 @@ class my_data_requests_page implements renderable, templatable {
                 $canceltext = get_string('cancelrequest', 'tool_dataprivacy');
                 $actions[] = new action_menu_link_secondary($cancelurl, null, $canceltext, $canceldata);
             }
-            if ($candownload) {
+            if ($candownload && $usercontext) {
                 $downloadurl = moodle_url::make_pluginfile_url($usercontext->id, 'tool_dataprivacy', 'export', $requestid, '/',
                         'export.zip', true);
                 $downloadtext = get_string('download', 'tool_dataprivacy');
index 4e62b74..0cebeb0 100644 (file)
@@ -70,6 +70,27 @@ class initiate_data_request_task extends adhoc_task {
             return;
         }
 
+        $requestedby = $datarequest->get('requestedby');
+        $valid = true;
+        $comment = '';
+        $foruser = $datarequest->get('userid');
+        if ($foruser != $requestedby) {
+            if (!$valid = api::can_create_data_request_for_user($foruser, $requestedby)) {
+                $params = (object)[
+                    'requestedby' => $requestedby,
+                    'userid' => $foruser
+                ];
+                $comment = get_string('errornocapabilitytorequestforothers', 'tool_dataprivacy', $params);
+                mtrace($comment);
+            }
+        }
+        // Reject the request outright if it's invalid.
+        if (!$valid) {
+            $dpo = $datarequest->get('dpo');
+            api::update_request_status($requestid, api::DATAREQUEST_STATUS_REJECTED, $dpo, $comment);
+            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);
index bacf29a..589ef01 100644 (file)
@@ -182,23 +182,27 @@ class process_data_request_task extends adhoc_task {
         }
         mtrace('Message sent to user: ' . $messagetextdata['username']);
 
-        // Send to the requester as well. requestedby is 0 if the request was made on behalf of the user by a DPO.
-        if (!empty($request->requestedby) && $foruser->id != $request->requestedby) {
-            $requestedby = core_user::get_user($request->requestedby);
-            $message->userto = $requestedby;
-            $messagetextdata['username'] = fullname($requestedby);
-            // Render message email body.
-            $messagehtml = $output->render_from_template('tool_dataprivacy/data_request_results_email', $messagetextdata);
-            $message->fullmessage = html_to_text($messagehtml);
-            $message->fullmessagehtml = $messagehtml;
-
-            // Send message.
-            if ($emailonly) {
-                email_to_user($requestedby, $dpo, $subject, $message->fullmessage, $messagehtml);
-            } else {
-                message_send($message);
+        // Send to requester as well if this request was made on behalf of another user who's not a DPO,
+        // and has the capability to make data requests for the user (e.g. Parent).
+        if (!api::is_site_dpo($request->requestedby) && $foruser->id != $request->requestedby) {
+            // Ensure the requester has the capability to make data requests for this user.
+            if (api::can_create_data_request_for_user($request->userid, $request->requestedby)) {
+                $requestedby = core_user::get_user($request->requestedby);
+                $message->userto = $requestedby;
+                $messagetextdata['username'] = fullname($requestedby);
+                // Render message email body.
+                $messagehtml = $output->render_from_template('tool_dataprivacy/data_request_results_email', $messagetextdata);
+                $message->fullmessage = html_to_text($messagehtml);
+                $message->fullmessagehtml = $messagehtml;
+
+                // Send message.
+                if ($emailonly) {
+                    email_to_user($requestedby, $dpo, $subject, $message->fullmessage, $messagehtml);
+                } else {
+                    message_send($message);
+                }
+                mtrace('Message sent to requester: ' . $messagetextdata['username']);
             }
-            mtrace('Message sent to requester: ' . $messagetextdata['username']);
         }
 
         if ($request->type == api::DATAREQUEST_TYPE_DELETE) {
index b42aaf3..c02d9d4 100644 (file)
@@ -23,6 +23,7 @@
  */
 
 use tool_dataprivacy\api;
+use tool_dataprivacy\local\helper;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -58,27 +59,12 @@ class tool_dataprivacy_data_request_form extends moodleform {
 
         } else {
             // Get users whom you are being a guardian to if your role has the capability to make data requests for children.
-            $allusernames = get_all_user_name_fields(true, 'u');
-            $sql = "SELECT u.id, $allusernames
-                      FROM {role_assignments} ra, {context} c, {user} u
-                     WHERE ra.userid = :userid
-                           AND ra.contextid = c.id
-                           AND c.instanceid = u.id
-                           AND c.contextlevel = :contextlevel";
-            $params = [
-                'userid' => $USER->id,
-                'contextlevel' => CONTEXT_USER
-            ];
-            $children = $DB->get_records_sql($sql, $params);
-
-            if ($children) {
-                $useroptions = [];
-                $useroptions[$USER->id] = fullname($USER);
-                foreach ($children as $child) {
-                    $childcontext = context_user::instance($child->id);
-                    if (has_capability('tool/dataprivacy:makedatarequestsforchildren', $childcontext)) {
-                        $useroptions[$child->id] = fullname($child);
-                    }
+            if ($children = helper::get_children_of_user($USER->id)) {
+                $useroptions = [
+                    $USER->id => fullname($USER)
+                ];
+                foreach ($children as $key => $child) {
+                    $useroptions[$key] = fullname($child);
                 }
                 $mform->addElement('autocomplete', 'userid', get_string('requestfor', 'tool_dataprivacy'), $useroptions);
                 $mform->addRule('userid', null, 'required', null, 'client');
index fbfaedb..6b34085 100644 (file)
@@ -93,6 +93,7 @@ $string['effectiveretentionperioduser'] = '{$a} (since the last time the user ac
 $string['emailsalutation'] = 'Dear {$a},';
 $string['errorinvalidrequeststatus'] = 'Invalid request status!';
 $string['errorinvalidrequesttype'] = 'Invalid request type!';
+$string['errornocapabilitytorequestforothers'] = 'User {$a->requestedby} doesn\'t have the capability to make a data request on behalf of user {$a->userid}';
 $string['errornoexpiredcontexts'] = 'There are no expired contexts to process';
 $string['errorcontexthasunexpiredchildren'] = 'The context "{$a}" still has sub-contexts that have not yet expired. No contexts have been flagged for deletion.';
 $string['errorrequestalreadyexists'] = 'You already have an ongoing request.';
@@ -195,6 +196,7 @@ $string['requestfor'] = 'Requesting for';
 $string['requeststatus'] = 'Status';
 $string['requestsubmitted'] = 'Your request has been submitted to the site\'s Data Protection Officer';
 $string['requesttype'] = 'Type';
+$string['requesttypeuser'] = '{$a->typename} ({$a->user})';
 $string['requesttype_help'] = 'Select the reason why you would like to contact the site\'s Data Protection Officer';
 $string['requesttypedelete'] = 'Delete all of my personal data';
 $string['requesttypedeleteshort'] = 'Delete';
index 2589607..6e06478 100644 (file)
@@ -57,6 +57,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
     public function test_update_request_status() {
         $generator = new testing_data_generator();
         $s1 = $generator->create_user();
+        $this->setUser($s1);
 
         // Create the sample data request.
         $datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
@@ -145,6 +146,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         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');
 
@@ -186,6 +188,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         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');
 
@@ -203,6 +206,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $teacher = $generator->create_user();
 
         // Create the sample data request.
+        $this->setUser($student);
         $datarequest = api::create_data_request($student->id, api::DATAREQUEST_TYPE_EXPORT);
 
         $requestid = $datarequest->get('id');
@@ -286,6 +290,71 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
         $this->assertEquals($user->id, $datarequest->get('userid'));
         $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($comment, $datarequest->get('comments'));
+
+        // Test adhoc task creation.
+        $adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class);
+        $this->assertCount(1, $adhoctasks);
+    }
+
+    /**
+     * Test for api::create_data_request() made by DPO.
+     */
+    public function test_create_data_request_by_dpo() {
+        global $USER;
+
+        $generator = new testing_data_generator();
+        $user = $generator->create_user();
+        $comment = 'sample comment';
+
+        // Login as DPO (Admin is DPO by default).
+        $this->setAdminUser();
+
+        // Test data request creation.
+        $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
+        $this->assertEquals($user->id, $datarequest->get('userid'));
+        $this->assertEquals($USER->id, $datarequest->get('requestedby'));
+        $this->assertEquals($USER->id, $datarequest->get('dpo'));
+        $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
+        $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $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);
+    }
+
+    /**
+     * Test for api::create_data_request() made by a parent.
+     */
+    public function test_create_data_request_by_parent() {
+        global $DB;
+
+        $generator = new testing_data_generator();
+        $user = $generator->create_user();
+        $parent = $generator->create_user();
+        $comment = 'sample comment';
+
+        // Get the teacher role pretend it's the parent roles ;).
+        $systemcontext = context_system::instance();
+        $usercontext = context_user::instance($user->id);
+        $parentroleid = $DB->get_field('role', 'id', array('shortname' => 'teacher'));
+        // Give the manager role with the capability to manage data requests.
+        assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentroleid, $systemcontext->id, true);
+        // Assign the parent to user.
+        role_assign($parentroleid, $parent->id, $usercontext->id);
+
+        // Login as the user's parent.
+        $this->setUser($parent);
+
+        // Test data request creation.
+        $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
+        $this->assertEquals($user->id, $datarequest->get('userid'));
+        $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($comment, $datarequest->get('comments'));
@@ -351,8 +420,10 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $comment = 'sample comment';
 
         // Make a data request as user 1.
+        $this->setUser($user1);
         $d1 = api::create_data_request($user1->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
         // Make a data request as user 2.
+        $this->setUser($user2);
         $d2 = api::create_data_request($user2->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
 
         // Fetching data requests of specific users.
@@ -406,6 +477,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $user1 = $generator->create_user();
 
         // Make a data request as user 1.
+        $this->setUser($user1);
         $request = api::create_data_request($user1->id, api::DATAREQUEST_TYPE_EXPORT);
         // Set the status.
         api::update_request_status($request->get('id'), $status);
index 1994e29..2761ca2 100644 (file)
@@ -74,10 +74,8 @@ class provider implements
      */
     public static function add_contexts_for_userid(contextlist $contextlist, $userid) {
         $sql = "
-            SELECT ctx.id
-              FROM {context} ctx
-              JOIN {logstore_standard_log} l
-                ON l.contextid = ctx.id
+            SELECT l.contextid
+              FROM {logstore_standard_log} l
              WHERE l.userid = :userid1
                 OR l.relateduserid = :userid2
                 OR l.realuserid = :userid3";
index 9b481d3..b2174ef 100644 (file)
@@ -91,8 +91,10 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l
         $sql = "SELECT DISTINCT ctx.id
                   FROM {context} ctx
              LEFT JOIN {tool_monitor_rules} mr ON ctx.instanceid = mr.userid AND ctx.contextlevel = :contextuserrule
+                       AND mr.userid = :useridsubscriptions
              LEFT JOIN {tool_monitor_subscriptions} ms ON ctx.instanceid = ms.userid AND ctx.contextlevel = :contextusersub
-                 WHERE (ms.userid = :useridrules OR mr.userid = :useridsubscriptions)";
+                       AND ms.userid = :useridrules
+                 WHERE ms.id IS NOT NULL OR mr.id IS NOT NULL";
 
         $contextlist = new contextlist();
         $contextlist->add_from_sql($sql, $params);
diff --git a/admin/tool/phpunit/classes/privacy/provider.php b/admin/tool/phpunit/classes/privacy/provider.php
new file mode 100644 (file)
index 0000000..ab9c4a8
--- /dev/null
@@ -0,0 +1,46 @@
+<?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/>.
+
+/**
+ * Privacy Subsystem implementation for tool_phpunit.
+ *
+ * @package    tool_phpunit
+ * @copyright  2018 Mark Nelson <markn@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace tool_phpunit\privacy;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Privacy Subsystem for tool_phpunit implementing null_provider.
+ *
+ * @copyright  2018 Mark Nelson <markn@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class provider implements \core_privacy\local\metadata\null_provider {
+
+    /**
+     * Get the language string identifier with the component's language
+     * file to explain why this plugin stores no data.
+     *
+     * @return  string
+     */
+    public static function get_reason() : string {
+        return 'privacy:metadata';
+    }
+}
index 6fb5330..5ede161 100644 (file)
@@ -278,9 +278,7 @@ class page_agreedocs implements renderable, templatable {
         // and $SESSION->wantsurl is defined, redirect to the return page.
         $hasagreedsignupuser = empty($USER->id) && $this->signupuserpolicyagreed;
         $hasagreedloggeduser = $USER->id == $userid && !empty($USER->policyagreed);
-        $canrevoke = api::can_revoke_policies($USER->id);
-        if (!is_siteadmin() && ($hasagreedsignupuser ||
-            ($hasagreedloggeduser && !$canrevoke))) {
+        if (!is_siteadmin() && ($hasagreedsignupuser || $hasagreedloggeduser)) {
             $this->redirect_to_previous_url();
         }
 
@@ -295,7 +293,6 @@ class page_agreedocs implements renderable, templatable {
 
         // Page setup.
         $PAGE->set_context(context_system::instance());
-        $PAGE->set_pagelayout('standard');
         $PAGE->set_url($myurl);
         $PAGE->set_heading($SITE->fullname);
         $PAGE->set_title(get_string('policiesagreements', 'tool_policy'));
index c420c5f..4be8554 100644 (file)
@@ -41,6 +41,7 @@ $agreedocs = optional_param_array('agreedoc', null, PARAM_INT);
 $behalfid = optional_param('userid', null, PARAM_INT);
 
 $PAGE->set_context(context_system::instance());
+$PAGE->set_pagelayout('standard');
 $PAGE->set_url('/admin/tool/policy/index.php');
 $PAGE->set_popup_notification_allowed(false);
 
index 02588bd..b8c6d6e 100644 (file)
@@ -80,6 +80,7 @@ Feature: Viewing acceptances reports and accepting on behalf of other users
     And I press "Next"
     And I set the field "I agree to the This site policy" to "1"
     And I press "Next"
+    And I should not see "Next"
     And I navigate to "Users > Privacy and policies > Manage policies" in site administration
     And I click on "1 of 4 (25%)" "link" in the "This site policy" "table_row"
     And I click on "Consent not given" "link" in the "User One" "table_row"
index 2ab7ff0..5a3476a 100644 (file)
@@ -75,8 +75,7 @@ class provider implements
     public static function get_contexts_for_userid(int $userid) : contextlist {
         $sql = "SELECT ctx.id
                   FROM {auth_oauth2_linked_login} ao
-                  JOIN {user} u ON ao.userid = u.id
-                  JOIN {context} ctx ON ctx.instanceid = u.id AND ctx.contextlevel = :contextlevel
+                  JOIN {context} ctx ON ctx.instanceid = ao.userid AND ctx.contextlevel = :contextlevel
                  WHERE ao.userid = :userid";
         $params = ['userid' => $userid, 'contextlevel' => CONTEXT_USER];
         $contextlist = new contextlist();
index 1fdcf1d..9a3f87a 100644 (file)
@@ -239,15 +239,17 @@ abstract class base_converter implements loggable {
     protected function replace_tempdir() {
         global $CFG;
 
+        $tempdir = $this->get_tempdir_path();
+
         if (empty($CFG->keeptempdirectoriesonbackup)) {
-            fulldelete($this->get_tempdir_path());
+            fulldelete($tempdir);
         } else {
-            if (!rename($this->get_tempdir_path(), $this->get_tempdir_path()  . '_' . $this->get_name() . '_' . $this->id . '_source')) {
+            if (!rename($tempdir, $tempdir . '_' . $this->get_name() . '_' . $this->id . '_source')) {
                 throw new convert_exception('failed_rename_source_tempdir');
             }
         }
 
-        if (!rename($this->get_workdir_path(), $this->get_tempdir_path())) {
+        if (!rename($this->get_workdir_path(), $tempdir)) {
             throw new convert_exception('failed_move_converted_into_place');
         }
     }
index 686d6a3..39ff45c 100644 (file)
@@ -250,11 +250,13 @@ class provider implements
                  ON tpl.contextid = ctx.id
           LEFT JOIN {" . template_cohort::TABLE . "} tch
                  ON tch.templateid = tpl.id
+                AND tch.usermodified = :userid2
           LEFT JOIN {" . template_competency::TABLE . "} tc
                  ON tc.templateid = tpl.id
+                AND tc.usermodified = :userid3
               WHERE tpl.usermodified = :userid1
-                 OR tch.usermodified = :userid2
-                 OR tc.usermodified = :userid3";
+                 OR tch.id IS NOT NULL
+                 OR tc.id IS NOT NULL";
         $params = ['userid1' => $userid, 'userid2' => $userid, 'userid3' => $userid];
         $contextlist->add_from_sql($sql, $params);
 
@@ -309,12 +311,14 @@ class provider implements
                 AND ctx.contextlevel = :userlevel
           LEFT JOIN {" . plan_competency::TABLE . "} pc
                  ON pc.planid = p.id
+                AND pc.usermodified = :userid3
           LEFT JOIN {" . user_competency_plan::TABLE . "} upc
                  ON upc.planid = p.id
+                AND upc.usermodified = :userid4
               WHERE p.usermodified = :userid1
                  OR p.reviewerid = :userid2
-                 OR pc.usermodified = :userid3
-                 OR upc.usermodified = :userid4";
+                 OR pc.id IS NOT NULL
+                 OR upc.id IS NOT NULL";
         $params = [
             'userlevel' => CONTEXT_USER,
             'userid1' => $userid,
@@ -333,17 +337,19 @@ class provider implements
                 AND ctx.contextlevel = :userlevel1
           LEFT JOIN {" . evidence::TABLE . "} e
                  ON e.usercompetencyid = uc.id
+                AND (e.usermodified = :userid3 OR e.actionuserid = :userid4)
           LEFT JOIN {" . user_evidence::TABLE . "} ue
                  ON ue.userid = ctx.instanceid
                 AND ctx.contextlevel = :userlevel2
+                AND ue.usermodified = :userid5
           LEFT JOIN {" . user_evidence_competency::TABLE . "} uec
                  ON uec.userevidenceid = ue.id
+                AND uec.usermodified = :userid6
               WHERE uc.usermodified = :userid1
                  OR uc.reviewerid = :userid2
-                 OR e.usermodified = :userid3
-                 OR e.actionuserid = :userid4
-                 OR ue.usermodified = :userid5
-                 OR uec.usermodified = :userid6";
+                 OR e.id IS NOT NULL
+                 OR ue.id IS NOT NULL
+                 OR uec.id IS NOT NULL";
         $params = [
             'userlevel1' => CONTEXT_USER,
             'userlevel2' => CONTEXT_USER,
@@ -367,15 +373,19 @@ class provider implements
           LEFT JOIN {" . user_competency::TABLE . "} uc
                  ON uc.userid = ctx.instanceid
                 AND ctx.contextlevel = :userlevel2
+                AND uc.userid = :userid2
           LEFT JOIN {" . user_evidence::TABLE . "} ue
                  ON ue.userid = ctx.instanceid
                 AND ctx.contextlevel = :userlevel3
+                AND ue.userid = :userid3
           LEFT JOIN {" . user_competency_course::TABLE . "} ucc
-                 ON ucc.courseid = ctx.instanceid AND ctx.contextlevel = :courselevel
+                 ON ucc.courseid = ctx.instanceid
+                AND ctx.contextlevel = :courselevel
+                AND ucc.userid = :userid4
               WHERE p.userid = :userid1
-                 OR uc.userid = :userid2
-                 OR ue.userid = :userid3
-                 OR ucc.userid = :userid4";
+                 OR uc.id IS NOT NULL
+                 OR ue.id IS NOT NULL
+                 OR ucc.id IS NOT NULL";
         $params = [
             'userlevel1' => CONTEXT_USER,
             'userlevel2' => CONTEXT_USER,
index e8d2f9e..0be20a2 100644 (file)
@@ -89,8 +89,10 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l
 
         $join = "JOIN {course_completion_criteria} {$cccalias} ON {$joinfield} = {$cccalias}.course
              LEFT JOIN {course_modules_completion} {$cmcalias} ON {$cccalias}.moduleinstance = {$cmcalias}.coursemoduleid
-             LEFT JOIN {course_completion_crit_compl} {$ccccalias} ON {$ccccalias}.criteriaid = {$cccalias}.id";
-        $where = "{$cmcalias}.userid = :{$prefix}_moduleuserid OR {$ccccalias}.userid = :{$prefix}_courseuserid";
+                        AND {$cmcalias}.userid = :{$prefix}_moduleuserid
+             LEFT JOIN {course_completion_crit_compl} {$ccccalias} ON {$ccccalias}.criteriaid = {$cccalias}.id
+                        AND {$ccccalias}.userid = :{$prefix}_courseuserid";
+        $where = "{$cmcalias}.id IS NOT NULL OR {$ccccalias}.id IS NOT NULL";
         $params = ["{$prefix}_moduleuserid" => $userid, "{$prefix}_courseuserid" => $userid];
 
         return [$join, $where, $params];
index b064d5a..30ff391 100644 (file)
@@ -103,8 +103,9 @@ trait completion_creation {
      * Complete some of the course completion criteria.
      *
      * @param  stdClass $user The user object
+     * @param  bool $modulecompletion If true will complete the activity module completion thing.
      */
-    public function complete_course($user) {
+    public function complete_course($user, $modulecompletion = true) {
         $this->getDataGenerator()->enrol_user($user->id, $this->course->id, 'student');
         $completion = new \completion_info($this->course);
         $criteriacompletions = $completion->get_completions($user->id, COMPLETION_CRITERIA_TYPE_ROLE);
@@ -112,7 +113,9 @@ trait completion_creation {
         foreach ($criteriacompletions as $ccompletion) {
             $criteria->complete($ccompletion);
         }
-        // Set activity as complete.
-        $completion->update_state($this->cm, COMPLETION_COMPLETE, $user->id);
+        if ($modulecompletion) {
+            // Set activity as complete.
+            $completion->update_state($this->cm, COMPLETION_COMPLETE, $user->id);
+        }
     }
 }
index 7e1939a..d41863a 100644 (file)
@@ -47,7 +47,7 @@ class core_completion_privacy_test extends \core_privacy\tests\provider_testcase
         $this->resetAfterTest();
         $user = $this->getDataGenerator()->create_user();
         $this->create_course_completion();
-        $this->complete_course($user);
+        $this->complete_course($user, false);
 
         list($join, $where, $params) = \core_completion\privacy\provider::get_course_completion_join_sql($user->id, 'comp', 'c.id');
         $sql = "SELECT DISTINCT c.id
index 41a8136..4d1941a 100644 (file)
@@ -158,13 +158,14 @@ class provider implements
 
         foreach ($coursedata as $course) {
             $context = \context_course::instance($course->id);
+            $courseformat = $course->format !== 'site' ? get_string('pluginname', 'format_' . $course->format) : get_string('site');
             $data = (object) [
                 'fullname' => $course->fullname,
                 'shortname' => $course->shortname,
                 'idnumber' => $course->idnumber,
                 'summary' => writer::with_context($context)->rewrite_pluginfile_urls([], 'course', 'summary', 0,
                                                                                      format_string($course->summary)),
-                'format' => get_string('pluginname', 'format_' . $course->format),
+                'format' => $courseformat,
                 'startdate' => transform::datetime($course->startdate),
                 'enddate' => transform::datetime($course->enddate)
             ];
index b2d9033..3f4ecee 100644 (file)
@@ -105,7 +105,7 @@ class core_course_privacy_testcase extends \core_privacy\tests\provider_testcase
         $this->resetAfterTest();
 
         // Create a course and a single module.
-        $course1 = $this->getDataGenerator()->create_course(['fullname' => 'Course 1', 'shortname' => 'C1']);
+        $course1 = $this->getDataGenerator()->create_course(['fullname' => 'Course 1', 'shortname' => 'C1', 'format' => 'site']);
         $context1 = context_course::instance($course1->id);
         $modassign = $this->getDataGenerator()->create_module('assign', ['course' => $course1->id, 'name' => 'assign test 1']);
         $assigncontext = context_module::instance($modassign->cmid);
diff --git a/enrol/lti/classes/privacy/provider.php b/enrol/lti/classes/privacy/provider.php
new file mode 100644 (file)
index 0000000..c8a43b0
--- /dev/null
@@ -0,0 +1,201 @@
+<?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/>.
+/**
+ * Privacy Subsystem implementation for enrol_lti.
+ *
+ * @package    enrol_lti
+ * @copyright  2018 Mark Nelson <markn@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace enrol_lti\privacy;
+
+use core_privacy\local\metadata\collection;
+use core_privacy\local\request\approved_contextlist;
+use core_privacy\local\request\contextlist;
+use core_privacy\local\request\transform;
+use core_privacy\local\request\writer;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Privacy Subsystem for enrol_lti.
+ *
+ * @copyright  2018 Mark Nelson <markn@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class provider implements
+    \core_privacy\local\metadata\provider,
+    \core_privacy\local\request\plugin\provider {
+
+    /**
+     * Return the fields which contain personal data.
+     *
+     * @param collection $items a reference to the collection to use to store the metadata.
+     * @return collection the updated collection of metadata items.
+     */
+    public static function get_metadata(collection $items) : collection {
+        $items->add_database_table(
+            'enrol_lti_users',
+            [
+                'userid' => 'privacy:metadata:enrol_lti_users:userid',
+                'lastgrade' => 'privacy:metadata:enrol_lti_users:lastgrade',
+                'lastaccess' => 'privacy:metadata:enrol_lti_users:lastaccess',
+                'timecreated' => 'privacy:metadata:enrol_lti_users:timecreated'
+            ],
+            'privacy:metadata:enrol_lti_users'
+        );
+
+        return $items;
+    }
+
+    /**
+     * Get the list of contexts that contain user information for the specified user.
+     *
+     * @param int $userid The user to search.
+     * @return contextlist The contextlist containing the list of contexts used in this plugin.
+     */
+    public static function get_contexts_for_userid(int $userid) : contextlist {
+        $contextlist = new contextlist();
+
+        $sql = "SELECT DISTINCT ctx.id
+                  FROM {enrol_lti_users} ltiusers
+                  JOIN {enrol_lti_tools} ltitools
+                    ON ltiusers.toolid = ltitools.id
+                  JOIN {context} ctx
+                    ON ctx.id = ltitools.contextid
+                 WHERE ltiusers.userid = :userid";
+        $params = ['userid' => $userid];
+        $contextlist->add_from_sql($sql, $params);
+
+        return $contextlist;
+    }
+
+    /**
+     * Export all user data for the specified user, in the specified contexts.
+     *
+     * @param approved_contextlist $contextlist The approved contexts to export information for.
+     */
+    public static function export_user_data(approved_contextlist $contextlist) {
+        global $DB;
+
+        if (empty($contextlist->count())) {
+            return;
+        }
+
+        $user = $contextlist->get_user();
+
+        list($contextsql, $contextparams) = $DB->get_in_or_equal($contextlist->get_contextids(), SQL_PARAMS_NAMED);
+
+        $sql = "SELECT ltiusers.lastgrade, ltiusers.lastaccess, ltiusers.timecreated, ltitools.contextid
+                  FROM {enrol_lti_users} ltiusers
+                  JOIN {enrol_lti_tools} ltitools
+                    ON ltiusers.toolid = ltitools.id
+                  JOIN {context} ctx
+                    ON ctx.id = ltitools.contextid
+                 WHERE ctx.id {$contextsql}
+                   AND ltiusers.userid = :userid";
+        $params = $contextparams + ['userid' => $user->id];
+        $ltiusers = $DB->get_recordset_sql($sql, $params);
+        self::recordset_loop_and_export($ltiusers, 'contextid', [], function($carry, $record) {
+            $carry[] = [
+                'lastgrade' => $record->lastgrade,
+                'timecreated' => transform::datetime($record->lastaccess),
+                'timemodified' => transform::datetime($record->timecreated)
+            ];
+            return $carry;
+        }, function($contextid, $data) {
+            $context = \context::instance_by_id($contextid);
+            $finaldata = (object) $data;
+            writer::with_context($context)->export_data(['enrol_lti_users'], $finaldata);
+        });
+    }
+
+    /**
+     * Delete all user data which matches the specified context.
+     *
+     * @param \context $context A user context.
+     */
+    public static function delete_data_for_all_users_in_context(\context $context) {
+        global $DB;
+
+        if (!($context instanceof \context_course || $context instanceof \context_module)) {
+            return;
+        }
+
+        $enrolltitools = $DB->get_fieldset_select('enrol_lti_tools', 'id', 'contextid = :contextid',
+            ['contextid' => $context->id]);
+        if (!empty($enrolltitools)) {
+            list($sql, $params) = $DB->get_in_or_equal($enrolltitools, SQL_PARAMS_NAMED);
+            $DB->delete_records_select('enrol_lti_users', 'toolid ' . $sql, $params);
+        }
+    }
+
+    /**
+     * Delete all user data for the specified user, in the specified contexts.
+     *
+     * @param approved_contextlist $contextlist The approved contexts and user information to delete information for.
+     */
+    public static function delete_data_for_user(approved_contextlist $contextlist) {
+        global $DB;
+
+        $userid = $contextlist->get_user()->id;
+
+        foreach ($contextlist->get_contexts() as $context) {
+            if (!($context instanceof \context_course || $context instanceof \context_module)) {
+                return;
+            }
+
+            $enrolltitools = $DB->get_fieldset_select('enrol_lti_tools', 'id', 'contextid = :contextid',
+                ['contextid' => $context->id]);
+            if (!empty($enrolltitools)) {
+                list($sql, $params) = $DB->get_in_or_equal($enrolltitools, SQL_PARAMS_NAMED);
+                $params = array_merge($params, ['userid' => $userid]);
+                $DB->delete_records_select('enrol_lti_users', "toolid $sql AND userid = :userid", $params);
+            }
+        }
+    }
+
+    /**
+     * Loop and export from a recordset.
+     *
+     * @param \moodle_recordset $recordset The recordset.
+     * @param string $splitkey The record key to determine when to export.
+     * @param mixed $initial The initial data to reduce from.
+     * @param callable $reducer The function to return the dataset, receives current dataset, and the current record.
+     * @param callable $export The function to export the dataset, receives the last value from $splitkey and the dataset.
+     * @return void
+     */
+    protected static function recordset_loop_and_export(\moodle_recordset $recordset, $splitkey, $initial,
+            callable $reducer, callable $export) {
+        $data = $initial;
+        $lastid = null;
+
+        foreach ($recordset as $record) {
+            if ($lastid && $record->{$splitkey} != $lastid) {
+                $export($lastid, $data);
+                $data = $initial;
+            }
+            $data = $reducer($data, $record);
+            $lastid = $record->{$splitkey};
+        }
+        $recordset->close();
+
+        if (!empty($lastid)) {
+            $export($lastid, $data);
+        }
+    }
+}
index f2db034..0de5dd1 100644 (file)
@@ -62,6 +62,11 @@ $string['lti:unenrol'] = 'Unenrol users from the course';
 $string['opentool'] = 'Open tool';
 $string['pluginname'] = 'Publish as LTI tool';
 $string['pluginname_desc'] = 'The \'Publish as LTI tool\' plugin, together with the LTI authentication plugin, allows remote users to access selected courses and activities. In other words, Moodle functions as an LTI tool provider.';
+$string['privacy:metadata:enrol_lti_users'] = 'The list of users enrolled via an LTI provider';
+$string['privacy:metadata:enrol_lti_users:userid'] = 'The ID of the user';
+$string['privacy:metadata:enrol_lti_users:lastgrade'] = 'The last grade the user was recorded of having';
+$string['privacy:metadata:enrol_lti_users:lastaccess'] = 'The date at which the user was enrolled';
+$string['privacy:metadata:enrol_lti_users:timecreated'] = 'The date at which the user was enrolled';
 $string['registration'] = 'Published tool registration';
 $string['registrationurl'] = 'Registration URL';
 $string['registrationurl_help'] = 'If a registration URL (also called proxy URL) is used, then the tool is automatically configured.';
diff --git a/enrol/lti/tests/privacy_provider_test.php b/enrol/lti/tests/privacy_provider_test.php
new file mode 100644 (file)
index 0000000..034b683
--- /dev/null
@@ -0,0 +1,202 @@
+<?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/>.
+
+/**
+ * Privacy provider tests.
+ *
+ * @package    enrol_lti
+ * @copyright  2018 Mark Nelson <markn@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+use enrol_lti\privacy\provider;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Privacy provider tests class.
+ *
+ * @package    enrol_lti
+ * @copyright  2018 Mark Nelson <markn@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class enrol_lti_privacy_provider_testcase extends \core_privacy\tests\provider_testcase {
+
+    /**
+     * @var stdClass The user
+     */
+    protected $user = null;
+
+    /**
+     * @var stdClass The course
+     */
+    protected $course = null;
+
+    /**
+     * @var stdClass The activity
+     */
+    protected $activity = null;
+
+    /**
+     * Basic setup for these tests.
+     */
+    public function setUp() {
+        $this->resetAfterTest();
+
+        $this->course = $this->getDataGenerator()->create_course();
+        $this->user = $this->getDataGenerator()->create_user();
+        $this->activity = $this->getDataGenerator()->create_module('forum', ['course' => $this->course->id]);
+
+        // Get the course and activity contexts.
+        $coursecontext = \context_course::instance($this->course->id);
+        $cmcontext = \context_module::instance($this->activity->cmid);
+
+        // Create LTI tools in different contexts.
+        $this->create_lti_users($coursecontext, $this->user->id);
+        $this->create_lti_users($coursecontext, $this->user->id);
+        $this->create_lti_users($cmcontext, $this->user->id);
+
+        // Create another LTI user.
+        $user = $this->getDataGenerator()->create_user();
+        $this->create_lti_users($coursecontext, $user->id);
+    }
+
+    /**
+     * Test getting the context for the user ID related to this plugin.
+     */
+    public function test_get_contexts_for_userid() {
+        $contextlist = provider::get_contexts_for_userid($this->user->id);
+
+        $this->assertCount(2, $contextlist);
+
+        $contextforcourse = $contextlist->current();
+        $context = context_course::instance($this->course->id);
+        $this->assertEquals($context->id, $contextforcourse->id);
+
+        $contextlist->next();
+        $contextforactivity = $contextlist->current();
+        $context = context_module::instance($this->activity->cmid);
+        $this->assertEquals($context->id, $contextforactivity->id);
+    }
+
+    /**
+     * Test for provider::export_user_data().
+     */
+    public function test_export_for_context() {
+        $coursecontext = context_course::instance($this->course->id);
+        $cmcontext = \context_module::instance($this->activity->cmid);
+
+        // Export all of the data for the course context.
+        $this->export_context_data_for_user($this->user->id, $coursecontext, 'enrol_lti');
+        $writer = \core_privacy\local\request\writer::with_context($coursecontext);
+        $this->assertTrue($writer->has_any_data());
+
+        $data = (array) $writer->get_data(['enrol_lti_users']);
+        $this->assertCount(2, $data);
+        foreach ($data as $ltiuser) {
+            $this->assertArrayHasKey('lastgrade', $ltiuser);
+            $this->assertArrayHasKey('timecreated', $ltiuser);
+            $this->assertArrayHasKey('timemodified', $ltiuser);
+        }
+
+        // Export all of the data for the activity context.
+        $this->export_context_data_for_user($this->user->id, $cmcontext, 'enrol_lti');
+        $writer = \core_privacy\local\request\writer::with_context($cmcontext);
+        $this->assertTrue($writer->has_any_data());
+
+        $data = (array) $writer->get_data(['enrol_lti_users']);
+        $this->assertCount(1, $data);
+        foreach ($data as $ltiuser) {
+            $this->assertArrayHasKey('lastgrade', $ltiuser);
+            $this->assertArrayHasKey('timecreated', $ltiuser);
+            $this->assertArrayHasKey('timemodified', $ltiuser);
+        }
+    }
+
+    /**
+     * Test for provider::delete_data_for_all_users_in_context().
+     */
+    public function test_delete_data_for_all_users_in_context() {
+        global $DB;
+
+        $count = $DB->count_records('enrol_lti_users');
+        $this->assertEquals(4, $count);
+
+        // Delete data based on context.
+        $coursecontext = context_course::instance($this->course->id);
+        provider::delete_data_for_all_users_in_context($coursecontext);
+
+        $ltiusers = $DB->get_records('enrol_lti_users');
+        $this->assertCount(1, $ltiusers);
+
+        $ltiuser = reset($ltiusers);
+        $this->assertEquals($ltiuser->userid, $this->user->id);
+    }
+
+    /**
+     * Test for provider::delete_data_for_user().
+     */
+    public function test_delete_data_for_user() {
+        global $DB;
+
+        $cmcontext = context_module::instance($this->activity->cmid);
+        $coursecontext = context_course::instance($this->course->id);
+
+        $count = $DB->count_records('enrol_lti_users');
+        $this->assertEquals(4, $count);
+
+        $contextlist = new \core_privacy\local\request\approved_contextlist($this->user, 'core_backup',
+            [$coursecontext->id, $cmcontext->id]);
+        provider::delete_data_for_user($contextlist);
+
+        $ltiusers = $DB->get_records('enrol_lti_users');
+        $this->assertCount(1, $ltiusers);
+
+        $ltiuser = reset($ltiusers);
+        $this->assertNotEquals($ltiuser->userid, $this->user->id);
+    }
+
+    /**
+     * Creates a LTI user given the provided context
+     *
+     * @param context $context
+     * @param int $userid
+     */
+    private function create_lti_users(\context $context, int $userid) {
+        global $DB;
+
+        // Create a tool.
+        $ltitool = (object) [
+            'enrolid' => 5,
+            'contextid' => $context->id,
+            'roleinstructor' => 5,
+            'rolelearner' => 5,
+            'timecreated' => time(),
+            'timemodified' => time() + DAYSECS
+        ];
+        $toolid = $DB->insert_record('enrol_lti_tools', $ltitool);
+
+        // Create a user.
+        $ltiuser = (object) [
+            'userid' => $userid,
+            'toolid' => $toolid,
+            'lastgrade' => 50,
+            'lastaccess' => time() + DAYSECS,
+            'timecreated' => time()
+        ];
+        $DB->insert_record('enrol_lti_users', $ltiuser);
+    }
+}
index 4976504..282253a 100644 (file)
@@ -127,18 +127,21 @@ class provider implements
         $sql = "
             SELECT DISTINCT ctx.id
               FROM {context} ctx
-         LEFT JOIN {grade_outcomes_history} goh
-                ON (goh.courseid > 0 AND goh.courseid = ctx.instanceid AND ctx.contextlevel = :courselevel1)
+         LEFT JOIN {grade_outcomes_history} goh ON goh.loggeduser = :userid1 AND (
+                   (goh.courseid > 0 AND goh.courseid = ctx.instanceid AND ctx.contextlevel = :courselevel1)
                 OR ((goh.courseid IS NULL OR goh.courseid < 1) AND ctx.id = :syscontextid)
-         LEFT JOIN {grade_categories_history} gch
-                ON gch.courseid = ctx.instanceid
+            )
+         LEFT JOIN {grade_categories_history} gch ON gch.loggeduser = :userid2 AND (
+                   gch.courseid = ctx.instanceid
                AND ctx.contextlevel = :courselevel2
-         LEFT JOIN {grade_items_history} gih
-                ON gih.courseid = ctx.instanceid
+            )
+         LEFT JOIN {grade_items_history} gih ON gih.loggeduser = :userid3 AND (
+                   gih.courseid = ctx.instanceid
                AND ctx.contextlevel = :courselevel3
-             WHERE goh.loggeduser = :userid1
-                OR gch.loggeduser = :userid2
-                OR gih.loggeduser = :userid3";
+            )
+             WHERE goh.id IS NOT NULL
+                OR gch.id IS NOT NULL
+                OR gih.id IS NOT NULL";
         $params = [
             'syscontextid' => SYSCONTEXTID,
             'courselevel1' => CONTEXT_COURSE,
@@ -159,13 +162,16 @@ class provider implements
                AND ctx.contextlevel = :courselevel
          LEFT JOIN {grade_grades} gg
                 ON gg.itemid = gi.id
+               AND (gg.userid = :userid1 OR gg.usermodified = :userid2)
          LEFT JOIN {grade_grades_history} ggh
                 ON ggh.itemid = gi.id
-             WHERE gg.userid = :userid1
-                OR gg.usermodified = :userid2
-                OR ggh.userid = :userid3
+               AND (
+                   ggh.userid = :userid3
                 OR ggh.loggeduser = :userid4
-                OR ggh.usermodified = :userid5";
+                OR ggh.usermodified = :userid5
+            )
+             WHERE gg.id IS NOT NULL
+                OR ggh.id IS NOT NULL";
         $params = [
             'courselevel' => CONTEXT_COURSE,
             'userid1' => $userid,
@@ -184,14 +190,14 @@ class provider implements
              JOIN {grade_grades_history} ggh
                ON ctx.contextlevel = :userlevel
               AND ggh.userid = ctx.instanceid
-        LEFT JOIN {grade_items} gi
-               ON ggh.itemid = gi.id
-            WHERE gi.id IS NULL
               AND (
                   ggh.userid = :userid1
                OR ggh.usermodified = :userid2
                OR ggh.loggeduser = :userid3
-                  )";
+              )
+        LEFT JOIN {grade_items} gi
+               ON ggh.itemid = gi.id
+            WHERE gi.id IS NULL";
         $params = [
             'userlevel' => CONTEXT_USER,
             'userid1' => $userid,
index a0f4494..d5ec198 100644 (file)
@@ -37,6 +37,7 @@ use \core_privacy\local\request\writer;
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class provider implements
+        \core_privacy\local\metadata\provider,
         \core_privacy\local\request\user_preference_provider {
 
     /**
index 073daac..f20c57c 100644 (file)
  */
 
 $string['privacy:metadata'] = 'The privacy subsystem does not store any data of its own and is designed to act as a channel between components and the interface used to describe, export, and remove their data.';
+$string['trace:done'] = 'Complete';
+$string['trace:exportcomplete'] = 'Export complete';
+$string['trace:exportingapproved'] = 'Performing primary export for {$a->total} components ({$a->datetime})';
+$string['trace:exportingrelated'] = 'Performing related export for {$a->total} components ({$a->datetime})';
+$string['trace:finalisingexport'] = 'Finalising export';
+$string['trace:processingcomponent'] = 'Processing {$a->component} ({$a->progress}/{$a->total}) ({$a->datetime})';
+$string['trace:fetchcomponents'] = 'Fetching {$a->total} components ({$a->datetime})';
+$string['trace:deletingapproved'] = 'Performing removal of approved {$a->total} contexts ({$a->datetime})';
+$string['trace:deletingcontext'] = 'Performing removal of context from {$a->total} components ({$a->datetime})';
index 7166872..ac3359c 100644 (file)
@@ -110,7 +110,7 @@ class file_temp_cleanup_task extends scheduled_task {
         $this->execute_on($CFG->tempdir);
 
         // Run on $CFG->backuptempdir too, if different from the default one, '$CFG->tempdir/backup'.
-        if (realpath(dirname($CFG->backuptempdir)) !== $CFG->tempdir) {
+        if (realpath(dirname($CFG->backuptempdir)) !== realpath($CFG->tempdir)) {
             // The $CFG->backuptempdir setting is different from the default '$CFG->tempdir/backup'.
             $this->execute_on($CFG->backuptempdir);
         }
index ffb71fb..e2b5c78 100644 (file)
@@ -12,7 +12,7 @@ Feature: Atto accessibility checker
     Then I should see "Images require alternative text."
     And I follow "/broken-image"
     And I wait "2" seconds
-    And I click on "Image" "button"
+    And I click on "Insert or edit image" "button"
     And the field "Enter URL" matches value "/broken-image"
     And I set the field "Describe this image for someone who cannot see it" to "No more warning!"
     And I press "Save image"
@@ -20,7 +20,7 @@ Feature: Atto accessibility checker
     And I should see "Congratulations, no accessibility problems found!"
     And I click on ".moodle-dialogue-focused .closebutton" "css_element"
     And I select the text in the "Description" Atto editor
-    And I click on "Image" "button"
+    And I click on "Insert or edit image" "button"
     And I set the field "Describe this image for someone who cannot see it" to ""
     And I set the field "Description not necessary" to "1"
     And I press "Save image"
index e66929e..941acdf 100644 (file)
@@ -11,7 +11,7 @@ Feature: Add images to Atto
     And I open my profile in edit mode
     When I set the field "Description" to "<p>Image test</p>"
     And I select the text in the "Description" Atto editor
-    And I click on "Image" "button"
+    And I click on "Insert or edit image" "button"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle-logo.png" "link"
@@ -43,7 +43,7 @@ Feature: Add images to Atto
     And I click on "Update profile" "button"
     And I click on "Edit profile" "link" in the "region-main" "region"
     And I select the text in the "Description" Atto editor
-    And I click on "Image" "button"
+    And I click on "Insert or edit image" "button"
     Then the field "Describe this image for someone who cannot see it" matches value "It's the Moodle"
     And the field "Width" matches value "123"
     And the field "Height" matches value "456"
@@ -54,7 +54,7 @@ Feature: Add images to Atto
     And I open my profile in edit mode
     And I set the field "Description" to "<p>Image: <img src='/nothing/here'>.</p>"
     And I select the text in the "Description" Atto editor
-    When I click on "Image" "button"
+    When I click on "Insert or edit image" "button"
     Then the field "Enter URL" matches value "/nothing/here"
     And I set the field "Describe this image for someone who cannot see it" to "Something"
     And I set the field "Enter URL" to ""
@@ -63,7 +63,7 @@ Feature: Add images to Atto
     And I press "Update profile"
     And I click on "Edit profile" "link" in the "region-main" "region"
     And I select the text in the "Description" Atto editor
-    And I click on "Image" "button"
+    And I click on "Insert or edit image" "button"
     And the field "Enter URL" matches value "/nothing/again"
     And the field "Width" matches value "123"
     And the field "Height" matches value "456"
index 03cc0de..d6e42d3 100644 (file)
@@ -17,7 +17,7 @@ Feature: Add media to Atto
     And I set the field "Blog entry body" to "<p>Media test</p>"
     And I select the text in the "Blog entry body" Atto editor
     And I set the field "Entry title" to "The best video in the entire world (not really)"
-    And I click on "Media" "button"
+    And I click on "Insert or edit an audio/video file" "button"
 
   @javascript
   Scenario: Insert some media as a link
index a7290d9..3281ba6 100644 (file)
@@ -58,7 +58,7 @@ Feature: Atto Autosave
     And I navigate to "Edit settings" node in "Course administration"
     And I set the field "Course summary" to "<p>Image test</p>"
     And I select the text in the "Course summary" Atto editor
-    And I click on "Image" "button"
+    And I click on "Insert or edit image" "button"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle-logo.png" "link"
index 8db487a..4c4deef 100644 (file)
@@ -171,8 +171,8 @@ class cronlib_testcase extends basic_testcase {
             $isvalid = true;
             $isvalid = $isvalid && !$iter->isDot();
             // Remove the default $CFG->tempdir/backup directory and $CFG->tempdir/.htaccess file from this comparison.
-            $isvalid = $isvalid && !($iter->isDir() && ($iter->getRealpath() === "{$tmpdir}/backup"));
-            $isvalid = $isvalid && !($iter->isFile() && ($iter->getRealpath() === "{$tmpdir}/.htaccess"));
+            $isvalid = $isvalid && !($iter->isDir() && ($iter->getRealpath() === $tmpdir . DIRECTORY_SEPARATOR . 'backup'));
+            $isvalid = $isvalid && !($iter->isFile() && ($iter->getRealpath() === $tmpdir . DIRECTORY_SEPARATOR . '.htaccess'));
             if ($isvalid) {
                 $actual[] = $iter->getRealPath();
             }
index d98f529..39e549f 100644 (file)
@@ -210,6 +210,7 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide
         $writer = writer::with_context(\context_system::instance());
 
         $contacts = (array) $writer->get_data([get_string('contacts', 'core_message')]);
+        usort($contacts, ['static', 'sort_contacts']);
 
         $this->assertCount(3, $contacts);
 
@@ -274,6 +275,7 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide
         $dbm2 = $DB->get_record('messages', ['id' => $m2]);
         $dbm3 = $DB->get_record('messages', ['id' => $m3]);
 
+        usort($messages, ['static', 'sort_messages']);
         $m1 = array_shift($messages);
         $m2 = array_shift($messages);
         $m3 = array_shift($messages);
@@ -303,6 +305,7 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide
         $dbm5 = $DB->get_record('messages', ['id' => $m5]);
         $dbm6 = $DB->get_record('messages', ['id' => $m6]);
 
+        usort($messages, ['static', 'sort_messages']);
         $m4 = array_shift($messages);
         $m5 = array_shift($messages);
         $m6 = array_shift($messages);
@@ -584,4 +587,26 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide
 
         return $DB->insert_record('notifications', $record);
     }
+
+    /**
+     * Comparison function for sorting messages.
+     *
+     * @param   \stdClass $a
+     * @param   \stdClass $b
+     * @return  bool
+     */
+    protected static function sort_messages($a, $b) {
+        return $a->message > $b->message;
+    }
+
+    /**
+     * Comparison function for sorting contacts.
+     *
+     * @param   \stdClass $a
+     * @param   \stdClass $b
+     * @return  bool
+     */
+    protected static function sort_contacts($a, $b) {
+        return $a->contact > $b->contact;
+    }
 }
index c706711..a99868f 100644 (file)
@@ -139,15 +139,53 @@ class provider implements metadataprovider, pluginprovider, preference_provider
                   JOIN {modules} m ON cm.module = m.id AND m.name = :modulename
                   JOIN {assign} a ON cm.instance = a.id
                   JOIN {context} ctx ON cm.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
-             LEFT JOIN {assign_grades} ag ON a.id = ag.assignment
-             LEFT JOIN {assign_overrides} ao ON a.id = ao.assignid
-             LEFT JOIN {assign_submission} asn ON a.id = asn.assignment
-             LEFT JOIN {assign_user_flags} auf ON a.id = auf.assignment
-             LEFT JOIN {assign_user_mapping} aum ON a.id = aum.assignment
-                 WHERE ag.userid = :userid OR ag.grader = :graderid OR ao.userid = :aouserid
-                       OR asn.userid = :asnuserid OR auf.userid = :aufuserid OR aum.userid = :aumuserid";
+                  JOIN {assign_grades} ag ON a.id = ag.assignment AND (ag.userid = :userid OR ag.grader = :graderid)";
+
+                  global $DB;
+
         $contextlist = new contextlist();
         $contextlist->add_from_sql($sql, $params);
+
+        $sql = "SELECT ctx.id
+                  FROM {course_modules} cm
+                  JOIN {modules} m ON cm.module = m.id AND m.name = :modulename
+                  JOIN {assign} a ON cm.instance = a.id
+                  JOIN {context} ctx ON cm.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
+                  JOIN {assign_overrides} ao ON a.id = ao.assignid
+                 WHERE ao.userid = :aouserid";
+
+        $contextlist->add_from_sql($sql, $params);
+
+        $sql = "SELECT ctx.id
+                  FROM {course_modules} cm
+                  JOIN {modules} m ON cm.module = m.id AND m.name = :modulename
+                  JOIN {assign} a ON cm.instance = a.id
+                  JOIN {context} ctx ON cm.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
+                  JOIN {assign_submission} asn ON a.id = asn.assignment
+                 WHERE asn.userid = :asnuserid";
+
+        $contextlist->add_from_sql($sql, $params);
+
+        $sql = "SELECT ctx.id
+                  FROM {course_modules} cm
+                  JOIN {modules} m ON cm.module = m.id AND m.name = :modulename
+                  JOIN {assign} a ON cm.instance = a.id
+                  JOIN {context} ctx ON cm.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
+                  JOIN {assign_user_flags} auf ON a.id = auf.assignment
+                 WHERE auf.userid = :aufuserid";
+
+        $contextlist->add_from_sql($sql, $params);
+
+        $sql = "SELECT ctx.id
+                  FROM {course_modules} cm
+                  JOIN {modules} m ON cm.module = m.id AND m.name = :modulename
+                  JOIN {assign} a ON cm.instance = a.id
+                  JOIN {context} ctx ON cm.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
+                  JOIN {assign_user_mapping} aum ON a.id = aum.assignment
+                 WHERE aum.userid = :aumuserid";
+
+        $contextlist->add_from_sql($sql, $params);
+
         manager::plugintype_class_callback('assignfeedback', self::ASSIGNFEEDBACK_INTERFACE,
                 'get_context_for_userid_within_feedback', [$userid, $contextlist]);
         manager::plugintype_class_callback('assignsubmission', self::ASSIGNSUBMISSION_INTERFACE,
index e07665b..b2c36a8 100644 (file)
@@ -105,9 +105,10 @@ class provider implements
             INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname
             INNER JOIN {data} d ON d.id = cm.instance
             INNER JOIN {data_records} dr ON dr.dataid = d.id
-            LEFT JOIN {comments} com ON com.commentarea=:commentarea and com.itemid = dr.id
-            LEFT JOIN {rating} r ON r.contextid = c.id AND r.itemid  = dr.id AND r.component = :moddata AND r.ratingarea = :ratingarea
-                 WHERE dr.userid = :userid OR com.userid = :userid1 OR r.userid = :userid2";
+             LEFT JOIN {comments} com ON com.commentarea=:commentarea and com.itemid = dr.id AND com.userid = :userid1
+             LEFT JOIN {rating} r ON r.contextid = c.id AND r.itemid  = dr.id AND r.component = :moddata
+                       AND r.ratingarea = :ratingarea AND r.userid = :userid2
+                 WHERE dr.userid = :userid OR com.id IS NOT NULL OR r.id IS NOT NULL";
 
         $params = [
             'modname'       => 'data',
index a2b0706..afb6417 100644 (file)
@@ -133,19 +133,24 @@ class provider implements
                AND ctx.contextlevel = :modulelevel
          LEFT JOIN {lesson_attempts} la
                 ON la.lessonid = l.id
+               AND la.userid = :userid1
          LEFT JOIN {lesson_branch} lb
                 ON lb.lessonid = l.id
+               AND lb.userid = :userid2
          LEFT JOIN {lesson_grades} lg
                 ON lg.lessonid = l.id
+               AND lg.userid = :userid3
          LEFT JOIN {lesson_overrides} lo
                 ON lo.lessonid = l.id
+               AND lo.userid = :userid4
          LEFT JOIN {lesson_timer} lt
                 ON lt.lessonid = l.id
-             WHERE la.userid = :userid1
-                OR lb.userid = :userid2
-                OR lg.userid = :userid3
-                OR lt.userid = :userid4
-                OR lo.userid = :userid5";
+               AND lt.userid = :userid5
+             WHERE la.id IS NOT NULL
+                OR lb.id IS NOT NULL
+                OR lg.id IS NOT NULL
+                OR lo.id IS NOT NULL
+                OR lt.id IS NOT NULL";
 
         $params = [
             'lesson' => 'lesson',
index 3cbc37b..103a371 100644 (file)
@@ -38,7 +38,7 @@ Feature: In a lesson activity, a teacher can duplicate a lesson page
       | id_jumpto_1 | Previous page |
     # Atto needs focus to add image, select empty p tag to do so.
     And I select the text in the "id_contents_editor" Atto editor
-    And I click on "Image" "button" in the "[data-fieldtype=editor]" "css_element"
+    And I click on "Insert or edit image" "button" in the "[data-fieldtype=editor]" "css_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle_logo.jpg" "link"
@@ -74,7 +74,7 @@ Feature: In a lesson activity, a teacher can duplicate a lesson page
       | id_score_1 | 0 |
     # Atto needs focus to add image, select empty p tag to do so.
     And I select the text in the "id_answer_editor_0" Atto editor
-    And I click on "Image" "button" in the "//*[@id='id_answer_editor_0']/ancestor::*[@data-fieldtype='editor']" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@id='id_answer_editor_0']/ancestor::*[@data-fieldtype='editor']" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle_logo.jpg" "link"
@@ -111,7 +111,7 @@ Feature: In a lesson activity, a teacher can duplicate a lesson page
       | id_score_1 | 0 |
     # Atto needs focus to add image, select empty p tag to do so.
     And I select the text in the "id_response_editor_0" Atto editor
-    And I click on "Image" "button" in the "//*[@id='id_response_editor_0']/ancestor::*[@data-fieldtype='editor']" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@id='id_response_editor_0']/ancestor::*[@data-fieldtype='editor']" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle_logo.jpg" "link"
index 0dc725c..9659dac 100644 (file)
@@ -45,7 +45,7 @@ Feature: In a lesson activity, teacher can add embedded images in questions answ
       | id_score_2 | 0 |
     # Atto needs focus to add image, select empty p tag to do so.
     And I select the text in the "id_answer_editor_2" Atto editor
-    And I click on "Image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_answer_editor_2']]" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_answer_editor_2']]" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle_logo.jpg" "link"
@@ -67,7 +67,7 @@ Feature: In a lesson activity, teacher can add embedded images in questions answ
       | id_jumpto_1 | This page |
     # Atto needs focus to add image, select empty p tag to do so.
     And I select the text in the "id_response_editor_0" Atto editor
-    And I click on "Image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_response_editor_0']]" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_response_editor_0']]" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle_logo.jpg" "link"
index 92f4b30..e3361cb 100644 (file)
@@ -148,7 +148,8 @@ class provider implements
             " . $qubaid->from . "
             WHERE (
                 qa.userid = :qauserid OR
-                " . $qubaid->where() . "
+                " . $qubaid->where() . " OR
+                qo.id IS NOT NULL
             ) AND qa.preview = 0
         ";
 
index 5ae87fe..e361b2a 100644 (file)
@@ -73,7 +73,7 @@ Feature: Teachers can override the grade for any question
     And I set the field "Comment" to "Administrator's comment"
     # Atto needs focus to add image, select empty p tag to do so.
     And I select the text in the "Comment" Atto editor
-    And I click on "Image" "button" in the "[data-fieldtype=editor]" "css_element"
+    And I click on "Insert or edit image" "button" in the "[data-fieldtype=editor]" "css_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodle_logo.jpg" "link"
index 4ec1c45..44eb15b 100644 (file)
@@ -29,7 +29,7 @@ Feature: Teachers can embed images into instructions and conclusion fields
     And I expand all fieldsets
     And I set the field "Instructions for submission" to "<p>Image test</p>"
     And I select the text in the "Instructions for submission" Atto editor
-    And I click on "Image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_instructauthorseditor']]" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_instructauthorseditor']]" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodlelogo.png" "link"
@@ -42,7 +42,7 @@ Feature: Teachers can embed images into instructions and conclusion fields
     And I expand all fieldsets
     And I set the field "Instructions for assessment" to "<p>Image test</p>"
     And I select the text in the "Instructions for assessment" Atto editor
-    And I click on "Image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_instructreviewerseditor']]" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_instructreviewerseditor']]" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodlelogo.png" "link"
@@ -55,7 +55,7 @@ Feature: Teachers can embed images into instructions and conclusion fields
     And I expand all fieldsets
     And I set the field "Conclusion" to "<p>Image test</p>"
     And I select the text in the "Conclusion" Atto editor
-    And I click on "Image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_conclusioneditor']]" "xpath_element"
+    And I click on "Insert or edit image" "button" in the "//*[@data-fieldtype='editor']/*[descendant::*[@id='id_conclusioneditor']]" "xpath_element"
     And I click on "Browse repositories..." "button"
     And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
     And I click on "moodlelogo.png" "link"
index 088476d..5ced07d 100644 (file)
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- Generator: Adobe Illustrator 21.1.0, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
 <svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
-        viewBox="0 0 16 16" style="enable-background:new 0 0 16 16;" xml:space="preserve">
+        viewBox="0 0 16 16" style="enable-background:new 0 0 16 16;" xml:space="preserve" preserveAspectRatio="xMinYMid meet">
 <style type="text/css">
        .st0{fill:#999999;}
 </style>
index fb0a82f..354bff8 100644 (file)
@@ -36,6 +36,7 @@ use core_privacy\local\metadata\collection;
 class provider implements
     // This portfolio plugin does not store any data itself.
     // It has no database tables, and it purely acts as a conduit, sending data externally.
+    \core_privacy\local\metadata\provider,
     \core_portfolio\privacy\portfolio_provider {
 
     /**
index f1b96c9..090b1eb 100644 (file)
@@ -36,6 +36,7 @@ use core_privacy\local\metadata\collection;
 class provider implements
         // This portfolio plugin does not store any data itself.
         // It has no database tables, and it purely acts as a conduit, sending data externally.
+        \core_privacy\local\metadata\provider,
         \core_portfolio\privacy\portfolio_provider {
 
     /**
index f8734af..9fe739a 100644 (file)
@@ -36,6 +36,7 @@ use core_privacy\local\metadata\collection;
 class provider implements
         // This portfolio plugin does not store any data itself.
         // It has no database tables, and it purely acts as a conduit, sending data externally.
+        \core_privacy\local\metadata\provider,
         \core_portfolio\privacy\portfolio_provider {
 
     /**
index be1719e..a8731be 100644 (file)
@@ -36,6 +36,7 @@ use core_privacy\local\metadata\collection;
 class provider implements
         // This portfolio plugin does not store any data itself.
         // It has no database tables, and it purely acts as a conduit, sending data externally.
+        \core_privacy\local\metadata\provider,
         \core_portfolio\privacy\portfolio_provider {
 
     /**
index 79e0338..4f2fafe 100644 (file)
@@ -36,6 +36,7 @@ use core_privacy\local\metadata\collection;
 class provider implements
         // This portfolio plugin does not store any data itself.
         // It has no database tables, and it purely acts as a conduit, sending data externally.
+        \core_privacy\local\metadata\provider,
         \core_portfolio\privacy\portfolio_provider {
 
     /**
index 2e82e6a..bf25a06 100644 (file)
@@ -162,7 +162,7 @@ class moodle_content_writer implements content_writer {
      * @return  string                      The processed string
      */
     public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text) : string {
-        return str_replace('@@PLUGINFILE@@/', $this->get_files_target_path($component, $filearea, $itemid).'/', $text);
+        return str_replace('@@PLUGINFILE@@/', $this->get_files_target_url($component, $filearea, $itemid).'/', $text);
     }
 
     /**
@@ -272,7 +272,9 @@ class moodle_content_writer implements content_writer {
         // Join the directory together with the name.
         $filepath = implode(DIRECTORY_SEPARATOR, $path) . DIRECTORY_SEPARATOR . $name;
 
-        return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath);
+        // To use backslash, it must be doubled ("\\\\" PHP string).
+        $separator = str_replace('\\', '\\\\', DIRECTORY_SEPARATOR);
+        return preg_replace('@(' . $separator . '|/)+@', $separator, $filepath);
     }
 
     /**
@@ -291,7 +293,9 @@ class moodle_content_writer implements content_writer {
         // Join the directory together with the name.
         $filepath = implode(DIRECTORY_SEPARATOR, $path);
 
-        return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath);
+        // To use backslash, it must be doubled ("\\\\" PHP string).
+        $separator = str_replace('\\', '\\\\', DIRECTORY_SEPARATOR);
+        return preg_replace('@(' . $separator . '|/)+@', $separator, $filepath);
     }
 
     /**
@@ -314,6 +318,25 @@ class moodle_content_writer implements content_writer {
         return implode(DIRECTORY_SEPARATOR, $parts);
     }
 
+    /**
+     * Get a relative url to the directory of the exported files within a subcontext.
+     *
+     * @param string $component The name of the component that the files belong to.
+     * @param string $filearea The filearea within that component.
+     * @param string $itemid Which item those files belong to.
+     * @return string The url
+     */
+    protected function get_files_target_url($component, $filearea, $itemid) : string {
+        // We do not need to include the component because we organise things by context.
+        $parts = ['_files', $filearea];
+
+        if (!empty($itemid)) {
+            $parts[] = $itemid;
+        }
+
+        return implode('/', $parts);
+    }
+
     /**
      * Write the data to the specified path.
      *
index 0cd66f2..69f9667 100644 (file)
@@ -168,8 +168,23 @@ class manager {
      * @return contextlist_collection the collection of contextlist items for the respective components.
      */
     public function get_contexts_for_userid(int $userid) : contextlist_collection {
+        $progress = static::get_log_tracer();
+
+        $components = $this->get_component_list();
+        $a = (object) [
+            'total' => count($components),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
         $clcollection = new contextlist_collection($userid);
-        foreach ($this->get_component_list() as $component) {
+
+        $progress->output(get_string('trace:fetchcomponents', 'core_privacy', $a), 1);
+        foreach ($components as $component) {
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
             if ($this->component_implements($component, \core_privacy\local\request\core_user_data_provider::class)) {
                 $contextlist = $this->get_provider_classname($component)::get_contexts_for_userid($userid);
             } else {
@@ -187,6 +202,7 @@ class manager {
                 $clcollection->add_contextlist($contextlist);
             }
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
 
         return $clcollection;
     }
@@ -202,13 +218,29 @@ class manager {
      * approved_contextlists' components is not a core_data_provider.
      */
     public function export_user_data(contextlist_collection $contextlistcollection) {
+        $progress = static::get_log_tracer();
+
+        $a = (object) [
+            'total' => count($contextlistcollection),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
+
         // Export for the various components/contexts.
+        $progress->output(get_string('trace:exportingapproved', 'core_privacy', $a), 1);
         foreach ($contextlistcollection as $approvedcontextlist) {
+
             if (!$approvedcontextlist instanceof \core_privacy\local\request\approved_contextlist) {
                 throw new \moodle_exception('Contextlist must be an approved_contextlist');
             }
 
             $component = $approvedcontextlist->get_component();
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
+
             // Core user data providers.
             if ($this->component_implements($component, \core_privacy\local\request\core_user_data_provider::class)) {
                 if (count($approvedcontextlist)) {
@@ -221,9 +253,19 @@ class manager {
                 local\request\helper::export_data_for_null_provider($approvedcontextlist);
             }
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
 
         // Check each component for non contextlist items too.
-        foreach ($this->get_component_list() as $component) {
+        $components = $this->get_component_list();
+        $a->total = count($components);
+        $a->progress = 0;
+        $a->datetime = userdate(time());
+        $progress->output(get_string('trace:exportingrelated', 'core_privacy', $a), 1);
+        foreach ($components as $component) {
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
             // Core user preference providers.
             if ($this->component_implements($component, \core_privacy\local\request\user_preference_provider::class)) {
                 $this->get_provider_classname($component)::export_user_preferences($contextlistcollection->get_userid());
@@ -235,8 +277,13 @@ class manager {
                 $this->get_provider_classname($component)::export_context_data($contextlistcollection);
             }
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
+
+        $progress->output(get_string('trace:finalisingexport', 'core_privacy'), 1);
+        $location = local\request\writer::with_context(\context_system::instance())->finalise_content();
 
-        return local\request\writer::with_context(\context_system::instance())->finalise_content();
+        $progress->output(get_string('trace:exportcomplete', 'core_privacy'), 1);
+        return $location;
     }
 
     /**
@@ -251,13 +298,29 @@ class manager {
      * for an approved_contextlist isn't a core provider.
      */
     public function delete_data_for_user(contextlist_collection $contextlistcollection) {
+        $progress = static::get_log_tracer();
+
+        $a = (object) [
+            'total' => count($contextlistcollection),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
+
         // Delete the data.
+        $progress->output(get_string('trace:deletingapproved', 'core_privacy', $a), 1);
         foreach ($contextlistcollection as $approvedcontextlist) {
             if (!$approvedcontextlist instanceof \core_privacy\local\request\approved_contextlist) {
                 throw new \moodle_exception('Contextlist must be an approved_contextlist');
             }
 
-            if ($this->component_is_core_provider($approvedcontextlist->get_component())) {
+            $component = $approvedcontextlist->get_component();
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
+
+            if ($this->component_is_core_provider($component)) {
                 if (count($approvedcontextlist)) {
                     // The component knows about data that it has.
                     // Have it delete its own data.
@@ -268,6 +331,7 @@ class manager {
             // Delete any shared user data it doesn't know about.
             local\request\helper::delete_data_for_user($approvedcontextlist);
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
     }
 
     /**
@@ -276,7 +340,23 @@ class manager {
      * @param   context         $context   The specific context to delete data for.
      */
     public function delete_data_for_all_users_in_context(\context $context) {
+        $progress = static::get_log_tracer();
+
+        $components = $this->get_component_list();
+        $a = (object) [
+            'total' => count($components),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
+
+        $progress->output(get_string('trace:deletingcontext', 'core_privacy', $a), 1);
         foreach ($this->get_component_list() as $component) {
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
+
             if ($this->component_implements($component, \core_privacy\local\request\core_user_data_provider::class)) {
                 // This component knows about specific data that it owns.
                 // Have it delete all of that user data for the context.
@@ -286,6 +366,7 @@ class manager {
             // Delete any shared user data it doesn't know about.
             local\request\helper::delete_data_for_all_users_in_context($component, $context);
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
     }
 
     /**
@@ -378,4 +459,19 @@ class manager {
 
         return null;
     }
+
+    /**
+     * Get the tracer used for logging.
+     *
+     * The text tracer is used except for unit tests.
+     *
+     * @return  \progress_trace
+     */
+    protected static function get_log_tracer() {
+        if (PHPUNIT_TEST) {
+            return new \null_progress_trace();
+        }
+
+        return new \text_progress_trace();
+    }
 }
index d46ae39..93c7026 100644 (file)
@@ -411,7 +411,10 @@ class content_writer implements \core_privacy\local\request\content_writer {
      */
     public function export_file(array $subcontext, \stored_file $file) : \core_privacy\local\request\content_writer  {
         if (!$file->is_directory()) {
-            $filepath = explode(DIRECTORY_SEPARATOR, $file->get_filepath());
+            $filepath = $file->get_filepath();
+            // Directory separator in the stored_file class should always be '/'. The following line is just a fail safe.
+            $filepath = str_replace(DIRECTORY_SEPARATOR, '/', $filepath);
+            $filepath = explode('/', $filepath);
             $filepath[] = $file->get_filename();
             $filepath = array_filter($filepath);
             $filepath = implode('/', $filepath);
index 7e81a3d..f4ccc3b 100644 (file)
@@ -1162,12 +1162,18 @@ class moodle_content_writer_test extends advanced_testcase {
         if (null === $subcontext) {
             $rcm = $rc->getMethod('get_context_path');
             $rcm->setAccessible(true);
-            return $rcm->invoke($writer);
+            $path = $rcm->invoke($writer);
         } else {
             $rcm = $rc->getMethod('get_path');
             $rcm->setAccessible(true);
-            return $rcm->invoke($writer, $subcontext, $name);
+            $path = $rcm->invoke($writer, $subcontext, $name);
         }
+
+        // PHPUnit uses mikey179/vfsStream which is a stream wrapper for a virtual file system that uses '/'
+        // as the directory separator.
+        $path = str_replace(DIRECTORY_SEPARATOR, '/', $path);
+
+        return $path;
     }
 
     /**
index 943d54e..496d996 100644 (file)
@@ -163,6 +163,18 @@ class provider_testcase extends advanced_testcase {
         }
     }
 
+    /**
+     * Test that all providers implement some form of compliant provider.
+     *
+     * @dataProvider get_component_list
+     * @param string $component frankenstyle component name, e.g. 'mod_assign'
+     * @param string $classname the fully qualified provider classname
+     */
+    public function test_all_providers_compliant($component, $classname) {
+        $manager = new manager();
+        $this->assertTrue($manager->component_is_compliant($component));
+    }
+
     /**
      * Data provider for the metadata\provider tests.
      *
index 377ae22..0c2df5f 100644 (file)
@@ -251,7 +251,7 @@ class qtype_calculatedmulti extends qtype_calculated {
                 }
                 $replaces[$formula] = $str;
             }
-            $anstext = str_replace(arary_keys($replaces), arary_values($replaces), $anssubstituted);
+            $anstext = str_replace(array_keys($replaces), array_values($replaces), $anssubstituted);
             $comment->stranswers[$key] = $anssubstituted.'<br/>'.$anstext;
         }
         return fullclone($comment);
index 1bab73b..4237ea2 100644 (file)
@@ -139,12 +139,13 @@ class provider implements
 
         // Join the rating table with the specified alias and the relevant join params.
         $join = "LEFT JOIN {rating} {$alias} ON ";
+        $join .= "{$alias}.userid = :ratinguserid{$count} AND ";
         $join .= "{$alias}.component = :ratingcomponent{$count} AND ";
         $join .= "{$alias}.ratingarea = :ratingarea{$count} AND ";
         $join .= "{$alias}.itemid = {$itemidjoin}";
 
         // Match against the specified user.
-        $userwhere = "{$alias}.userid = :ratinguserid{$count}";
+        $userwhere = "{$alias}.id IS NOT NULL";
 
         $params = [
             'ratingcomponent' . $count  => $component,
index 590da44..36cb2b3 100644 (file)
     .form-group {
         margin: 0.1rem 0.25rem 0.1rem 0;
     }
+    br + label {
+        justify-content: flex-start;
+        width: 100%;
+    }
 }
 
 #jump-to-activity.custom-select {
index e5e796f..1bf85c5 100644 (file)
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2018050900.01;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2018051200.00;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 
-$release  = '3.5beta+ (Build: 20180508)'; // Human-friendly version name
+$release  = '3.5beta+ (Build: 20180512)'; // Human-friendly version name
 
 $branch   = '35';                       // This version's branch.
 $maturity = MATURITY_BETA;             // This version's maturity level.