From 827c8e98ac6223479ef9424c81e5ac77562595e0 Mon Sep 17 00:00:00 2001
From: Sara Arjona <sara@moodle.com>
Date: Mon, 14 May 2018 13:23:01 +0200
Subject: [PATCH 1/4] MDL-62433 tool_policy: Review privacy provider for adding
 versions

---
 .../tool/policy/classes/privacy/provider.php  | 141 ++++++++++++---
 admin/tool/policy/lang/en/tool_policy.php     |  17 +-
 .../policy/tests/privacy_provider_test.php    | 162 +++++++++++-------
 3 files changed, 232 insertions(+), 88 deletions(-)

diff --git a/admin/tool/policy/classes/privacy/provider.php b/admin/tool/policy/classes/privacy/provider.php
index 7de5edcfcdc..772819df705 100644
--- a/admin/tool/policy/classes/privacy/provider.php
+++ b/admin/tool/policy/classes/privacy/provider.php
@@ -49,11 +49,11 @@ class provider implements
     /**
      * 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.
+     * @param   collection $collection The initialised collection to add items to.
+     * @return  collection A listing of user data stored through this system.
      */
-    public static function get_metadata(collection $items) : collection {
-        $items->add_database_table(
+    public static function get_metadata(collection $collection) : collection {
+        $collection->add_database_table(
             'tool_policy_acceptances',
             [
                 'policyversionid' => 'privacy:metadata:acceptances:policyversionid',
@@ -68,7 +68,29 @@ class provider implements
             'privacy:metadata:acceptances'
         );
 
-        return $items;
+        $collection->add_database_table(
+            'tool_policy_versions',
+            [
+                'name' => 'privacy:metadata:versions:name',
+                'type' => 'privacy:metadata:versions:type',
+                'audience' => 'privacy:metadata:versions:audience',
+                'archived' => 'privacy:metadata:versions:archived',
+                'usermodified' => 'privacy:metadata:versions:usermodified',
+                'timecreated' => 'privacy:metadata:versions:timecreated',
+                'timemodified' => 'privacy:metadata:versions:timemodified',
+                'policyid' => 'privacy:metadata:versions:policyid',
+                'revision' => 'privacy:metadata:versions:revision',
+                'summary' => 'privacy:metadata:versions:summary',
+                'summaryformat' => 'privacy:metadata:versions:summaryformat',
+                'content' => 'privacy:metadata:versions:content',
+                'contentformat' => 'privacy:metadata:versions:contentformat',
+            ],
+            'privacy:metadata:versions'
+        );
+
+        $collection->add_subsystem_link('core_files', [], 'privacy:metadata:subsystem:corefiles');
+
+        return $collection;
     }
 
     /**
@@ -79,11 +101,21 @@ class provider implements
      */
     public static function get_contexts_for_userid(int $userid) : contextlist {
         $contextlist = new contextlist();
-        $contextlist->add_from_sql('SELECT DISTINCT c.id
-            FROM {tool_policy_acceptances} a
-            JOIN {context} c ON a.userid = c.instanceid AND c.contextlevel = ?
-            WHERE a.userid = ? OR a.usermodified = ?',
-            [CONTEXT_USER, $userid, $userid]);
+
+        $sql = "SELECT c.id
+                  FROM {context} c
+             LEFT JOIN {tool_policy_versions} v ON v.usermodified = c.instanceid
+             LEFT JOIN {tool_policy_acceptances} a ON a.userid = c.instanceid
+                 WHERE c.contextlevel = :contextlevel
+                   AND (v.usermodified = :usermodified OR a.userid = :userid OR a.usermodified = :behalfuserid)";
+        $params = [
+            'contextlevel' => CONTEXT_USER,
+            'usermodified' => $userid,
+            'userid'       => $userid,
+            'behalfuserid' => $userid,
+        ];
+        $contextlist->add_from_sql($sql, $params);
+
         return $contextlist;
     }
 
@@ -94,41 +126,106 @@ class provider implements
      */
     public static function export_user_data(approved_contextlist $contextlist) {
         global $DB;
-        foreach ($contextlist->get_contexts() as $context) {
-            if ($context->contextlevel != CONTEXT_USER) {
-                continue;
+
+        // Remove contexts different from USER.
+        $contexts = array_reduce($contextlist->get_contexts(), function($carry, $context) {
+            if ($context->contextlevel == CONTEXT_USER) {
+                $carry[$context->instanceid] = $context;
             }
+            return $carry;
+        }, []);
+
+        if (empty($contexts)) {
+            return;
+        }
+
+        // Export user agreements.
+        $subcontext = [
+            get_string('privacyandpolicies', 'admin'),
+            get_string('useracceptances', 'tool_policy')
+        ];
+        $policyversionids = [];
+        foreach ($contexts as $context) {
             $user = $contextlist->get_user();
             $agreements = $DB->get_records_sql('SELECT a.id, a.userid, v.name, v.revision, a.usermodified, a.timecreated,
                   a.timemodified, a.note, v.archived, p.currentversionid, a.status, a.policyversionid
                 FROM {tool_policy_acceptances} a
-                JOIN {tool_policy_versions} v ON v.id=a.policyversionid
+                JOIN {tool_policy_versions} v ON v.id = a.policyversionid
                 JOIN {tool_policy} p ON v.policyid = p.id
                 WHERE a.userid = ? AND (a.userid = ? OR a.usermodified = ?)
                 ORDER BY a.userid, v.archived, v.timecreated DESC',
                 [$context->instanceid, $user->id, $user->id]);
             foreach ($agreements as $agreement) {
                 $context = \context_user::instance($agreement->userid);
-                $subcontext = [
-                    get_string('userpoliciesagreements', 'tool_policy'),
-                    transform::user($agreement->userid)
-                ];
                 $name = 'policyagreement-' . $agreement->policyversionid;
                 $agreementcontent = (object) [
-                    'userid' => transform::user($agreement->userid),
-                    'status' => $agreement->status,
-                    'versionid' => $agreement->policyversionid,
                     'name' => $agreement->name,
                     'revision' => $agreement->revision,
                     'isactive' => transform::yesno($agreement->policyversionid == $agreement->currentversionid),
-                    'usermodified' => transform::user($agreement->usermodified),
+                    'isagreed' => transform::yesno($agreement->status),
+                    'agreedby' => transform::user($agreement->usermodified),
                     'timecreated' => transform::datetime($agreement->timecreated),
                     'timemodified' => transform::datetime($agreement->timemodified),
                     'note' => $agreement->note,
                 ];
                 writer::with_context($context)->export_related_data($subcontext, $name, $agreementcontent);
+                $policyversionids[$agreement->policyversionid] = $agreement->policyversionid;
             }
         }
+
+        // Export policy versions (agreed or modified by the user).
+        $userid = $contextlist->get_user()->id;
+        $context = \context_system::instance();
+        $subcontext = [
+            get_string('policydocuments', 'tool_policy')
+        ];
+        $writer = writer::with_context($context);
+        list($contextsql, $contextparams) = $DB->get_in_or_equal(array_keys($contexts), SQL_PARAMS_NAMED);
+        list($versionsql, $versionparams) = $DB->get_in_or_equal($policyversionids, SQL_PARAMS_NAMED);
+        $sql = "SELECT v.id,
+                       v.name,
+                       v.revision,
+                       v.summary,
+                       v.content,
+                       v.archived,
+                       v.usermodified,
+                       v.timecreated,
+                       v.timemodified,
+                       p.currentversionid
+                  FROM {tool_policy_versions} v
+                  JOIN {tool_policy} p ON p.id = v.policyid
+                 WHERE v.usermodified {$contextsql} OR v.id {$versionsql}";
+        $params = array_merge($contextparams, $versionparams);
+        $versions = $DB->get_recordset_sql($sql, $params);
+        foreach ($versions as $version) {
+            $name = 'policyversion-' . $version->id;
+            $versioncontent = (object) [
+                'name' => $version->name,
+                'revision' => $version->revision,
+                'summary' => $writer->rewrite_pluginfile_urls(
+                    $subcontext,
+                    'tool_policy',
+                    'policydocumentsummary',
+                    $version->id,
+                    $version->summary
+                ),
+                'content' => $writer->rewrite_pluginfile_urls(
+                    $subcontext,
+                    'tool_policy',
+                    'policydocumentcontent',
+                    $version->id,
+                    $version->content
+                ),
+                'isactive' => transform::yesno($version->id == $version->currentversionid),
+                'isarchived' => transform::yesno($version->archived),
+                'createdbyme' => transform::yesno($version->usermodified == $userid),
+                'timecreated' => transform::datetime($version->timecreated),
+                'timemodified' => transform::datetime($version->timemodified),
+            ];
+            $writer->export_related_data($subcontext, $name, $versioncontent);
+            $writer->export_area_files($subcontext, 'tool_policy', 'policydocumentsummary', $version->id);
+            $writer->export_area_files($subcontext, 'tool_policy', 'policydocumentcontent', $version->id);
+        }
     }
 
     /**
diff --git a/admin/tool/policy/lang/en/tool_policy.php b/admin/tool/policy/lang/en/tool_policy.php
index efd944db838..ef5a5dc1e55 100644
--- a/admin/tool/policy/lang/en/tool_policy.php
+++ b/admin/tool/policy/lang/en/tool_policy.php
@@ -124,6 +124,7 @@ $string['policydoctype0'] = 'Site policy';
 $string['policydoctype1'] = 'Privacy policy';
 $string['policydoctype2'] = 'Third parties policy';
 $string['policydoctype99'] = 'Other policy';
+$string['policydocuments'] = 'Policy documents';
 $string['policyversionacceptedinbehalf'] = 'Consent for this policy has been given on your behalf.';
 $string['policyversionacceptedinotherlang'] = 'Consent for this policy version has been given in a different language.';
 $string['previousversions'] = '{$a} previous versions';
@@ -136,6 +137,21 @@ $string['privacy:metadata:acceptances:usermodified'] = 'The ID of the user accep
 $string['privacy:metadata:acceptances:timecreated'] = 'The time when the user agreed to the policy';
 $string['privacy:metadata:acceptances:timemodified'] = 'The time when the user modified their agreement';
 $string['privacy:metadata:acceptances:note'] = 'Any comments added by a user when giving consent on behalf of another user';
+$string['privacy:metadata:subsystem:corefiles'] = 'Policy tool stores files embedded in to the policies summary or content text';
+$string['privacy:metadata:versions'] = 'Information from versions of the policy documents';
+$string['privacy:metadata:versions:name'] = 'The policy document name.';
+$string['privacy:metadata:versions:type'] = 'The policy document type: site, privacy, third party...';
+$string['privacy:metadata:versions:audience'] = 'The policy document audience: all users, only authenticated users or only guests.';
+$string['privacy:metadata:versions:archived'] = 'Whether the policy version is active or not.';
+$string['privacy:metadata:versions:usermodified'] = 'The ID of the user modifying the policy.';
+$string['privacy:metadata:versions:timecreated'] = 'The time when the policy version was created by the user.';
+$string['privacy:metadata:versions:timemodified'] = 'The time when the policy version was modified by the user.';
+$string['privacy:metadata:versions:policyid'] = 'The ID of the policy document where this version belongs.';
+$string['privacy:metadata:versions:revision'] = 'The policy version name.';
+$string['privacy:metadata:versions:summary'] = 'The policy version summary.';
+$string['privacy:metadata:versions:summaryformat'] = 'The policy version summary text format.';
+$string['privacy:metadata:versions:content'] = 'The policy version content.';
+$string['privacy:metadata:versions:contentformat'] = 'The policy version content text format.';
 $string['privacysettings'] = 'Privacy settings';
 $string['readpolicy'] = 'Please read our {$a}';
 $string['refertofullpolicytext'] = 'Please refer to the full {$a} if you would like to review the text.';
@@ -154,7 +170,6 @@ $string['status2'] = 'Inactive';
 $string['useracceptancecount'] = '{$a->agreedcount} of {$a->userscount} ({$a->percent}%)';
 $string['useracceptancecountna'] = 'N/A';
 $string['useracceptances'] = 'User agreements';
-$string['userpoliciesagreements'] = 'User agreements to policies';
 $string['userpolicysettings'] = 'Policies';
 $string['usersaccepted'] = 'Agreements';
 $string['viewarchived'] = 'View previous versions';
diff --git a/admin/tool/policy/tests/privacy_provider_test.php b/admin/tool/policy/tests/privacy_provider_test.php
index d392cc15fcf..12ae6c148ac 100644
--- a/admin/tool/policy/tests/privacy_provider_test.php
+++ b/admin/tool/policy/tests/privacy_provider_test.php
@@ -42,6 +42,9 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
     /** @var stdClass The user object. */
     protected $user;
 
+    /** @var stdClass The manager user object. */
+    protected $manager;
+
     /**
      * Setup function. Will create a user.
      */
@@ -50,31 +53,15 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
 
         $generator = $this->getDataGenerator();
         $this->user = $generator->create_user();
-    }
 
-    /**
-     * Test for provider::get_metadata().
-     */
-    public function test_get_metadata() {
-        $collection = new collection('tool_policy');
-        $newcollection = provider::get_metadata($collection);
-        $itemcollection = $newcollection->get_collection();
-        $this->assertCount(1, $itemcollection);
-
-        $table = reset($itemcollection);
-        $this->assertEquals('tool_policy_acceptances', $table->get_name());
-
-        $privacyfields = $table->get_privacy_fields();
-        $this->assertArrayHasKey('policyversionid', $privacyfields);
-        $this->assertArrayHasKey('userid', $privacyfields);
-        $this->assertArrayHasKey('status', $privacyfields);
-        $this->assertArrayHasKey('lang', $privacyfields);
-        $this->assertArrayHasKey('usermodified', $privacyfields);
-        $this->assertArrayHasKey('timecreated', $privacyfields);
-        $this->assertArrayHasKey('timemodified', $privacyfields);
-        $this->assertArrayHasKey('note', $privacyfields);
-
-        $this->assertEquals('privacy:metadata:acceptances', $table->get_summary());
+        // Create manager user.
+        $this->manager = $generator->create_user();
+        $syscontext = context_system::instance();
+        $rolemanagerid = create_role('Policy manager', 'policymanager', 'Can manage policy documents');
+        assign_capability('tool/policy:managedocs', CAP_ALLOW, $rolemanagerid, $syscontext->id);
+        assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $rolemanagerid, $syscontext->id);
+        role_assign($rolemanagerid, $this->manager->id, $syscontext->id);
+        accesslib_clear_all_caches_for_unit_testing();
     }
 
     /**
@@ -84,16 +71,22 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         global $CFG;
 
         // When there are no policies or agreements context list is empty.
+        $contextlist = \tool_policy\privacy\provider::get_contexts_for_userid($this->manager->id);
+        $this->assertEmpty($contextlist);
         $contextlist = \tool_policy\privacy\provider::get_contexts_for_userid($this->user->id);
         $this->assertEmpty($contextlist);
 
         // Create a policy.
-        $this->setAdminUser();
+        $this->setUser($this->manager);
         $CFG->sitepolicyhandler = 'tool_policy';
         $policy = $this->add_policy();
         api::make_current($policy->get('id'));
 
-        // When there are no agreements context list is empty.
+        // After creating a policy, there should be manager context.
+        $contextlist = \tool_policy\privacy\provider::get_contexts_for_userid($this->manager->id);
+        $this->assertEquals(1, $contextlist->count());
+
+        // But when there are no agreements, user context list is empty.
         $contextlist = \tool_policy\privacy\provider::get_contexts_for_userid($this->user->id);
         $this->assertEmpty($contextlist);
 
@@ -107,12 +100,19 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
     }
 
     public function test_export_own_agreements() {
-        global $CFG, $USER;
+        global $CFG;
 
-        // Create policies and agree to them as admin.
-        $this->setAdminUser();
-        $admin = fullclone($USER);
-        $admincontext = \context_user::instance($admin->id);
+        // Create policies and agree to them as manager.
+        $this->setUser($this->manager);
+        $managercontext = \context_user::instance($this->manager->id);
+        $systemcontext = \context_system::instance();
+        $agreementsubcontext = [
+            get_string('privacyandpolicies', 'admin'),
+            get_string('useracceptances', 'tool_policy')
+        ];
+        $versionsubcontext = [
+            get_string('policydocuments', 'tool_policy')
+        ];
         $CFG->sitepolicyhandler = 'tool_policy';
         $policy1 = $this->add_policy();
         api::make_current($policy1->get('id'));
@@ -132,26 +132,43 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         $approvedcontextlist = new approved_contextlist($this->user, 'tool_policy', [$usercontext->id]);
         provider::export_user_data($approvedcontextlist);
 
-        // User can not see admin's agreements but can see his own.
-        $writer = writer::with_context($admincontext);
-        $dataadmin = $writer->get_related_data([get_string('userpoliciesagreements', 'tool_policy'), $admin->id]);
-        $this->assertEmpty($dataadmin);
+        // User can not see manager's agreements but can see his own.
+        $writer = writer::with_context($managercontext);
+        $datamanager = $writer->get_related_data($agreementsubcontext);
+        $this->assertEmpty($datamanager);
 
         $writer = writer::with_context($usercontext);
-        $datauser = $writer->get_related_data([get_string('userpoliciesagreements', 'tool_policy'), $this->user->id]);
+        $datauser = $writer->get_related_data($agreementsubcontext);
         $this->assertCount(2, (array) $datauser);
         $this->assertEquals($policy1->get('name'), $datauser['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($this->user->id, $datauser['policyagreement-'.$policy1->get('id')]->usermodified);
+        $this->assertEquals($this->user->id, $datauser['policyagreement-'.$policy1->get('id')]->agreedby);
         $this->assertEquals($policy2->get('name'), $datauser['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($this->user->id, $datauser['policyagreement-'.$policy2->get('id')]->usermodified);
+        $this->assertEquals($this->user->id, $datauser['policyagreement-'.$policy2->get('id')]->agreedby);
+
+        // User can see policy documents.
+        $writer = writer::with_context($systemcontext);
+        $dataversion = $writer->get_related_data($versionsubcontext);
+        $this->assertCount(2, (array) $dataversion);
+        $this->assertEquals($policy1->get('name'), $dataversion['policyversion-'.$policy1->get('id')]->name);
+        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy1->get('id')]->createdbyme);
+        $this->assertEquals($policy2->get('name'), $dataversion['policyversion-'.$policy2->get('id')]->name);
+        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy2->get('id')]->createdbyme);
     }
 
     public function test_export_agreements_on_behalf() {
-        global $CFG, $USER;
+        global $CFG;
 
         // Create policies.
-        $this->setAdminUser();
-        $admin = fullclone($USER);
+        $this->setUser($this->manager);
+        $managercontext = \context_user::instance($this->manager->id);
+        $systemcontext = \context_system::instance();
+        $agreementsubcontext = [
+            get_string('privacyandpolicies', 'admin'),
+            get_string('useracceptances', 'tool_policy')
+        ];
+        $versionsubcontext = [
+            get_string('policydocuments', 'tool_policy')
+        ];
         $CFG->sitepolicyhandler = 'tool_policy';
         $policy1 = $this->add_policy();
         api::make_current($policy1->get('id'));
@@ -160,7 +177,6 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
 
         // Agree to the policies for oneself and for another user.
         $usercontext = \context_user::instance($this->user->id);
-        $admincontext = \context_user::instance($USER->id);
         api::accept_policies([$policy1->get('id'), $policy2->get('id')]);
         api::accept_policies([$policy1->get('id'), $policy2->get('id')], $this->user->id, 'Mynote');
 
@@ -174,47 +190,63 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         $approvedcontextlist = new approved_contextlist($this->user, 'tool_policy', [$usercontext->id]);
         provider::export_user_data($approvedcontextlist);
 
-        // User can not see admin's agreements but can see his own.
-        $writer = writer::with_context($admincontext);
-        $dataadmin = $writer->get_related_data([get_string('userpoliciesagreements', 'tool_policy'), $admin->id]);
-        $this->assertEmpty($dataadmin);
+        // User can not see manager's agreements but can see his own.
+        $writer = writer::with_context($managercontext);
+        $datamanager = $writer->get_related_data($agreementsubcontext);
+        $this->assertEmpty($datamanager);
 
         $writer = writer::with_context($usercontext);
-        $datauser = $writer->get_related_data([get_string('userpoliciesagreements', 'tool_policy'), $this->user->id]);
+        $datauser = $writer->get_related_data($agreementsubcontext);
         $this->assertCount(2, (array) $datauser);
         $this->assertEquals($policy1->get('name'), $datauser['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($admin->id, $datauser['policyagreement-'.$policy1->get('id')]->usermodified);
+        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy1->get('id')]->agreedby);
         $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy1->get('id')]->note);
         $this->assertEquals($policy2->get('name'), $datauser['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($admin->id, $datauser['policyagreement-'.$policy2->get('id')]->usermodified);
+        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy2->get('id')]->agreedby);
         $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy2->get('id')]->note);
 
-        // Request export for the admin.
-        writer::reset();
-        $contextlist = provider::get_contexts_for_userid($USER->id);
-        $this->assertEquals([$admincontext->id, $usercontext->id], $contextlist->get_contextids(), '', 0.0, 10, true);
+        $writer = writer::with_context($systemcontext);
+        $dataversion = $writer->get_related_data($versionsubcontext);
+        $this->assertCount(2, (array) $dataversion);
+        $this->assertEquals($policy1->get('name'), $dataversion['policyversion-'.$policy1->get('id')]->name);
+        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy1->get('id')]->createdbyme);
+        $this->assertEquals($policy2->get('name'), $dataversion['policyversion-'.$policy2->get('id')]->name);
+        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy2->get('id')]->createdbyme);
 
-        $approvedcontextlist = new approved_contextlist($USER, 'tool_policy', $contextlist->get_contextids());
+        // Request export for the manager.
+        writer::reset();
+        $contextlist = provider::get_contexts_for_userid($this->manager->id);
+        $this->assertEquals([$managercontext->id, $usercontext->id], $contextlist->get_contextids(), '', 0.0, 10, true);
+
+        $approvedcontextlist = new approved_contextlist($this->manager, 'tool_policy', $contextlist->get_contextids());
         provider::export_user_data($approvedcontextlist);
 
-        // Admin can see all four agreements.
-        $writer = writer::with_context($admincontext);
-        $dataadmin = $writer->get_related_data([get_string('userpoliciesagreements', 'tool_policy'), $admin->id]);
-        $this->assertCount(2, (array) $dataadmin);
-        $this->assertEquals($policy1->get('name'), $dataadmin['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($admin->id, $dataadmin['policyagreement-'.$policy1->get('id')]->usermodified);
-        $this->assertEquals($policy2->get('name'), $dataadmin['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($admin->id, $dataadmin['policyagreement-'.$policy2->get('id')]->usermodified);
+        // Manager can see all four agreements.
+        $writer = writer::with_context($managercontext);
+        $datamanager = $writer->get_related_data($agreementsubcontext);
+        $this->assertCount(2, (array) $datamanager);
+        $this->assertEquals($policy1->get('name'), $datamanager['policyagreement-'.$policy1->get('id')]->name);
+        $this->assertEquals($this->manager->id, $datamanager['policyagreement-'.$policy1->get('id')]->agreedby);
+        $this->assertEquals($policy2->get('name'), $datamanager['policyagreement-'.$policy2->get('id')]->name);
+        $this->assertEquals($this->manager->id, $datamanager['policyagreement-'.$policy2->get('id')]->agreedby);
 
         $writer = writer::with_context($usercontext);
-        $datauser = $writer->get_related_data([get_string('userpoliciesagreements', 'tool_policy'), $this->user->id]);
+        $datauser = $writer->get_related_data($agreementsubcontext);
         $this->assertCount(2, (array) $datauser);
         $this->assertEquals($policy1->get('name'), $datauser['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($admin->id, $datauser['policyagreement-'.$policy1->get('id')]->usermodified);
+        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy1->get('id')]->agreedby);
         $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy1->get('id')]->note);
         $this->assertEquals($policy2->get('name'), $datauser['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($admin->id, $datauser['policyagreement-'.$policy2->get('id')]->usermodified);
+        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy2->get('id')]->agreedby);
         $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy2->get('id')]->note);
+
+        $writer = writer::with_context($systemcontext);
+        $dataversion = $writer->get_related_data($versionsubcontext);
+        $this->assertCount(2, (array) $dataversion);
+        $this->assertEquals($policy1->get('name'), $dataversion['policyversion-'.$policy1->get('id')]->name);
+        $this->assertEquals(get_string('yes'), $dataversion['policyversion-'.$policy1->get('id')]->createdbyme);
+        $this->assertEquals($policy2->get('name'), $dataversion['policyversion-'.$policy2->get('id')]->name);
+        $this->assertEquals(get_string('yes'), $dataversion['policyversion-'.$policy2->get('id')]->createdbyme);
     }
 
     /**

From f63745eabdb8aae573d6b6e7702de4103c3d7e4b Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Tue, 15 May 2018 10:57:28 +0800
Subject: [PATCH 2/4] MDL-62433 tool_policy: Adjustments to the data export

---
 .../tool/policy/classes/privacy/provider.php  | 249 ++++++++++++------
 admin/tool/policy/lang/en/tool_policy.php     |  41 +--
 .../policy/tests/privacy_provider_test.php    | 205 ++++++++------
 3 files changed, 311 insertions(+), 184 deletions(-)

diff --git a/admin/tool/policy/classes/privacy/provider.php b/admin/tool/policy/classes/privacy/provider.php
index 772819df705..e01168fad39 100644
--- a/admin/tool/policy/classes/privacy/provider.php
+++ b/admin/tool/policy/classes/privacy/provider.php
@@ -102,17 +102,30 @@ class provider implements
     public static function get_contexts_for_userid(int $userid) : contextlist {
         $contextlist = new contextlist();
 
+        // Policies a user has modified.
         $sql = "SELECT c.id
                   FROM {context} c
-             LEFT JOIN {tool_policy_versions} v ON v.usermodified = c.instanceid
-             LEFT JOIN {tool_policy_acceptances} a ON a.userid = c.instanceid
-                 WHERE c.contextlevel = :contextlevel
-                   AND (v.usermodified = :usermodified OR a.userid = :userid OR a.usermodified = :behalfuserid)";
+                  JOIN {tool_policy_versions} v ON v.usermodified = :userid
+                 WHERE c.contextlevel = :contextlevel";
+        $params = [
+            'contextlevel' => CONTEXT_SYSTEM,
+            'userid' => $userid,
+        ];
+        $contextlist->add_from_sql($sql, $params);
+
+        // Policies a user has accepted.
+        $sql = "SELECT c.id
+                  FROM {context} c
+                  JOIN {tool_policy_acceptances} a ON c.instanceid = a.userid
+                 WHERE
+                    c.contextlevel = :contextlevel
+                   AND (
+                    a.userid = :userid OR a.usermodified = :usermodified
+                   )";
         $params = [
             'contextlevel' => CONTEXT_USER,
+            'userid' => $userid,
             'usermodified' => $userid,
-            'userid'       => $userid,
-            'behalfuserid' => $userid,
         ];
         $contextlist->add_from_sql($sql, $params);
 
@@ -132,6 +145,9 @@ class provider implements
             if ($context->contextlevel == CONTEXT_USER) {
                 $carry[$context->instanceid] = $context;
             }
+            if ($context->contextlevel == CONTEXT_SYSTEM) {
+                $carry[$context->instanceid] = $context;
+            }
             return $carry;
         }, []);
 
@@ -145,86 +161,20 @@ class provider implements
             get_string('useracceptances', 'tool_policy')
         ];
         $policyversionids = [];
-        foreach ($contexts as $context) {
-            $user = $contextlist->get_user();
-            $agreements = $DB->get_records_sql('SELECT a.id, a.userid, v.name, v.revision, a.usermodified, a.timecreated,
-                  a.timemodified, a.note, v.archived, p.currentversionid, a.status, a.policyversionid
-                FROM {tool_policy_acceptances} a
-                JOIN {tool_policy_versions} v ON v.id = a.policyversionid
-                JOIN {tool_policy} p ON v.policyid = p.id
-                WHERE a.userid = ? AND (a.userid = ? OR a.usermodified = ?)
-                ORDER BY a.userid, v.archived, v.timecreated DESC',
-                [$context->instanceid, $user->id, $user->id]);
-            foreach ($agreements as $agreement) {
-                $context = \context_user::instance($agreement->userid);
-                $name = 'policyagreement-' . $agreement->policyversionid;
-                $agreementcontent = (object) [
-                    'name' => $agreement->name,
-                    'revision' => $agreement->revision,
-                    'isactive' => transform::yesno($agreement->policyversionid == $agreement->currentversionid),
-                    'isagreed' => transform::yesno($agreement->status),
-                    'agreedby' => transform::user($agreement->usermodified),
-                    'timecreated' => transform::datetime($agreement->timecreated),
-                    'timemodified' => transform::datetime($agreement->timemodified),
-                    'note' => $agreement->note,
-                ];
-                writer::with_context($context)->export_related_data($subcontext, $name, $agreementcontent);
-                $policyversionids[$agreement->policyversionid] = $agreement->policyversionid;
-            }
-        }
 
-        // Export policy versions (agreed or modified by the user).
-        $userid = $contextlist->get_user()->id;
-        $context = \context_system::instance();
-        $subcontext = [
-            get_string('policydocuments', 'tool_policy')
-        ];
-        $writer = writer::with_context($context);
-        list($contextsql, $contextparams) = $DB->get_in_or_equal(array_keys($contexts), SQL_PARAMS_NAMED);
-        list($versionsql, $versionparams) = $DB->get_in_or_equal($policyversionids, SQL_PARAMS_NAMED);
-        $sql = "SELECT v.id,
-                       v.name,
-                       v.revision,
-                       v.summary,
-                       v.content,
-                       v.archived,
-                       v.usermodified,
-                       v.timecreated,
-                       v.timemodified,
-                       p.currentversionid
-                  FROM {tool_policy_versions} v
-                  JOIN {tool_policy} p ON p.id = v.policyid
-                 WHERE v.usermodified {$contextsql} OR v.id {$versionsql}";
-        $params = array_merge($contextparams, $versionparams);
-        $versions = $DB->get_recordset_sql($sql, $params);
-        foreach ($versions as $version) {
-            $name = 'policyversion-' . $version->id;
-            $versioncontent = (object) [
-                'name' => $version->name,
-                'revision' => $version->revision,
-                'summary' => $writer->rewrite_pluginfile_urls(
-                    $subcontext,
-                    'tool_policy',
-                    'policydocumentsummary',
-                    $version->id,
-                    $version->summary
-                ),
-                'content' => $writer->rewrite_pluginfile_urls(
-                    $subcontext,
-                    'tool_policy',
-                    'policydocumentcontent',
-                    $version->id,
-                    $version->content
-                ),
-                'isactive' => transform::yesno($version->id == $version->currentversionid),
-                'isarchived' => transform::yesno($version->archived),
-                'createdbyme' => transform::yesno($version->usermodified == $userid),
-                'timecreated' => transform::datetime($version->timecreated),
-                'timemodified' => transform::datetime($version->timemodified),
-            ];
-            $writer->export_related_data($subcontext, $name, $versioncontent);
-            $writer->export_area_files($subcontext, 'tool_policy', 'policydocumentsummary', $version->id);
-            $writer->export_area_files($subcontext, 'tool_policy', 'policydocumentcontent', $version->id);
+        $agreementsql = "SELECT
+                            a.id, a.userid, v.name, v.revision, a.usermodified, a.timecreated,
+                            a.timemodified, a.note, v.archived, p.currentversionid, a.status, a.policyversionid
+                         FROM {tool_policy_acceptances} a
+                         JOIN {tool_policy_versions} v ON v.id = a.policyversionid
+                         JOIN {tool_policy} p ON v.policyid = p.id
+                        WHERE a.userid = :userid OR a.usermodified = :usermodified";
+        foreach ($contexts as $context) {
+            if ($context->contextlevel == CONTEXT_USER) {
+                static::export_policy_agreements_for_context($context);
+            } else if ($context->contextlevel == CONTEXT_SYSTEM) {
+                static::export_authored_policies($contextlist->get_user());
+            }
         }
     }
 
@@ -247,4 +197,133 @@ class provider implements
      */
     public static function delete_data_for_user(approved_contextlist $contextlist) {
     }
+
+    /**
+     * Export all policy agreements relating to the specified user context.
+     *
+     * @param \context_user $context The context to export
+     */
+    protected static function export_policy_agreements_for_context(\context_user $context) {
+        global $DB;
+
+        $agreementsql = "
+            SELECT
+                a.id, a.userid, a.timemodified, a.note, a.status, a.policyversionid, a.usermodified, a.timecreated,
+                v.id AS versionid, v.archived, v.name, v.revision,
+                v.summary, v.summaryformat,
+                v.content, v.contentformat,
+                p.currentversionid
+             FROM {tool_policy_acceptances} a
+             JOIN {tool_policy_versions} v ON v.id = a.policyversionid
+             JOIN {tool_policy} p ON v.policyid = p.id
+            WHERE a.userid = :userid OR a.usermodified = :usermodified";
+
+        // Fetch all agreements related to this user.
+        $agreements = $DB->get_recordset_sql($agreementsql, [
+            'userid' => $context->instanceid,
+            'usermodified' => $context->instanceid,
+        ]);
+
+        $basecontext = [
+            get_string('privacyandpolicies', 'admin'),
+            get_string('useracceptances', 'tool_policy'),
+        ];
+
+        foreach ($agreements as $agreement) {
+            $subcontext = array_merge($basecontext, [get_string('policynamedversion', 'tool_policy', $agreement)]);
+
+            $summary = writer::with_context($context)->rewrite_pluginfile_urls(
+                $subcontext,
+                'tool_policy',
+                'policydocumentsummary',
+                $agreement->versionid,
+                $agreement->summary
+            );
+            $content = writer::with_context($context)->rewrite_pluginfile_urls(
+                $subcontext,
+                'tool_policy',
+                'policydocumentcontent',
+                $agreement->versionid,
+                $agreement->content
+            );
+            $agreementcontent = (object) [
+                'name' => $agreement->name,
+                'revision' => $agreement->revision,
+                'isactive' => transform::yesno($agreement->policyversionid == $agreement->currentversionid),
+                'isagreed' => transform::yesno($agreement->status),
+                'agreedby' => transform::user($agreement->usermodified),
+                'timecreated' => transform::datetime($agreement->timecreated),
+                'timemodified' => transform::datetime($agreement->timemodified),
+                'note' => $agreement->note,
+                'summary' => format_text($summary, $agreement->summaryformat),
+                'content' => format_text($content, $agreement->contentformat),
+            ];
+
+            writer::with_context($context)
+                ->export_data($subcontext, $agreementcontent)
+                ->export_area_files($subcontext, 'tool_policy', 'policydocumentsummary', $agreement->versionid)
+                ->export_area_files($subcontext, 'tool_policy', 'policydocumentcontent', $agreement->versionid);
+        }
+        $agreements->close();
+    }
+
+    /**
+     * Export all policy agreements that the user authored.
+     */
+    protected static function export_authored_policies(\stdClass $user) {
+        global $DB;
+
+        // Authored policies are exported against the system.
+        $context = \context_system::instance();
+        $basecontext = [
+            get_string('policydocuments', 'tool_policy'),
+        ];
+
+        $sql = "SELECT v.id,
+                       v.name,
+                       v.revision,
+                       v.summary,
+                       v.content,
+                       v.archived,
+                       v.usermodified,
+                       v.timecreated,
+                       v.timemodified,
+                       p.currentversionid
+                  FROM {tool_policy_versions} v
+                  JOIN {tool_policy} p ON p.id = v.policyid
+                 WHERE v.usermodified = :userid";
+        $versions = $DB->get_recordset_sql($sql, ['userid' => $user->id]);
+        foreach ($versions as $version) {
+            $subcontext = array_merge($basecontext, [get_string('policynamedversion', 'tool_policy', $version)]);
+
+            $versioncontent = (object) [
+                'name' => $version->name,
+                'revision' => $version->revision,
+                'summary' => writer::with_context($context)->rewrite_pluginfile_urls(
+                    $subcontext,
+                    'tool_policy',
+                    'policydocumentsummary',
+                    $version->id,
+                    $version->summary
+                ),
+                'content' => writer::with_context($context)->rewrite_pluginfile_urls(
+                    $subcontext,
+                    'tool_policy',
+                    'policydocumentcontent',
+                    $version->id,
+                    $version->content
+                ),
+                'isactive' => transform::yesno($version->id == $version->currentversionid),
+                'isarchived' => transform::yesno($version->archived),
+                'createdbyme' => transform::yesno($version->usermodified == $user->id),
+                'timecreated' => transform::datetime($version->timecreated),
+                'timemodified' => transform::datetime($version->timemodified),
+            ];
+            writer::with_context($context)
+                ->export_data($subcontext, $versioncontent)
+                ->export_area_files($subcontext, 'tool_policy', 'policydocumentsummary', $version->id)
+                ->export_area_files($subcontext, 'tool_policy', 'policydocumentcontent', $version->id);
+        }
+        $versions->close();
+    }
 }
diff --git a/admin/tool/policy/lang/en/tool_policy.php b/admin/tool/policy/lang/en/tool_policy.php
index ef5a5dc1e55..5b06df4b278 100644
--- a/admin/tool/policy/lang/en/tool_policy.php
+++ b/admin/tool/policy/lang/en/tool_policy.php
@@ -125,33 +125,34 @@ $string['policydoctype1'] = 'Privacy policy';
 $string['policydoctype2'] = 'Third parties policy';
 $string['policydoctype99'] = 'Other policy';
 $string['policydocuments'] = 'Policy documents';
+$string['policynamedversion'] = 'Policy {$a->name} (version {$a->revision})';
 $string['policyversionacceptedinbehalf'] = 'Consent for this policy has been given on your behalf.';
 $string['policyversionacceptedinotherlang'] = 'Consent for this policy version has been given in a different language.';
 $string['previousversions'] = '{$a} previous versions';
-$string['privacy:metadata:acceptances'] = 'Information from policy agreements made by site users';
-$string['privacy:metadata:acceptances:policyversionid'] = 'The ID of the accepted version policy.';
-$string['privacy:metadata:acceptances:userid'] = 'The ID of the user who agreed to the policy.';
-$string['privacy:metadata:acceptances:status'] = 'The status of the agreement: 0 if not accepted; 1 otherwise.';
-$string['privacy:metadata:acceptances:lang'] = 'The current language displayed when the policy is accepted.';
-$string['privacy:metadata:acceptances:usermodified'] = 'The ID of the user accepting the policy, if made on behalf of another user.';
+$string['privacy:metadata:acceptances'] = 'Information about policy agreements made by users';
+$string['privacy:metadata:acceptances:policyversionid'] = 'The version of the policy which was accepted.';
+$string['privacy:metadata:acceptances:userid'] = 'The user who this policy acceptances relates to.';
+$string['privacy:metadata:acceptances:status'] = 'The status of the agreement.';
+$string['privacy:metadata:acceptances:lang'] = 'The language used to display the policy when it was accepted.';
+$string['privacy:metadata:acceptances:usermodified'] = 'The user who accepted the policy, if made on behalf of another user.';
 $string['privacy:metadata:acceptances:timecreated'] = 'The time when the user agreed to the policy';
-$string['privacy:metadata:acceptances:timemodified'] = 'The time when the user modified their agreement';
+$string['privacy:metadata:acceptances:timemodified'] = 'The time when the user updated their agreement';
 $string['privacy:metadata:acceptances:note'] = 'Any comments added by a user when giving consent on behalf of another user';
-$string['privacy:metadata:subsystem:corefiles'] = 'Policy tool stores files embedded in to the policies summary or content text';
+$string['privacy:metadata:subsystem:corefiles'] = 'The policy tool stores files includes in the summary and content of a policy.';
 $string['privacy:metadata:versions'] = 'Information from versions of the policy documents';
-$string['privacy:metadata:versions:name'] = 'The policy document name.';
-$string['privacy:metadata:versions:type'] = 'The policy document type: site, privacy, third party...';
-$string['privacy:metadata:versions:audience'] = 'The policy document audience: all users, only authenticated users or only guests.';
+$string['privacy:metadata:versions:name'] = 'The name of the policy.';
+$string['privacy:metadata:versions:type'] = 'The type of policy document type.';
+$string['privacy:metadata:versions:audience'] = 'The intended audience of the policy.';
 $string['privacy:metadata:versions:archived'] = 'Whether the policy version is active or not.';
-$string['privacy:metadata:versions:usermodified'] = 'The ID of the user modifying the policy.';
-$string['privacy:metadata:versions:timecreated'] = 'The time when the policy version was created by the user.';
-$string['privacy:metadata:versions:timemodified'] = 'The time when the policy version was modified by the user.';
-$string['privacy:metadata:versions:policyid'] = 'The ID of the policy document where this version belongs.';
-$string['privacy:metadata:versions:revision'] = 'The policy version name.';
-$string['privacy:metadata:versions:summary'] = 'The policy version summary.';
-$string['privacy:metadata:versions:summaryformat'] = 'The policy version summary text format.';
-$string['privacy:metadata:versions:content'] = 'The policy version content.';
-$string['privacy:metadata:versions:contentformat'] = 'The policy version content text format.';
+$string['privacy:metadata:versions:usermodified'] = 'The user who modified the policy.';
+$string['privacy:metadata:versions:timecreated'] = 'The time that this version of the policy was created.';
+$string['privacy:metadata:versions:timemodified'] = 'The time that this version of the policy was updated.';
+$string['privacy:metadata:versions:policyid'] = 'The policy that this version is associated with.';
+$string['privacy:metadata:versions:revision'] = 'The revision name of this version of the policy.';
+$string['privacy:metadata:versions:summary'] = 'The summary of this version of the policy.';
+$string['privacy:metadata:versions:summaryformat'] = 'The format of the summary field.';
+$string['privacy:metadata:versions:content'] = 'The content of this version of the policy.';
+$string['privacy:metadata:versions:contentformat'] = 'The format of the content field.';
 $string['privacysettings'] = 'Privacy settings';
 $string['readpolicy'] = 'Please read our {$a}';
 $string['refertofullpolicytext'] = 'Please refer to the full {$a} if you would like to review the text.';
diff --git a/admin/tool/policy/tests/privacy_provider_test.php b/admin/tool/policy/tests/privacy_provider_test.php
index 12ae6c148ac..438572bc4b2 100644
--- a/admin/tool/policy/tests/privacy_provider_test.php
+++ b/admin/tool/policy/tests/privacy_provider_test.php
@@ -99,9 +99,12 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         $this->assertEquals(1, $contextlist->count());
     }
 
-    public function test_export_own_agreements() {
+    public function test_export_agreements() {
         global $CFG;
 
+        $otheruser = $this->getDataGenerator()->create_user();
+        $otherusercontext = \context_user::instance($otheruser->id);
+
         // Create policies and agree to them as manager.
         $this->setUser($this->manager);
         $managercontext = \context_user::instance($this->manager->id);
@@ -127,6 +130,7 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
 
         // Request export for this user.
         $contextlist = provider::get_contexts_for_userid($this->user->id);
+        $this->assertCount(1, $contextlist);
         $this->assertEquals([$usercontext->id], $contextlist->get_contextids());
 
         $approvedcontextlist = new approved_contextlist($this->user, 'tool_policy', [$usercontext->id]);
@@ -134,31 +138,104 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
 
         // User can not see manager's agreements but can see his own.
         $writer = writer::with_context($managercontext);
-        $datamanager = $writer->get_related_data($agreementsubcontext);
-        $this->assertEmpty($datamanager);
+        $this->assertFalse($writer->has_any_data());
 
         $writer = writer::with_context($usercontext);
-        $datauser = $writer->get_related_data($agreementsubcontext);
-        $this->assertCount(2, (array) $datauser);
-        $this->assertEquals($policy1->get('name'), $datauser['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($this->user->id, $datauser['policyagreement-'.$policy1->get('id')]->agreedby);
-        $this->assertEquals($policy2->get('name'), $datauser['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($this->user->id, $datauser['policyagreement-'.$policy2->get('id')]->agreedby);
+        $this->assertTrue($writer->has_any_data());
 
+        // Test policy 1.
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy1->to_record())]);
+        $datauser = $writer->get_data($subcontext);
+        $this->assertEquals($policy1->get('name'), $datauser->name);
+        $this->assertEquals($this->user->id, $datauser->agreedby);
+        $this->assertEquals(strip_tags($policy1->get('summary')), strip_tags($datauser->summary));
+        $this->assertEquals(strip_tags($policy1->get('content')), strip_tags($datauser->content));
+
+        // Test policy 2.
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy2->to_record())]);
+        $datauser = $writer->get_data($subcontext);
+        $this->assertEquals($policy2->get('name'), $datauser->name);
+        $this->assertEquals($this->user->id, $datauser->agreedby);
+        $this->assertEquals(strip_tags($policy2->get('summary')), strip_tags($datauser->summary));
+        $this->assertEquals(strip_tags($policy2->get('content')), strip_tags($datauser->content));
+
+        /**
         // User can see policy documents.
         $writer = writer::with_context($systemcontext);
-        $dataversion = $writer->get_related_data($versionsubcontext);
-        $this->assertCount(2, (array) $dataversion);
-        $this->assertEquals($policy1->get('name'), $dataversion['policyversion-'.$policy1->get('id')]->name);
-        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy1->get('id')]->createdbyme);
-        $this->assertEquals($policy2->get('name'), $dataversion['policyversion-'.$policy2->get('id')]->name);
-        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy2->get('id')]->createdbyme);
+        $this->assertTrue($writer->has_any_data());
+
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy1->to_record())]);
+        $dataversion = $writer->get_data($subcontext);
+        $this->assertEquals($policy1->get('name'), $dataversion->name);
+        $this->assertEquals(get_string('no'), $dataversion->createdbyme);
+
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy2->to_record())]);
+        $dataversion = $writer->get_data($subcontext);
+        $this->assertEquals($policy1->get('name'), $dataversion->name);
+        $this->assertEquals(get_string('no'), $dataversion->createdbyme);
+         */
     }
 
