mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-22 00:42:54 +02:00
Code Issues Projects Releases Wiki Activity

MDL-62309 tool_policy: Amend behaviour of handler's accept() method

Browse Source 
When accepting the policies via the sitepolicy handler, only compulsory
policies are to be marked as accepted. Optional policies will be left as
pending. Users must express their consent explicitly for them.

As a side product of the change, unit tests are added for the whole
handler class.
This commit is contained in:
David Mudrák 2018-10-05 17:09:29 +02:00
parent ad5e2135c5
commit 222c378870
4 changed files with 267 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
admin/tool/policy/classes/privacy/local/sitepolicy/handler.php
View File

@ -79,26 +79,37 @@ class handler extends \core_privacy\local\sitepolicy\handler {
*/
public static function accept() {
global $USER, $DB;
if (!isloggedin()) {
return false;
}
if ($USER->policyagreed) {
return false;
}
if (!isguestuser()) {
// Accepts all policies with a current version for logged users on behalf of the current user.
if (!$versions = api::get_current_versions_ids(policy_version::AUDIENCE_LOGGEDIN)) {
return false;
}
api::accept_policies(array_values($versions));
if (isguestuser()) {
// For guests, agreement is stored in the session only.
$USER->policyagreed = 1;
return true;
}
if (!isguestuser()) {
// For the guests agreement in stored in session only, for other users - in DB.
$DB->set_field('user', 'policyagreed', 1, array('id' => $USER->id));
// Find all compulsory policies and mark them as accepted.
$compulsory = [];
foreach (api::list_current_versions(policy_version::AUDIENCE_LOGGEDIN) as $policyversion) {
if ($policyversion->optional == policy_version::AGREEMENT_COMPULSORY) {
$compulsory[] = $policyversion->id;
}
}
if ($compulsory) {
api::accept_policies($compulsory);
}
// Mark policies as agreed.
$DB->set_field('user', 'policyagreed', 1, array('id' => $USER->id));
$USER->policyagreed = 1;
return true;
}
@ -110,7 +121,7 @@ class handler extends \core_privacy\local\sitepolicy\handler {
public static function signup_form($mform) {
if (static::is_defined()) {
// This plugin displays policies to the user who is signing up before the signup form is shown.
// By the time user has access to signup form they have already agreed to the policies.
// By the time user has access to signup form they have already agreed to all compulsory policies.
$mform->addElement('hidden', 'policyagreed', 1);
$mform->setType('policyagreed', PARAM_INT);
}

88
admin/tool/policy/classes/test/helper.php Normal file
View File

@ -0,0 +1,88 @@
<?php
// This file is part of Moodle - https://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/>.
/**
* Provides the {@link \tool_policy\test\helper} class.
*
* @package tool_policy
* @category test
* @copyright 2018 David Mudrák <david@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace tool_policy\test;
use tool_policy\api;
use tool_policy\policy_version;
defined('MOODLE_INTERNAL') || die();
/**
* Provides some helper methods for unit-tests.
*
* @copyright 2018 David Mudrák <david@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class helper {
/**
* Helper method that creates a new policy for testing
*
* @param array $params
* @return policy_version
*/
public static function add_policy($params = []) {
static $counter = 0;
$counter++;
$defaults = [
'name' => 'Policy '.$counter,
'summary_editor' => ['text' => "P$counter summary", 'format' => FORMAT_HTML, 'itemid' => 0],
'content_editor' => ['text' => "P$counter content", 'format' => FORMAT_HTML, 'itemid' => 0],
];
$params = (array)$params + $defaults;
$formdata = api::form_policydoc_data(new policy_version(0));
foreach ($params as $key => $value) {
$formdata->$key = $value;
}
return api::form_policydoc_add($formdata);
}
/**
* Helper method that prepare a policy document with some versions.
*
* @param int $numversions The number of policy versions to create.
* @return array Array with all the policy versions created.
*/
public static function create_versions($numversions = 2) {
// Prepare a policy document with some versions.
$policy = self::add_policy([
'name' => 'Test policy',
'revision' => 'v1',
]);
for ($i = 2; $i <= $numversions; $i++) {
$formdata = api::form_policydoc_data($policy);
$formdata->revision = 'v'.$i;
api::form_policydoc_update_new($formdata);
}
$list = api::list_policies($policy->get('policyid'));
return $list[0]->draftversions;
}
}

89
admin/tool/policy/tests/api_test.php
View File

@ -25,6 +25,7 @@
use tool_policy\api;
use tool_policy\policy_version;
use tool_policy\test\helper;
defined('MOODLE_INTERNAL') || die();
@ -207,9 +208,9 @@ class tool_policy_api_testcase extends advanced_testcase {
$this->resetAfterTest();
$this->setAdminUser();
$policy1 = $this->add_policy(['audience' => policy_version::AUDIENCE_LOGGEDIN]);
$policy2 = $this->add_policy(['audience' => policy_version::AUDIENCE_GUESTS]);
$policy3 = $this->add_policy();
$policy1 = helper::add_policy(['audience' => policy_version::AUDIENCE_LOGGEDIN]);
$policy2 = helper::add_policy(['audience' => policy_version::AUDIENCE_GUESTS]);
$policy3 = helper::add_policy();
api::make_current($policy1->get('id'));
api::make_current($policy2->get('id'));
@ -242,54 +243,6 @@ class tool_policy_api_testcase extends advanced_testcase {
$policy3->get('policyid') => $policy3->get('id')], $ids);
}
/**
* Helper method that creates a new policy for testing
*
* @param array $params
* @return policy_version
*/
protected function add_policy($params = []) {
static $counter = 0;
$counter++;
$defaults = [
'name' => 'Policy '.$counter,
'summary_editor' => ['text' => "P$counter summary", 'format' => FORMAT_HTML, 'itemid' => 0],
'content_editor' => ['text' => "P$counter content", 'format' => FORMAT_HTML, 'itemid' => 0],
];
$params = (array)$params + $defaults;
$formdata = api::form_policydoc_data(new policy_version(0));
foreach ($params as $key => $value) {
$formdata->$key = $value;
}
return api::form_policydoc_add($formdata);
}
/**
* Helper method that prepare a policy document with some versions.
*
* @param int $numversions The number of policy versions to create.
* @return array Array with all the policy versions created.
*/
protected function create_versions($numversions = 2) {
// Prepare a policy document with some versions.
$policy = self::add_policy([
'name' => 'Test policy',
'revision' => 'v1',
]);
for ($i = 2; $i <= $numversions; $i++) {
$formdata = api::form_policydoc_data($policy);
$formdata->revision = 'v'.$i;
api::form_policydoc_update_new($formdata);
}
$list = api::list_policies($policy->get('policyid'));
return $list[0]->draftversions;
}
/**
* Test behaviour of the {@link api::can_user_view_policy_version()} method.
*/
@ -326,7 +279,7 @@ class tool_policy_api_testcase extends advanced_testcase {
accesslib_clear_all_caches_for_unit_testing();
// Prepare a policy document with some versions.
list($policy1, $policy2, $policy3) = $this->create_versions(3);
list($policy1, $policy2, $policy3) = helper::create_versions(3);
// Normally users do not have access to policy drafts.
$this->assertFalse(api::can_user_view_policy_version($policy1, null, $child->id));
@ -405,10 +358,10 @@ class tool_policy_api_testcase extends advanced_testcase {
accesslib_clear_all_caches_for_unit_testing();
$policy1 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy3 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy4 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy1 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy3 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy4 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$mixed = [$policy1->id, $policy2->id, $policy3->id, $policy4->id];
$compulsory = [$policy1->id, $policy2->id];
@ -481,10 +434,10 @@ class tool_policy_api_testcase extends advanced_testcase {
accesslib_clear_all_caches_for_unit_testing();
$policy1 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy3 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy4 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy1 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy3 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy4 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$mixed = [$policy1->id, $policy2->id, $policy3->id, $policy4->id];
$compulsory = [$policy1->id, $policy2->id];
@ -641,11 +594,11 @@ class tool_policy_api_testcase extends advanced_testcase {
$this->resetAfterTest();
$this->setAdminUser();
$policy1 = $this->add_policy()->to_record();
$policy1 = helper::add_policy()->to_record();
api::make_current($policy1->id);
$policy2 = $this->add_policy()->to_record();
$policy2 = helper::add_policy()->to_record();
api::make_current($policy2->id);
$policy3 = $this->add_policy(['optional' => true])->to_record();
$policy3 = helper::add_policy(['optional' => true])->to_record();
api::make_current($policy3->id);
// Accept policy on behalf of somebody else.
@ -694,14 +647,14 @@ class tool_policy_api_testcase extends advanced_testcase {
$user1 = $this->getDataGenerator()->create_user();
// Introducing a new policy.
list($policy1v1, $policy1v2) = $this->create_versions(2);
list($policy1v1, $policy1v2) = helper::create_versions(2);
api::make_current($policy1v1->id);
$this->assertEquals(0, $DB->get_field('user', 'policyagreed', ['id' => $user1->id]));
api::accept_policies([$policy1v1->id], $user1->id);
$this->assertEquals(1, $DB->get_field('user', 'policyagreed', ['id' => $user1->id]));
// Introducing another policy.
$policy2v1 = $this->add_policy()->to_record();
$policy2v1 = helper::add_policy()->to_record();
api::make_current($policy2v1->id);
$this->assertEquals(0, $DB->get_field('user', 'policyagreed', ['id' => $user1->id]));
api::accept_policies([$policy2v1->id], $user1->id);
@ -786,7 +739,7 @@ class tool_policy_api_testcase extends advanced_testcase {
$CFG->sitepolicyhandler = 'tool_policy';
$policy = $this->add_policy()->to_record();
$policy = helper::add_policy()->to_record();
api::make_current($policy->id);
// User has not accepted any policies.
@ -851,9 +804,9 @@ class tool_policy_api_testcase extends advanced_testcase {
$this->resetAfterTest();
$this->setAdminUser();
$policy1 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy1 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
api::make_current($policy1->id);
$policy2 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
api::make_current($policy2->id);
$this->assertEquals(api::get_agreement_optional($policy1->id), policy_version::AGREEMENT_OPTIONAL);

137
admin/tool/policy/tests/sitepolicy_handler_test.php Normal file
View File

@ -0,0 +1,137 @@
<?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/>.
/**
* Provides the {@link tool_policy_sitepolicy_handler_testcase} class.
*
* @package tool_policy
* @category test
* @copyright 2018 David Mudrák <david@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
use tool_policy\api;
use tool_policy\policy_version;
use tool_policy\privacy\local\sitepolicy\handler;
use tool_policy\test\helper;
defined('MOODLE_INTERNAL') || die();
global $CFG;
/**
* Unit tests for the {@link \tool_policy\privacy\local\sitepolicy\handler} class.
*
* @copyright 2018 David Mudrak <david@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class tool_policy_sitepolicy_handler_testcase extends advanced_testcase {
/**
* Test behaviour of the {@link \tool_policy\privacy\local\sitepolicy\handler::get_redirect_url()} method.
*/
public function test_get_redirect_url() {
$this->resetAfterTest();
$this->setAdminUser();
// No redirect for guests.
$this->assertNull(handler::get_redirect_url(true));
// No redirect if there is no policy.
$this->assertNull(handler::get_redirect_url());
// No redirect if no policy for logged in users.
$policy1 = helper::add_policy(['audience' => policy_version::AUDIENCE_GUESTS])->to_record();
api::make_current($policy1->id);
$this->assertNull(handler::get_redirect_url());
// URL only when there is actually some policy to show.
$policy2 = helper::add_policy(['audience' => policy_version::AUDIENCE_LOGGEDIN])->to_record();
api::make_current($policy2->id);
$this->assertInstanceOf('moodle_url', handler::get_redirect_url());
}
/**
* Test behaviour of the {@link \tool_policy\privacy\local\sitepolicy\handler::get_embed_url()} method.
*/
public function test_get_embed_url() {
$this->resetAfterTest();
$this->setAdminUser();
// No embed if there is no policy.
$this->assertNull(handler::get_embed_url());
$this->assertNull(handler::get_embed_url(true));
$policy1 = helper::add_policy(['audience' => policy_version::AUDIENCE_GUESTS])->to_record();
api::make_current($policy1->id);
// Policy exists for guests only.
$this->assertNull(handler::get_embed_url());
$this->assertInstanceOf('moodle_url', handler::get_embed_url(true));
$policy2 = helper::add_policy(['audience' => policy_version::AUDIENCE_LOGGEDIN])->to_record();
api::make_current($policy2->id);
// Some policy exists for all users.
$this->assertInstanceOf('moodle_url', handler::get_embed_url());
$this->assertInstanceOf('moodle_url', handler::get_embed_url(true));
}
/**
* Test behaviour of the {@link \tool_policy\privacy\local\sitepolicy\handler::accept()} method.
*/
public function test_accept() {
global $DB, $USER;
$this->resetAfterTest();
// False if not logged in.
$this->setUser(0);
$this->assertFalse(handler::accept());
// Guests accept policies implicitly by continuing to use the site.
$this->setGuestUser();
$this->assertTrue(handler::accept());
// Create one compulsory and one optional policy.
$this->setAdminUser();
$policy1 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
api::make_current($policy1->id);
$policy2 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
api::make_current($policy2->id);
$user1 = $this->getDataGenerator()->create_user();
$this->assertEquals(0, $DB->get_field('user', 'policyagreed', ['id' => $user1->id]));
$this->assertEmpty($DB->get_records('tool_policy_acceptances', ['userid' => $user1->id]));
$this->setUser($user1->id);
$this->assertEquals(0, $USER->policyagreed);
// Only the compulsory policy is marked as accepted when accepting via the handler.
$this->assertTrue(handler::accept());
$this->assertEquals(1, $DB->get_field('user', 'policyagreed', ['id' => $user1->id]));
$this->assertEquals(1, $USER->policyagreed);
$this->assertEquals(1, $DB->count_records('tool_policy_acceptances', ['userid' => $user1->id]));
$this->assertTrue($DB->record_exists('tool_policy_acceptances', ['userid' => $user1->id,
'policyversionid' => $policy1->id]));
}
/**
* Test presence of the {@link \tool_policy\privacy\local\sitepolicy\handler::signup_form()} method.
*/
public function test_signup_form() {
$this->assertTrue(method_exists('\tool_policy\privacy\local\sitepolicy\handler', 'signup_form'));
}
}