From 72de139a6ea58f76ba51a00c2fb90477c31fdbb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Wed, 26 Jun 2013 23:27:00 +0200 Subject: [PATCH] MDL-40337 cleanup role permissions code --- admin/roles/classes/permission_allow_form.php | 25 +++++------ .../classes/permission_prohibit_form.php | 23 ++++++----- admin/roles/classes/permissions_table.php | 31 +++++++------- admin/roles/permissions.php | 41 ++++++++++--------- 4 files changed, 63 insertions(+), 57 deletions(-) diff --git a/admin/roles/classes/permission_allow_form.php b/admin/roles/classes/permission_allow_form.php index 51a0c3e3d3c..1572c732c92 100644 --- a/admin/roles/classes/permission_allow_form.php +++ b/admin/roles/classes/permission_allow_form.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * This script serves draft files of current user + * Allow something form. * * @package core_role * @copyright 2009 Petr Skoda {@link http://skodak.org} @@ -26,40 +26,41 @@ defined('MOODLE_INTERNAL') || die(); require_once("$CFG->libdir/formslib.php"); - class core_role_permission_allow_form extends moodleform { - // Define the form - function definition() { + /** + * Define the form. + */ + protected function definition() { global $CFG; $mform = $this->_form; list($context, $capability, $overridableroles) = $this->_customdata; list($needed, $forbidden) = get_roles_with_cap_in_context($context, $capability->name); - foreach($needed as $id=>$unused) { + foreach ($needed as $id => $unused) { unset($overridableroles[$id]); } - foreach($forbidden as $id=>$unused) { + foreach ($forbidden as $id => $unused) { unset($overridableroles[$id]); } - $mform->addElement('header', 'allowheader', get_string('roleallowheader', 'role')); + $mform->addElement('header', 'allowheader', get_string('roleallowheader', 'core_role')); - $mform->addElement('select', 'roleid', get_string('roleselect', 'role'), $overridableroles); + $mform->addElement('select', 'roleid', get_string('roleselect', 'core_role'), $overridableroles); - $mform->addElement('hidden','capability'); + $mform->addElement('hidden', 'capability'); $mform->setType('capability', PARAM_CAPABILITY); $mform->setDefault('capability', $capability->name); - $mform->addElement('hidden','contextid'); + $mform->addElement('hidden', 'contextid'); $mform->setType('contextid', PARAM_INT); $mform->setDefault('contextid', $context->id); - $mform->addElement('hidden','allow'); + $mform->addElement('hidden', 'allow'); $mform->setType('allow', PARAM_INT); $mform->setDefault('allow', 1); - $this->add_action_buttons(true, get_string('allow', 'role')); + $this->add_action_buttons(true, get_string('allow', 'core_role')); } } diff --git a/admin/roles/classes/permission_prohibit_form.php b/admin/roles/classes/permission_prohibit_form.php index d8a85789b89..ad07ad627bd 100644 --- a/admin/roles/classes/permission_prohibit_form.php +++ b/admin/roles/classes/permission_prohibit_form.php @@ -1,5 +1,4 @@ . /** - * This script serves draft files of current user + * Prohibit something form. * * @package core_role * @copyright 2009 Petr Skoda {@link http://skodak.org} @@ -30,34 +29,36 @@ require_once("$CFG->libdir/formslib.php"); class core_role_permission_prohibit_form extends moodleform { - // Define the form - function definition() { + /** + * Define the form. + */ + protected function definition() { global $CFG; $mform = $this->_form; list($context, $capability, $overridableroles) = $this->_customdata; list($needed, $forbidden) = get_roles_with_cap_in_context($context, $capability->name); - foreach($forbidden as $id=>$unused) { + foreach ($forbidden as $id => $unused) { unset($overridableroles[$id]); } - $mform->addElement('header', 'ptohibitheader', get_string('roleprohibitheader', 'role')); + $mform->addElement('header', 'ptohibitheader', get_string('roleprohibitheader', 'core_role')); - $mform->addElement('select', 'roleid', get_string('roleselect', 'role'), $overridableroles); + $mform->addElement('select', 'roleid', get_string('roleselect', 'core_role'), $overridableroles); - $mform->addElement('hidden','capability'); + $mform->addElement('hidden', 'capability'); $mform->setType('capability', PARAM_CAPABILITY); $mform->setDefault('capability', $capability->name); - $mform->addElement('hidden','contextid'); + $mform->addElement('hidden', 'contextid'); $mform->setType('contextid', PARAM_INT); $mform->setDefault('contextid', $context->id); - $mform->addElement('hidden','prohibit'); + $mform->addElement('hidden', 'prohibit'); $mform->setType('prohibit', PARAM_INT); $mform->setDefault('prohibit', 1); - $this->add_action_buttons(true, get_string('prohibit', 'role')); + $this->add_action_buttons(true, get_string('prohibit', 'core_role')); } } diff --git a/admin/roles/classes/permissions_table.php b/admin/roles/classes/permissions_table.php index 081073dea96..4d317f5cad8 100644 --- a/admin/roles/classes/permissions_table.php +++ b/admin/roles/classes/permissions_table.php @@ -36,9 +36,12 @@ class core_role_permissions_table extends core_role_capability_table_base { protected $icons = array(); /** - * Constructor - * @param object $context the context this table relates to. + * Constructor. + * @param context $context the context this table relates to. * @param string $contextname print_context_name($context) - to save recomputing. + * @param array $allowoverrides + * @param array $allowsafeoverrides + * @param array $overridableroles */ public function __construct($context, $contextname, $allowoverrides, $allowsafeoverrides, $overridableroles) { parent::__construct($context, 'permissions'); @@ -53,9 +56,9 @@ class core_role_permissions_table extends core_role_capability_table_base { } protected function add_header_cells() { - echo '' . get_string('risks', 'role') . ''; - echo '' . get_string('neededroles', 'role') . ''; - echo '' . get_string('prohibitedroles', 'role') . ''; + echo '' . get_string('risks', 'core_role') . ''; + echo '' . get_string('neededroles', 'core_role') . ''; + echo '' . get_string('prohibitedroles', 'core_role') . ''; } protected function num_extra_columns() { @@ -78,25 +81,25 @@ class core_role_permissions_table extends core_role_capability_table_base { $forbiddenroles = array(); $allowable = $overridableroles; $forbitable = $overridableroles; - foreach ($neededroles as $id=>$unused) { + foreach ($neededroles as $id => $unused) { unset($allowable[$id]); } - foreach ($forbidden as $id=>$unused) { + foreach ($forbidden as $id => $unused) { unset($allowable[$id]); unset($forbitable[$id]); } - foreach ($roles as $id=>$name) { + foreach ($roles as $id => $name) { if (isset($needed[$id])) { $neededroles[$id] = $roles[$id]; if (isset($overridableroles[$id]) and ($allowoverrides or ($allowsafeoverrides and is_safe_capability($capability)))) { $preventurl = new moodle_url($PAGE->url, array('contextid'=>$contextid, 'roleid'=>$id, 'capability'=>$capability->name, 'prevent'=>1)); - $neededroles[$id] .= $OUTPUT->action_icon($preventurl, new pix_icon('t/delete', get_string('prevent', 'role'))); + $neededroles[$id] .= $OUTPUT->action_icon($preventurl, new pix_icon('t/delete', get_string('prevent', 'core_role'))); } } } $neededroles = implode(', ', $neededroles); - foreach ($roles as $id=>$name) { + foreach ($roles as $id => $name) { if (isset($forbidden[$id]) and ($allowoverrides or ($allowsafeoverrides and is_safe_capability($capability)))) { $forbiddenroles[$id] = $roles[$id]; if (isset($overridableroles[$id]) and prohibit_is_removable($id, $context, $capability->name)) { @@ -109,12 +112,12 @@ class core_role_permissions_table extends core_role_capability_table_base { if ($allowable and ($allowoverrides or ($allowsafeoverrides and is_safe_capability($capability)))) { $allowurl = new moodle_url($PAGE->url, array('contextid'=>$contextid, 'capability'=>$capability->name, 'allow'=>1)); - $neededroles .= '
'.$OUTPUT->action_icon($allowurl, new pix_icon('t/add', get_string('allow', 'role'))).'
'; + $neededroles .= '
'.$OUTPUT->action_icon($allowurl, new pix_icon('t/add', get_string('allow', 'core_role'))).'
'; } if ($forbitable and ($allowoverrides or ($allowsafeoverrides and is_safe_capability($capability)))) { $prohibiturl = new moodle_url($PAGE->url, array('contextid'=>$contextid, 'capability'=>$capability->name, 'prohibit'=>1)); - $forbiddenroles .= '
'.$OUTPUT->action_icon($prohibiturl, new pix_icon('t/add', get_string('prohibit', 'role'))).'
'; + $forbiddenroles .= '
'.$OUTPUT->action_icon($prohibiturl, new pix_icon('t/add', get_string('prohibit', 'core_role'))).'
'; } $risks = $this->get_risks($capability); @@ -128,11 +131,11 @@ class core_role_permissions_table extends core_role_capability_table_base { global $OUTPUT; $allrisks = get_all_risks(); - $risksurl = new moodle_url(get_docs_url(s(get_string('risks', 'role')))); + $risksurl = new moodle_url(get_docs_url(s(get_string('risks', 'core_role')))); $return = ''; - foreach ($allrisks as $type=>$risk) { + foreach ($allrisks as $type => $risk) { if ($risk & (int)$capability->riskbitmask) { if (!isset($this->icons[$type])) { $pixicon = new pix_icon('/i/' . str_replace('risk', 'risk_', $type), get_string($type . 'short', 'admin')); diff --git a/admin/roles/permissions.php b/admin/roles/permissions.php index 41222e28ced..1990ccaff50 100644 --- a/admin/roles/permissions.php +++ b/admin/roles/permissions.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * This script serves draft files of current user + * Change permissions. * * @package core_role * @copyright 2009 Petr Skoda {@link http://skodak.org} @@ -24,7 +24,7 @@ require('../../config.php'); -$contextid = required_param('contextid',PARAM_INT); +$contextid = required_param('contextid', PARAM_INT); $roleid = optional_param('roleid', 0, PARAM_INT); $capability = optional_param('capability', false, PARAM_CAPABILITY); @@ -52,7 +52,7 @@ if ($course) { } } -// security first +// Security first. require_login($course, false, $cm); require_capability('moodle/role:review', $context); $PAGE->set_url($url); @@ -60,7 +60,7 @@ $PAGE->set_context($context); $courseid = $course->id; -// These are needed early because of tabs.php +// These are needed early because of tabs.php. $assignableroles = get_assignable_roles($context, ROLENAME_BOTH); list($overridableroles, $overridecounts, $nameswithcounts) = get_overridable_roles($context, ROLENAME_BOTH, true); if ($capability) { @@ -70,9 +70,9 @@ if ($capability) { $allowoverrides = has_capability('moodle/role:override', $context); $allowsafeoverrides = has_capability('moodle/role:safeoverride', $context); -$contextname = print_context_name($context); -$title = get_string('permissionsincontext', 'role', $contextname); -$straction = get_string('permissions', 'role'); // Used by tabs.php +$contextname = $context->get_context_name(); +$title = get_string('permissionsincontext', 'core_role', $contextname); +$straction = get_string('permissions', 'core_role'); // Used by tabs.php. $currenttab = 'permissions'; $PAGE->set_pagelayout('admin'); @@ -98,7 +98,7 @@ switch ($context->contextlevel) { } break; case CONTEXT_MODULE: - $PAGE->set_heading(print_context_name($context, false)); + $PAGE->set_heading($context->get_context_name(false)); $PAGE->set_cacheable(false); break; case CONTEXT_BLOCK: @@ -106,12 +106,12 @@ switch ($context->contextlevel) { break; } -// handle confirmations and actions -// We have a capability and overrides are allowed or safe overrides are allowed and this is safe +// Handle confirmations and actions. +// We have a capability and overrides are allowed or safe overrides are allowed and this is safe. if ($capability && ($allowoverrides || ($allowsafeoverrides && is_safe_capability($capability)))) { - // If we already know the the role ID, it is overrideable, and we are setting prevent or unprohibit + // If we already know the the role ID, it is overrideable, and we are setting prevent or unprohibit. if (isset($overridableroles[$roleid]) && ($prevent || $unprohibit)) { - // We are preventing + // We are preventing. if ($prevent) { if ($confirm && data_submitted() && confirm_sesskey()) { role_change_permission($roleid, $context, $capability->name, CAP_PREVENT); @@ -120,10 +120,11 @@ if ($capability && ($allowoverrides || ($allowsafeoverrides && is_safe_capabilit } else { $a = (object)array('cap'=>get_capability_docs_link($capability)." ($capability->name)", 'role'=>$overridableroles[$roleid], 'context'=>$contextname); $message = get_string('confirmroleprevent', 'role', $a); - $continueurl = new moodle_url($PAGE->url, array('contextid'=>$context->id, 'roleid'=>$roleid, 'capability'=>$capability->name, 'prevent'=>1, 'sesskey'=>sesskey(), 'confirm'=>1)); + $continueurl = new moodle_url($PAGE->url, + array('contextid'=>$context->id, 'roleid'=>$roleid, 'capability'=>$capability->name, 'prevent'=>1, 'sesskey'=>sesskey(), 'confirm'=>1)); } } - // We are unprohibiting + // We are unprohibiting. if ($unprohibit) { if ($confirm && data_submitted() && confirm_sesskey()) { role_change_permission($roleid, $context, $capability->name, CAP_INHERIT); @@ -131,10 +132,11 @@ if ($capability && ($allowoverrides || ($allowsafeoverrides && is_safe_capabilit } else { $a = (object)array('cap'=>get_capability_docs_link($capability)." ($capability->name)", 'role'=>$overridableroles[$roleid], 'context'=>$contextname); $message = get_string('confirmroleunprohibit', 'role', $a); - $continueurl = new moodle_url($PAGE->url, array('contextid'=>$context->id, 'roleid'=>$roleid, 'capability'=>$capability->name, 'unprohibit'=>1, 'sesskey'=>sesskey(), 'confirm'=>1)); + $continueurl = new moodle_url($PAGE->url, + array('contextid'=>$context->id, 'roleid'=>$roleid, 'capability'=>$capability->name, 'unprohibit'=>1, 'sesskey'=>sesskey(), 'confirm'=>1)); } } - // Display and print + // Display and print. echo $OUTPUT->header(); echo $OUTPUT->heading($title); echo $OUTPUT->confirm($message, $continueurl, $PAGE->url); @@ -170,7 +172,7 @@ if ($capability && ($allowoverrides || ($allowsafeoverrides && is_safe_capabilit redirect($PAGE->url); } else { $a = (object)array('cap'=>get_capability_docs_link($capability)." ($capability->name)", 'context'=>$contextname); - $message = get_string('roleprohibitinfo', 'role', $a); + $message = get_string('roleprohibitinfo', 'core_role', $a); } } echo $OUTPUT->header(); @@ -187,7 +189,7 @@ echo $OUTPUT->heading($title); $table = new core_role_permissions_table($context, $contextname, $allowoverrides, $allowsafeoverrides, $overridableroles); echo $OUTPUT->box_start('generalbox capbox'); -// print link to advanced override page +// Print link to advanced override page. if ($overridableroles) { $overrideurl = new moodle_url('/admin/roles/override.php', array('contextid' => $context->id)); $select = new single_select($overrideurl, 'roleid', $nameswithcounts); @@ -200,9 +202,8 @@ echo $OUTPUT->box_end(); if ($context->contextlevel > CONTEXT_USER) { echo html_writer::start_tag('div', array('class'=>'backlink')); - echo html_writer::tag('a', get_string('backto', '', $contextname), array('href'=>get_context_url($context))); + echo html_writer::tag('a', get_string('backto', '', $contextname), array('href'=>$context->get_url())); echo html_writer::end_tag('div'); } echo $OUTPUT->footer($course); -