-    public function test_export_agreements_on_behalf() {
+    public function test_export_agreements_for_other() {
         global $CFG;
 
-        // Create policies.
+        $managercontext = \context_user::instance($this->manager->id);
+        $systemcontext = \context_system::instance();
+        $usercontext = \context_user::instance($this->user->id);
+
+        // Create policies and agree to them as manager.
+        $this->setUser($this->manager);
+        $agreementsubcontext = [
+            get_string('privacyandpolicies', 'admin'),
+            get_string('useracceptances', 'tool_policy')
+        ];
+        $versionsubcontext = [
+            get_string('policydocuments', 'tool_policy')
+        ];
+        $CFG->sitepolicyhandler = 'tool_policy';
+        $policy1 = $this->add_policy();
+        api::make_current($policy1->get('id'));
+        $policy2 = $this->add_policy();
+        api::make_current($policy2->get('id'));
+        api::accept_policies([$policy1->get('id'), $policy2->get('id')]);
+
+        // Agree to the other user's policies.
+        api::accept_policies([$policy1->get('id'), $policy2->get('id')], $this->user->id, 'My note');
+
+        // Request export for the manager.
+        $contextlist = provider::get_contexts_for_userid($this->manager->id);
+        $this->assertCount(3, $contextlist);
+        $this->assertEquals([$managercontext->id, $usercontext->id, $systemcontext->id], $contextlist->get_contextids(), '', 0.0, 1, true);
+
+        $approvedcontextlist = new approved_contextlist($this->user, 'tool_policy', [$usercontext->id]);
+        provider::export_user_data($approvedcontextlist);
+
+        // The user context has data.
+        $writer = writer::with_context($usercontext);
+        $this->assertTrue($writer->has_any_data());
+
+        // Test policy 1.
+        $writer = writer::with_context($usercontext);
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy1->to_record())]);
+        $datauser = $writer->get_data($subcontext);
+        $this->assertEquals($policy1->get('name'), $datauser->name);
+        $this->assertEquals($this->manager->id, $datauser->agreedby);
+        $this->assertEquals(strip_tags($policy1->get('summary')), strip_tags($datauser->summary));
+        $this->assertEquals(strip_tags($policy1->get('content')), strip_tags($datauser->content));
+
+        // Test policy 2.
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy2->to_record())]);
+        $datauser = $writer->get_data($subcontext);
+        $this->assertEquals($policy2->get('name'), $datauser->name);
+        $this->assertEquals($this->manager->id, $datauser->agreedby);
+        $this->assertEquals(strip_tags($policy2->get('summary')), strip_tags($datauser->summary));
+        $this->assertEquals(strip_tags($policy2->get('content')), strip_tags($datauser->content));
+    }
+
+    public function test_export_created_policies() {
+        global $CFG;
+
+        // Create policies and agree to them as manager.
         $this->setUser($this->manager);
         $managercontext = \context_user::instance($this->manager->id);
         $systemcontext = \context_system::instance();
@@ -174,79 +251,49 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         api::make_current($policy1->get('id'));
         $policy2 = $this->add_policy();
         api::make_current($policy2->get('id'));
-
-        // Agree to the policies for oneself and for another user.
-        $usercontext = \context_user::instance($this->user->id);
         api::accept_policies([$policy1->get('id'), $policy2->get('id')]);
-        api::accept_policies([$policy1->get('id'), $policy2->get('id')], $this->user->id, 'Mynote');
 
-        // Request export for this user.
-        $contextlist = provider::get_contexts_for_userid($this->user->id);
-        $this->assertEquals([$usercontext->id], $contextlist->get_contextids());
-
-        $writer = writer::with_context($usercontext);
-        $this->assertFalse($writer->has_any_data());
-
-        $approvedcontextlist = new approved_contextlist($this->user, 'tool_policy', [$usercontext->id]);
-        provider::export_user_data($approvedcontextlist);
-
-        // User can not see manager's agreements but can see his own.
-        $writer = writer::with_context($managercontext);
-        $datamanager = $writer->get_related_data($agreementsubcontext);
-        $this->assertEmpty($datamanager);
-
-        $writer = writer::with_context($usercontext);
-        $datauser = $writer->get_related_data($agreementsubcontext);
-        $this->assertCount(2, (array) $datauser);
-        $this->assertEquals($policy1->get('name'), $datauser['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy1->get('id')]->agreedby);
-        $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy1->get('id')]->note);
-        $this->assertEquals($policy2->get('name'), $datauser['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy2->get('id')]->agreedby);
-        $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy2->get('id')]->note);
-
-        $writer = writer::with_context($systemcontext);
-        $dataversion = $writer->get_related_data($versionsubcontext);
-        $this->assertCount(2, (array) $dataversion);
-        $this->assertEquals($policy1->get('name'), $dataversion['policyversion-'.$policy1->get('id')]->name);
-        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy1->get('id')]->createdbyme);
-        $this->assertEquals($policy2->get('name'), $dataversion['policyversion-'.$policy2->get('id')]->name);
-        $this->assertEquals(get_string('no'), $dataversion['policyversion-'.$policy2->get('id')]->createdbyme);
-
-        // Request export for the manager.
-        writer::reset();
+        // Agree to the policies for oneself.
         $contextlist = provider::get_contexts_for_userid($this->manager->id);
