From 9dcf28af7df03be26f62671633a2584bc486dad3 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Fri, 18 May 2018 11:51:16 +0800 Subject: [PATCH] MDL-62493 tool_policy: allow guests to sign up --- .../policy/classes/output/page_agreedocs.php | 34 ++++++------ admin/tool/policy/index.php | 2 +- admin/tool/policy/tests/behat/consent.feature | 52 +++++++++++++++++++ 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/admin/tool/policy/classes/output/page_agreedocs.php b/admin/tool/policy/classes/output/page_agreedocs.php index 5ede1610a2b..45853ee3e67 100644 --- a/admin/tool/policy/classes/output/page_agreedocs.php +++ b/admin/tool/policy/classes/output/page_agreedocs.php @@ -69,6 +69,9 @@ class page_agreedocs implements renderable, templatable { /** @var array Info or error messages to show. */ protected $messages = []; + /** @var bool This is an existing user (rather than non-loggedin/guest). */ + protected $isexistinguser; + /** * Prepare the page for rendering. * @@ -87,6 +90,7 @@ class page_agreedocs implements renderable, templatable { $this->action = $action; + $this->isexistinguser = isloggedin() && !isguestuser(); $behalfid = $behalfid ?: $USER->id; if ($realuser->id != $behalfid) { $this->behalfuser = core_user::get_user($behalfid, '*', MUST_EXIST); @@ -112,7 +116,7 @@ class page_agreedocs implements renderable, templatable { protected function accept_and_revoke_policies() { global $USER; - if (!empty($USER->id)) { + if ($this->isexistinguser) { // Existing user. if (!empty($this->action) && confirm_sesskey()) { // The form has been sent. Update policies acceptances according to $this->agreedocs. @@ -182,15 +186,13 @@ class page_agreedocs implements renderable, templatable { * Before display the consent page, the user has to view all the still-non-accepted policy docs. * This function checks if the non-accepted policy docs have been shown and redirect to them. * - * @param array $userid User identifier who wants to access to the consent page. - * @param url $returnurl URL to return after shown the policy docs. + * @param int $userid User identifier who wants to access to the consent page. + * @param moodle_url $returnurl URL to return after shown the policy docs. */ protected function redirect_to_policies($userid, $returnurl = null) { - global $USER; - - $acceptances = api::get_user_acceptances($userid); $allpolicies = $this->policies; - if (!empty($userid)) { + if ($this->isexistinguser) { + $acceptances = api::get_user_acceptances($userid); foreach ($allpolicies as $policy) { if (api::is_user_version_accepted($userid, $policy->id, $acceptances)) { // If this version is accepted by the user, remove from the pending policies list. @@ -259,31 +261,31 @@ class page_agreedocs implements renderable, templatable { // Guest users or not logged users (but the users during the signup process) are not allowed to access to this page. $newsignupuser = !empty($SESSION->wantsurl) && strpos($SESSION->wantsurl, 'login/signup.php') !== false; - if (isguestuser() || (empty($USER->id) && !$newsignupuser)) { + if (!$this->isexistinguser && !$newsignupuser) { $this->redirect_to_previous_url(); } // Check for correct user capabilities. - if (!empty($USER->id)) { + if ($this->isexistinguser) { // For existing users, it's needed to check if they have the capability for accepting policies. api::can_accept_policies($this->behalfid, true); } else { // For new users, the behalfid parameter is ignored. - if ($this->behalfid != $USER->id) { + if ($this->behalfid) { redirect(new moodle_url('/admin/tool/policy/index.php')); } } // If the current user has the $USER->policyagreed = 1 or $userpolicyagreed = 1 // and $SESSION->wantsurl is defined, redirect to the return page. - $hasagreedsignupuser = empty($USER->id) && $this->signupuserpolicyagreed; + $hasagreedsignupuser = !$this->isexistinguser && $this->signupuserpolicyagreed; $hasagreedloggeduser = $USER->id == $userid && !empty($USER->policyagreed); if (!is_siteadmin() && ($hasagreedsignupuser || $hasagreedloggeduser)) { $this->redirect_to_previous_url(); } $myparams = []; - if (!empty($USER->id) && !empty($this->behalfid) && $this->behalfid != $USER->id) { + if ($this->isexistinguser && !empty($this->behalfid) && $this->behalfid != $USER->id) { $myparams['userid'] = $this->behalfid; } $myurl = new moodle_url('/admin/tool/policy/index.php', $myparams); @@ -308,7 +310,6 @@ class page_agreedocs implements renderable, templatable { global $USER; // Get all the policy version acceptances for this user. - $acceptances = api::get_user_acceptances($userid); $lang = current_language(); foreach ($this->policies as $policy) { // Get a link to display the full policy document. @@ -320,9 +321,10 @@ class page_agreedocs implements renderable, templatable { $policymodal = html_writer::link($policy->url, $policy->name, $policyattributes); // Check if this policy version has been agreed or not. - if (!empty($userid)) { + if ($this->isexistinguser) { // Existing user. $versionagreed = false; + $acceptances = api::get_user_acceptances($userid); $policy->versionacceptance = api::get_user_version_acceptance($userid, $policy->id, $acceptances); if (!empty($policy->versionacceptance)) { // The policy version has ever been agreed. Check if status = 1 to know if still is accepted. @@ -352,13 +354,13 @@ class page_agreedocs implements renderable, templatable { * Export the page data for the mustache template. * * @param renderer_base $output renderer to be used to render the page elements. - * @return stdClass + * @return \stdClass */ public function export_for_template(renderer_base $output) { global $USER; $myparams = []; - if (!empty($USER->id) && !empty($this->behalfid) && $this->behalfid != $USER->id) { + if ($this->isexistinguser && !empty($this->behalfid) && $this->behalfid != $USER->id) { $myparams['userid'] = $this->behalfid; } $data = (object) [ diff --git a/admin/tool/policy/index.php b/admin/tool/policy/index.php index 4be85548368..067d8fabc34 100644 --- a/admin/tool/policy/index.php +++ b/admin/tool/policy/index.php @@ -45,7 +45,7 @@ $PAGE->set_pagelayout('standard'); $PAGE->set_url('/admin/tool/policy/index.php'); $PAGE->set_popup_notification_allowed(false); -if (!empty($USER->id)) { +if (isloggedin() && !isguestuser()) { // Existing user. $haspermissionagreedocs = api::can_accept_policies($behalfid); } else { diff --git a/admin/tool/policy/tests/behat/consent.feature b/admin/tool/policy/tests/behat/consent.feature index ec9aae6c9cd..fde43ccc0cb 100644 --- a/admin/tool/policy/tests/behat/consent.feature +++ b/admin/tool/policy/tests/behat/consent.feature @@ -611,3 +611,55 @@ Feature: User must accept policy managed by this plugin when logging in and sign And I should see "Policies and agreements" And I should see "No permission to agree to the policies on behalf of this user" And I should see "Sorry, you do not have the required permission to agree to the following policies on behalf of User 1" + + Scenario: Accept policy on sign up as a guest, one policy + Given the following config values are set as admin: + | registerauth | email | + | passwordpolicy | 0 | + | sitepolicyhandler | tool_policy | + Given the following policies exist: + | Policy | Name | Revision | Content | Summary | Status | + | P1 | This site policy | | full text1 | short text1 | archived | + | P1 | This site policy | | full text2 | short text2 | active | + | P1 | This site policy | | full text3 | short text3 | draft | + And I am on site homepage + And I follow "Log in" + # First log in as a guest + And I press "Log in as a guest" + # Now sign up + And I follow "Log in" + When I press "Create new account" + Then I should see "This site policy" + And I should see "short text2" + And I should see "full text2" + And I press "Next" + And I should see "Please agree to the following policies" + And I should see "This site policy" + And I should see "short text2" + And I should not see "full text2" + And I set the field "I agree to the This site policy" to "1" + And I press "Next" + And I should not see "I understand and agree" + And I set the following fields to these values: + | Username | user1 | + | Password | user1 | + | Email address | user1@address.invalid | + | Email (again) | user1@address.invalid | + | First name | User1 | + | Surname | L1 | + And I press "Create my new account" + And I should see "Confirm your account" + And I should see "An email should have been sent to your address at user1@address.invalid" + And I confirm email for "user1" + And I should see "Thanks, User1 L1" + And I should see "Your registration has been confirmed" + And I open my profile in edit mode + And the field "First name" matches value "User1" + And I log out + # Confirm that user can login and browse the site. + And I log in as "user1" + And I follow "Profile" in the user menu + # User can see his own agreements in the profile. + And I follow "Policies and agreements" + And "Agreed" "icon" should exist in the "This site policy" "table_row" + And I log out