From 6c089f4a286113d87bbb9f0d4fb895506946aca5 Mon Sep 17 00:00:00 2001 From: moodler Date: Fri, 18 Apr 2008 06:18:25 +0000 Subject: [PATCH] MDL-11323 Cleaned up the display of role aliases by extending get_assignable_roles and get_overrideable_roles to use role_fix_names better. The lists of assignable and overrideable are now correctly derived from the actual context (eg activity or module) while the aliases are derived from course that is the parent. Some other cleanups along the way. --- admin/roles/assign.php | 24 ++---------- admin/roles/override.php | 20 +--------- admin/roles/tabs.php | 31 ++++++++++------ lib/accesslib.php | 79 +++++++++++++++++++--------------------- 4 files changed, 62 insertions(+), 92 deletions(-) diff --git a/admin/roles/assign.php b/admin/roles/assign.php index fe574597abc..1b66ddcd342 100755 --- a/admin/roles/assign.php +++ b/admin/roles/assign.php @@ -45,13 +45,11 @@ } else { error('Invalid course id'); } - $coursecontext = $context; } else if (!empty($courseid)){ // we need this for user tabs in user context if (!$course = get_record('course', 'id', $courseid)) { error('Invalid course id'); } - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); } else { $courseid = SITEID; @@ -60,17 +58,12 @@ require_login($course); - if ($context->contextlevel == CONTEXT_COURSE) { - require_login($context->instanceid); - } else { - require_login(); - } - require_capability('moodle/role:assign', $context); /// needed for tabs.php - $overridableroles = get_overridable_roles($context); - $assignableroles = get_assignable_roles($context); // Plain role names, may be altered later + + $overridableroles = get_overridable_roles($context, 'name', ROLENAME_BOTH); + $assignableroles = get_assignable_roles($context, 'name', ROLENAME_BOTH); /// Get some language strings @@ -165,17 +158,6 @@ } -/// Rename some of the role names if needed - if (isset($coursecontext)) { - if ($aliasnames = get_records('role_names', 'contextid', $coursecontext->id)) { - foreach ($aliasnames as $alias) { - if (isset($assignableroles[$alias->roleid])) { - $assignableroles[$alias->roleid] = $alias->name.' ('.$assignableroles[$alias->roleid].')'; - } - } - } - } - /// Process incoming role assignment diff --git a/admin/roles/override.php b/admin/roles/override.php index 27a6c8518df..9aad240e719 100755 --- a/admin/roles/override.php +++ b/admin/roles/override.php @@ -28,13 +28,9 @@ if (!$course = get_record('course', 'id', $courseid)) { error('Bad course ID'); } - - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); - } else { $course = clone($SITE); $courseid = SITEID; - $coursecontext = $context; } require_login($course); @@ -52,8 +48,8 @@ } /// needed for tabs.php - $overridableroles = get_overridable_roles($context); - $assignableroles = get_assignable_roles($context); + $overridableroles = get_overridable_roles($context, 'name', ROLENAME_BOTH); + $assignableroles = get_assignable_roles($context, 'name', ROLENAME_BOTH); /// Get some language strings @@ -155,18 +151,6 @@ include_once('tabs.php'); } -/// Rename some of the role names if needed - if (isset($coursecontext)) { - if ($aliasnames = get_records('role_names', 'contextid', $coursecontext->id)) { - foreach ($aliasnames as $alias) { - if (isset($overridableroles[$alias->roleid])) { - $overridableroles[$alias->roleid] = $alias->name.' ('.$overridableroles[$alias->roleid].')'; - } - } - } - } - - print_heading_with_help(get_string('overridepermissionsin', 'role', print_context_name($context)), 'overrides'); if ($roleid) { diff --git a/admin/roles/tabs.php b/admin/roles/tabs.php index be51f4a714b..ee644407e6d 100755 --- a/admin/roles/tabs.php +++ b/admin/roles/tabs.php @@ -75,8 +75,6 @@ if ($currenttab != 'update') { require_login($course); - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); // for role names later - $fullmodulename = get_string("modulename", $module->name); $streditinga = get_string("editinga", "moodle", $fullmodulename); $strmodulenameplural = get_string("modulenameplural", $module->name); @@ -110,9 +108,6 @@ if ($currenttab != 'update') { if ($block = get_record('block', 'id', $blockinstance->blockid)) { $blockname = print_context_name($context); - // Prepare the last part of the breadcrumbs first - $navlinks[98] = array('name' => $blockname, 'link' => null, 'type' => 'misc'); - $navlinks[99] = array('name' => $straction, 'link' => null, 'type' => 'misc'); switch ($blockinstance->pagetype) { case 'course-view': @@ -120,12 +115,8 @@ if ($currenttab != 'update') { require_login($course); - if ($course->id != SITEID) { - $navlinks[0] = array('name' => $course->shortname, - 'link' => "$CFG->wwwroot/course/view.php?id=$course->id", - 'type' => 'misc'); - $coursecontext = get_context_instance(CONTEXT_COURSE, $course->id); // for role names later - } + $navlinks[] = array('name' => $blockname, 'link' => null, 'type' => 'misc'); + $navlinks[] = array('name' => $straction, 'link' => null, 'type' => 'misc'); $navigation = build_navigation($navlinks); print_header("$straction: $blockname", $course->fullname, $navigation); } @@ -133,14 +124,30 @@ if ($currenttab != 'update') { case 'blog-view': $strblogs = get_string('blogs','blog'); - $navlinks[0] = array('name' => $strblogs, + $navlinks[] = array('name' => $strblogs, 'link' => $CFG->wwwroot.'/blog/index.php', 'type' => 'misc'); + $navlinks[] = array('name' => $blockname, 'link' => null, 'type' => 'misc'); + $navlinks[] = array('name' => $straction, 'link' => null, 'type' => 'misc'); $navigation = build_navigation($navlinks); print_header("$straction: $strblogs", $SITE->fullname, $navigation); break; + case 'tag-index': + $strtags = get_string('tags'); + $navlinks[] = array('name' => $strtags, + 'link' => $CFG->wwwroot.'/tag/index.php', + 'type' => 'misc'); + $navlinks[] = array('name' => $blockname, 'link' => null, 'type' => 'misc'); + $navlinks[] = array('name' => $straction, 'link' => null, 'type' => 'misc'); + $navigation = build_navigation($navlinks); + print_header("$straction: $strtags", $SITE->fullname, $navigation); + break; + default: + $navlinks[] = array('name' => $blockname, 'link' => null, 'type' => 'misc'); + $navlinks[] = array('name' => $straction, 'link' => null, 'type' => 'misc'); + $navigation = build_navigation($navlinks); print_header("$straction: $blockname", $SITE->fullname, $navigation); break; } diff --git a/lib/accesslib.php b/lib/accesslib.php index cebf2322603..e57865e257b 100755 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -151,6 +151,11 @@ define('RISK_XSS', 0x0004); define('RISK_PERSONAL', 0x0008); define('RISK_SPAM', 0x0010); +// rolename displays +define('ROLENAME_ORIGINAL', 0);// the name as defined in the role definition +define('ROLENAME_ALIAS', 1); // the name as defined by a role alias +define('ROLENAME_BOTH', 2); // Both, like this: Role alias (Original) + require_once($CFG->dirroot.'/group/lib.php'); $context_cache = array(); // Cache of all used context objects for performance (by level and instance) @@ -4182,7 +4187,7 @@ function allow_assign($sroleid, $troleid) { * @param string $field * @return array */ -function get_assignable_roles ($context, $field="name") { +function get_assignable_roles ($context, $field='name', $rolenamedisplay=ROLENAME_ALIAS) { global $CFG; @@ -4200,24 +4205,15 @@ function get_assignable_roles ($context, $field="name") { $roleids = implode(',',$roleids); - // apply context role aliases if name requested - if ($field == 'name') { - $f = "COALESCE(rn.name, r.name) AS name"; - } else { - $f = "r.$field"; - } - // The subselect scopes the DISTINCT down to // the role ids - a DISTINCT over the whole of // the role table is much more expensive on some DBs - $sql = "SELECT r.id, $f + $sql = "SELECT r.id, r.$field FROM {$CFG->prefix}role r JOIN ( SELECT DISTINCT allowassign as allowedrole FROM {$CFG->prefix}role_allow_assign raa WHERE raa.roleid IN ($roleids) ) ar ON r.id=ar.allowedrole - LEFT OUTER JOIN {$CFG->prefix}role_names rn - ON (rn.roleid = r.id AND rn.contextid = $context->id) ORDER BY sortorder ASC"; $rs = get_recordset_sql($sql); @@ -4226,10 +4222,8 @@ function get_assignable_roles ($context, $field="name") { $roles[$r->id] = $r->{$field}; } rs_close($rs); - foreach ($roles as $roleid => $rolename) { - $roles[$roleid] = strip_tags(format_string($rolename, true)); - } - return $roles; + + return role_fix_names($roles, $context, $rolenamedisplay); } /** @@ -4240,7 +4234,7 @@ function get_assignable_roles ($context, $field="name") { * @param string $field * @return array */ -function get_assignable_roles_for_switchrole ($context, $field="name") { +function get_assignable_roles_for_switchrole ($context, $field='name', $rolenamedisplay=ROLENAME_ALIAS) { global $CFG; @@ -4258,18 +4252,10 @@ function get_assignable_roles_for_switchrole ($context, $field="name") { $roleids = implode(',',$roleids); - // apply context role aliases if name requested - if ($field == 'name') { - $f = "COALESCE(rn.name, r.name) AS name"; - } else { - $f = "r.$field"; - } - - // The subselect scopes the DISTINCT down to // the role ids - a DISTINCT over the whole of // the role table is much more expensive on some DBs - $sql = "SELECT r.id, $f + $sql = "SELECT r.id, r.$field FROM {$CFG->prefix}role r JOIN ( SELECT DISTINCT allowassign as allowedrole FROM {$CFG->prefix}role_allow_assign raa @@ -4278,8 +4264,6 @@ function get_assignable_roles_for_switchrole ($context, $field="name") { JOIN {$CFG->prefix}role_capabilities rc ON (r.id = rc.roleid AND rc.capability = 'moodle/course:view' AND rc.capability != 'moodle/site:doanything') - LEFT OUTER JOIN {$CFG->prefix}role_names rn - ON (rn.roleid = r.id AND rn.contextid = $context->id) ORDER BY sortorder ASC"; $rs = get_recordset_sql($sql); @@ -4288,10 +4272,8 @@ function get_assignable_roles_for_switchrole ($context, $field="name") { $roles[$r->id] = $r->{$field}; } rs_close($rs); - foreach ($roles as $roleid => $rolename) { - $roles[$roleid] = strip_tags(format_string($rolename, true)); - } - return $roles; + + return role_fix_names($roles, $context, $rolenamedisplay); } /** @@ -4299,19 +4281,19 @@ function get_assignable_roles_for_switchrole ($context, $field="name") { * @param object $context * @return array */ -function get_overridable_roles($context) { +function get_overridable_roles($context, $field='name', $rolenamedisplay=ROLENAME_ALIAS) { $options = array(); if ($roles = get_all_roles()) { foreach ($roles as $role) { if (user_can_override($context, $role->id)) { - $options[$role->id] = $role->name; + $options[$role->id] = $role->$field; } } } - return role_fix_names($options, $context); + return role_fix_names($options, $context, $rolenamedisplay); } /** @@ -5302,19 +5284,34 @@ function role_get_name($role, $coursecontext) { /** * Prepare list of roles for display, apply aliases and format text * @param array $roleoptions array roleid=>rolename - * @param object $coursecontext + * @param object $context * @return array of role names */ -function role_fix_names($roleoptions, $coursecontext) { - if ($aliasnames = get_records('role_names', 'contextid', $coursecontext->id)) { - foreach ($aliasnames as $alias) { - if (isset($roleoptions[$alias->roleid])) { - $roleoptions[$alias->roleid] = $alias->name; +function role_fix_names($roleoptions, $context, $rolenamedisplay=ROLENAME_ALIAS) { + if ($rolenamedisplay != ROLENAME_ORIGINAL && !empty($context->id)) { + if ($context->contextlevel == CONTEXT_MODULE || $context->contextlevel == CONTEXT_BLOCK) { // find the parent course context + if ($parentcontextid = array_shift(get_parent_contexts($context))) { + $context = get_context_instance_by_id($parentcontextid); + } + } + if ($aliasnames = get_records('role_names', 'contextid', $context->id)) { + if ($rolenamedisplay == ROLENAME_ALIAS) { + foreach ($aliasnames as $alias) { + if (isset($roleoptions[$alias->roleid])) { + $roleoptions[$alias->roleid] = format_string($alias->name); + } + } + } else if ($rolenamedisplay == ROLENAME_BOTH) { + foreach ($aliasnames as $alias) { + if (isset($roleoptions[$alias->roleid])) { + $roleoptions[$alias->roleid] = format_string($alias->name).' ('.format_string($roleoptions[$alias->roleid]).')'; + } + } } } } foreach ($roleoptions as $rid => $name) { - $roleoptions[$rid] = strip_tags(format_string($name)); + $roleoptions[$rid] = strip_tags($name); } return $roleoptions; }