-        $this->assertEquals([$managercontext->id, $usercontext->id], $contextlist->get_contextids(), '', 0.0, 10, true);
+        $this->assertCount(2, $contextlist);
+        $this->assertEquals([$managercontext->id, $systemcontext->id], $contextlist->get_contextids(), '', 0.0, 1, true);
 
         $approvedcontextlist = new approved_contextlist($this->manager, 'tool_policy', $contextlist->get_contextids());
         provider::export_user_data($approvedcontextlist);
 
-        // Manager can see all four agreements.
+        // User has agreed to policies.
         $writer = writer::with_context($managercontext);
-        $datamanager = $writer->get_related_data($agreementsubcontext);
-        $this->assertCount(2, (array) $datamanager);
-        $this->assertEquals($policy1->get('name'), $datamanager['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($this->manager->id, $datamanager['policyagreement-'.$policy1->get('id')]->agreedby);
-        $this->assertEquals($policy2->get('name'), $datamanager['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($this->manager->id, $datamanager['policyagreement-'.$policy2->get('id')]->agreedby);
+        $this->assertTrue($writer->has_any_data());
 
-        $writer = writer::with_context($usercontext);
-        $datauser = $writer->get_related_data($agreementsubcontext);
-        $this->assertCount(2, (array) $datauser);
-        $this->assertEquals($policy1->get('name'), $datauser['policyagreement-'.$policy1->get('id')]->name);
-        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy1->get('id')]->agreedby);
-        $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy1->get('id')]->note);
-        $this->assertEquals($policy2->get('name'), $datauser['policyagreement-'.$policy2->get('id')]->name);
-        $this->assertEquals($this->manager->id, $datauser['policyagreement-'.$policy2->get('id')]->agreedby);
-        $this->assertEquals('Mynote', $datauser['policyagreement-'.$policy2->get('id')]->note);
+        // Test policy 1.
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy1->to_record())]);
+        $datauser = $writer->get_data($subcontext);
+        $this->assertEquals($policy1->get('name'), $datauser->name);
+        $this->assertEquals($this->manager->id, $datauser->agreedby);
+        $this->assertEquals(strip_tags($policy1->get('summary')), strip_tags($datauser->summary));
+        $this->assertEquals(strip_tags($policy1->get('content')), strip_tags($datauser->content));
 
+        // Test policy 2.
+        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy2->to_record())]);
+        $datauser = $writer->get_data($subcontext);
+        $this->assertEquals($policy2->get('name'), $datauser->name);
+        $this->assertEquals($this->manager->id, $datauser->agreedby);
+        $this->assertEquals(strip_tags($policy2->get('summary')), strip_tags($datauser->summary));
+        $this->assertEquals(strip_tags($policy2->get('content')), strip_tags($datauser->content));
+
+        // User can see policy documents.
         $writer = writer::with_context($systemcontext);
-        $dataversion = $writer->get_related_data($versionsubcontext);
-        $this->assertCount(2, (array) $dataversion);
-        $this->assertEquals($policy1->get('name'), $dataversion['policyversion-'.$policy1->get('id')]->name);
-        $this->assertEquals(get_string('yes'), $dataversion['policyversion-'.$policy1->get('id')]->createdbyme);
-        $this->assertEquals($policy2->get('name'), $dataversion['policyversion-'.$policy2->get('id')]->name);
-        $this->assertEquals(get_string('yes'), $dataversion['policyversion-'.$policy2->get('id')]->createdbyme);
+        $this->assertTrue($writer->has_any_data());
+
+        $subcontext = array_merge($versionsubcontext, [get_string('policynamedversion', 'tool_policy', $policy1->to_record())]);
+        $dataversion = $writer->get_data($subcontext);
+        $this->assertEquals($policy1->get('name'), $dataversion->name);
+        $this->assertEquals(get_string('yes'), $dataversion->createdbyme);
+
+        $subcontext = array_merge($versionsubcontext, [get_string('policynamedversion', 'tool_policy', $policy2->to_record())]);
+        $dataversion = $writer->get_data($subcontext);
+        $this->assertEquals($policy2->get('name'), $dataversion->name);
+        $this->assertEquals(get_string('yes'), $dataversion->createdbyme);
     }
 
     /**

From 5e84f521fabce7b0e4b60c2b202160bfa2a55caa Mon Sep 17 00:00:00 2001
From: Sara Arjona <sara@moodle.com>
Date: Tue, 15 May 2018 10:35:09 +0200
Subject: [PATCH 3/4] MDL-62433 tool_policy: Some minor fixes to Privacy API
 implementation

- Export files for agreements too.
- Add versionid to the export path to avoid collision if version revision
is not defined.
---
 .../tool/policy/classes/privacy/provider.php  | 56 +++++++------------
 admin/tool/policy/lang/en/tool_policy.php     |  2 +-
 .../policy/tests/privacy_provider_test.php    | 25 +++------
 3 files changed, 29 insertions(+), 54 deletions(-)

diff --git a/admin/tool/policy/classes/privacy/provider.php b/admin/tool/policy/classes/privacy/provider.php
index e01168fad39..67e4e515e5d 100644
--- a/admin/tool/policy/classes/privacy/provider.php
+++ b/admin/tool/policy/classes/privacy/provider.php
@@ -140,36 +140,8 @@ class provider implements
     public static function export_user_data(approved_contextlist $contextlist) {
         global $DB;
 
-        // Remove contexts different from USER.
-        $contexts = array_reduce($contextlist->get_contexts(), function($carry, $context) {
-            if ($context->contextlevel == CONTEXT_USER) {
-                $carry[$context->instanceid] = $context;
-            }
-            if ($context->contextlevel == CONTEXT_SYSTEM) {
-                $carry[$context->instanceid] = $context;
-            }
-            return $carry;
-        }, []);
-
-        if (empty($contexts)) {
-            return;
-        }
-
         // Export user agreements.
-        $subcontext = [
-            get_string('privacyandpolicies', 'admin'),
-            get_string('useracceptances', 'tool_policy')
-        ];
-        $policyversionids = [];
-
-        $agreementsql = "SELECT
-                            a.id, a.userid, v.name, v.revision, a.usermodified, a.timecreated,
-                            a.timemodified, a.note, v.archived, p.currentversionid, a.status, a.policyversionid
-                         FROM {tool_policy_acceptances} a
-                         JOIN {tool_policy_versions} v ON v.id = a.policyversionid
-                         JOIN {tool_policy} p ON v.policyid = p.id
-                        WHERE a.userid = :userid OR a.usermodified = :usermodified";
-        foreach ($contexts as $context) {
+        foreach ($contextlist->get_contexts() as $context) {
             if ($context->contextlevel == CONTEXT_USER) {
                 static::export_policy_agreements_for_context($context);
             } else if ($context->contextlevel == CONTEXT_SYSTEM) {
@@ -182,6 +154,7 @@ class provider implements
      * Delete all data for all users in the specified context.
      *
      * We never delete user agreements to the policies because they are part of privacy data.
+     * We never delete policy versions because they are part of privacy data.
      *
      * @param \context $context The context to delete in.
      */
@@ -192,6 +165,7 @@ class provider implements
      * Delete all user data for the specified user, in the specified contexts.
      *
      * We never delete user agreements to the policies because they are part of privacy data.
+     * We never delete policy versions because they are part of privacy data.
      *
      * @param approved_contextlist $contextlist A list of contexts approved for deletion.
      */
@@ -206,10 +180,13 @@ class provider implements
     protected static function export_policy_agreements_for_context(\context_user $context) {
         global $DB;
 
+        $sysctx = \context_system::instance();
+        $fs = get_file_storage();
         $agreementsql = "
             SELECT
-                a.id, a.userid, a.timemodified, a.note, a.status, a.policyversionid, a.usermodified, a.timecreated,
-                v.id AS versionid, v.archived, v.name, v.revision,
+                a.id AS agreementid, a.userid, a.timemodified, a.note, a.status,
+                a.policyversionid AS versionid, a.usermodified, a.timecreated,
+                v.id, v.archived, v.name, v.revision,
                 v.summary, v.summaryformat,
                 v.content, v.contentformat,
                 p.currentversionid
@@ -249,7 +226,7 @@ class provider implements
             $agreementcontent = (object) [
                 'name' => $agreement->name,
                 'revision' => $agreement->revision,
-                'isactive' => transform::yesno($agreement->policyversionid == $agreement->currentversionid),
+                'isactive' => transform::yesno($agreement->versionid == $agreement->currentversionid),
                 'isagreed' => transform::yesno($agreement->status),
                 'agreedby' => transform::user($agreement->usermodified),
                 'timecreated' => transform::datetime($agreement->timecreated),
@@ -259,16 +236,23 @@ class provider implements
                 'content' => format_text($content, $agreement->contentformat),
             ];
 
-            writer::with_context($context)
-                ->export_data($subcontext, $agreementcontent)
-                ->export_area_files($subcontext, 'tool_policy', 'policydocumentsummary', $agreement->versionid)
-                ->export_area_files($subcontext, 'tool_policy', 'policydocumentcontent', $agreement->versionid);
+            writer::with_context($context)->export_data($subcontext, $agreementcontent);
+            // Manually export the files as they reside in the system context so we can't use
+            // the write's helper methods.
+            foreach ($fs->get_area_files($sysctx->id, 'tool_policy', 'policydocumentsummary', $agreement->versionid) as $file) {
+                writer::with_context($context)->export_file($subcontext, $file);
+            }
+            foreach ($fs->get_area_files($sysctx->id, 'tool_policy', 'policydocumentcontent', $agreement->versionid) as $file) {
+                writer::with_context($context)->export_file($subcontext, $file);
+            }
         }
         $agreements->close();
     }
 
     /**
      * Export all policy agreements that the user authored.
+     *
+     * @param stdClass $user The user who has created the policies to export.
      */
     protected static function export_authored_policies(\stdClass $user) {
         global $DB;
diff --git a/admin/tool/policy/lang/en/tool_policy.php b/admin/tool/policy/lang/en/tool_policy.php
index 5b06df4b278..9a177a06ceb 100644
--- a/admin/tool/policy/lang/en/tool_policy.php
+++ b/admin/tool/policy/lang/en/tool_policy.php
@@ -125,7 +125,7 @@ $string['policydoctype1'] = 'Privacy policy';
 $string['policydoctype2'] = 'Third parties policy';
 $string['policydoctype99'] = 'Other policy';
 $string['policydocuments'] = 'Policy documents';
-$string['policynamedversion'] = 'Policy {$a->name} (version {$a->revision})';
+$string['policynamedversion'] = 'Policy {$a->name} (version {$a->revision} - {$a->id})';
 $string['policyversionacceptedinbehalf'] = 'Consent for this policy has been given on your behalf.';
 $string['policyversionacceptedinotherlang'] = 'Consent for this policy version has been given in a different language.';
 $string['previousversions'] = '{$a} previous versions';
diff --git a/admin/tool/policy/tests/privacy_provider_test.php b/admin/tool/policy/tests/privacy_provider_test.php
index 438572bc4b2..78556c9bd52 100644
--- a/admin/tool/policy/tests/privacy_provider_test.php
+++ b/admin/tool/policy/tests/privacy_provider_test.php
@@ -158,22 +158,6 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         $this->assertEquals($this->user->id, $datauser->agreedby);
         $this->assertEquals(strip_tags($policy2->get('summary')), strip_tags($datauser->summary));
         $this->assertEquals(strip_tags($policy2->get('content')), strip_tags($datauser->content));
-
-        /**
-        // User can see policy documents.
-        $writer = writer::with_context($systemcontext);
-        $this->assertTrue($writer->has_any_data());
-
-        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy1->to_record())]);
-        $dataversion = $writer->get_data($subcontext);
-        $this->assertEquals($policy1->get('name'), $dataversion->name);
-        $this->assertEquals(get_string('no'), $dataversion->createdbyme);
-
-        $subcontext = array_merge($agreementsubcontext, [get_string('policynamedversion', 'tool_policy', $policy2->to_record())]);
-        $dataversion = $writer->get_data($subcontext);
-        $this->assertEquals($policy1->get('name'), $dataversion->name);
-        $this->assertEquals(get_string('no'), $dataversion->createdbyme);
-         */
     }
 
     public function test_export_agreements_for_other() {
@@ -205,7 +189,14 @@ class tool_policy_privacy_provider_testcase extends \core_privacy\tests\provider
         // Request export for the manager.
         $contextlist = provider::get_contexts_for_userid($this->manager->id);
         $this->assertCount(3, $contextlist);
-        $this->assertEquals([$managercontext->id, $usercontext->id, $systemcontext->id], $contextlist->get_contextids(), '', 0.0, 1, true);
+        $this->assertEquals(
+            [$managercontext->id, $usercontext->id, $systemcontext->id],
+            $contextlist->get_contextids(),
+            '',
+            0.0,
+            1,
+            true
+        );
 
         $approvedcontextlist = new approved_contextlist($this->user, 'tool_policy', [$usercontext->id]);
         provider::export_user_data($approvedcontextlist);

From 68ed91b7af39228bc0f41d298022779c897af70f Mon Sep 17 00:00:00 2001
From: Sara Arjona <sara@moodle.com>
Date: Tue, 15 May 2018 14:03:00 +0200
Subject: [PATCH 4/4] MDL-62433 tool_policy: Review language strings for
 consistency

All credit goes to Helen Foster. Thanks! :-*
---
 admin/tool/policy/lang/en/tool_policy.php | 26 +++++++++++------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/admin/tool/policy/lang/en/tool_policy.php b/admin/tool/policy/lang/en/tool_policy.php
index 9a177a06ceb..c451c1a5cd0 100644
--- a/admin/tool/policy/lang/en/tool_policy.php
+++ b/admin/tool/policy/lang/en/tool_policy.php
@@ -129,21 +129,21 @@ $string['policynamedversion'] = 'Policy {$a->name} (version {$a->revision} - {$a
 $string['policyversionacceptedinbehalf'] = 'Consent for this policy has been given on your behalf.';
 $string['policyversionacceptedinotherlang'] = 'Consent for this policy version has been given in a different language.';
 $string['previousversions'] = '{$a} previous versions';
-$string['privacy:metadata:acceptances'] = 'Information about policy agreements made by users';
-$string['privacy:metadata:acceptances:policyversionid'] = 'The version of the policy which was accepted.';
-$string['privacy:metadata:acceptances:userid'] = 'The user who this policy acceptances relates to.';
+$string['privacy:metadata:acceptances'] = 'Information about policy agreements made by users.';
+$string['privacy:metadata:acceptances:policyversionid'] = 'The version of the policy for which consent was given.';
+$string['privacy:metadata:acceptances:userid'] = 'The user for whom this policy agreement relates to.';
 $string['privacy:metadata:acceptances:status'] = 'The status of the agreement.';
-$string['privacy:metadata:acceptances:lang'] = 'The language used to display the policy when it was accepted.';
-$string['privacy:metadata:acceptances:usermodified'] = 'The user who accepted the policy, if made on behalf of another user.';
-$string['privacy:metadata:acceptances:timecreated'] = 'The time when the user agreed to the policy';
-$string['privacy:metadata:acceptances:timemodified'] = 'The time when the user updated their agreement';
-$string['privacy:metadata:acceptances:note'] = 'Any comments added by a user when giving consent on behalf of another user';
-$string['privacy:metadata:subsystem:corefiles'] = 'The policy tool stores files includes in the summary and content of a policy.';
-$string['privacy:metadata:versions'] = 'Information from versions of the policy documents';
+$string['privacy:metadata:acceptances:lang'] = 'The language used to display the policy when consent was given.';
+$string['privacy:metadata:acceptances:usermodified'] = 'The user who gave consent for the policy, if made on behalf of another user.';
+$string['privacy:metadata:acceptances:timecreated'] = 'The time when the user agreed to the policy.';
+$string['privacy:metadata:acceptances:timemodified'] = 'The time when the user updated their agreement.';
+$string['privacy:metadata:acceptances:note'] = 'Any comments added by a user when giving consent on behalf of another user.';
+$string['privacy:metadata:subsystem:corefiles'] = 'The policy tool stores files included in the summary and full policy.';
+$string['privacy:metadata:versions'] = 'Policy version information.';
 $string['privacy:metadata:versions:name'] = 'The name of the policy.';
-$string['privacy:metadata:versions:type'] = 'The type of policy document type.';
-$string['privacy:metadata:versions:audience'] = 'The intended audience of the policy.';
-$string['privacy:metadata:versions:archived'] = 'Whether the policy version is active or not.';
+$string['privacy:metadata:versions:type'] = 'Policy type.';
+$string['privacy:metadata:versions:audience'] = 'The type of users required to give their consent.';
+$string['privacy:metadata:versions:archived'] = 'The policy status (active or inactive).';
 $string['privacy:metadata:versions:usermodified'] = 'The user who modified the policy.';
 $string['privacy:metadata:versions:timecreated'] = 'The time that this version of the policy was created.';
 $string['privacy:metadata:versions:timemodified'] = 'The time that this version of the policy was updated.';