From 0ad8fd077c4e0424de29afca4fc45f44d29f7eff Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Thu, 4 May 2023 16:14:41 +0200 Subject: [PATCH 1/2] MDL-78175 course: move role renaming to participants --- .../tests/behat/edit_permissions.feature | 5 +- course/edit.php | 6 -- course/edit_form.php | 16 ----- course/tests/behat/rename_roles.feature | 11 ++- course/tests/behat/role_renaming.feature | 10 +-- enrol/classes/form/renameroles.php | 53 +++++++++++++++ enrol/renameroles.php | 67 +++++++++++++++++++ lang/en/moodle.php | 3 +- .../output/participants_action_bar.php | 5 +- lib/enrollib.php | 18 ++++- lib/tests/behat/behat_navigation.php | 7 ++ 11 files changed, 157 insertions(+), 44 deletions(-) create mode 100644 enrol/classes/form/renameroles.php create mode 100644 enrol/renameroles.php diff --git a/admin/tool/behat/tests/behat/edit_permissions.feature b/admin/tool/behat/tests/behat/edit_permissions.feature index e70832fd6d4..71f789ff59a 100644 --- a/admin/tool/behat/tests/behat/edit_permissions.feature +++ b/admin/tool/behat/tests/behat/edit_permissions.feature @@ -71,13 +71,12 @@ Feature: Edit capabilities @javascript Scenario: Edit permissions escapes role names correctly - When I am on the "C1" "Course" page logged in as "admin" - And I navigate to "Settings" in current page administration + When I am on the "Course 1" "renameroles" page logged in as "admin" And I set the following fields to these values: | Your word for 'Teacher' | Teacher >= editing | | Your word for 'Non-editing teacher' | Teacher < "editing" | | Your word for 'Student' | Studier & 'learner' | - And I press "Save and display" + And I press "Save" And I navigate to course participants Then I should see "Teacher >= editing (Teacher)" in the "Teacher 1" "table_row" And I should see "Teacher < \"editing\" (Non-editing teacher)" in the "Teaching Assistant" "table_row" diff --git a/course/edit.php b/course/edit.php index 93acabafc58..c3b6b2d04d4 100644 --- a/course/edit.php +++ b/course/edit.php @@ -129,12 +129,6 @@ if (!empty($course)) { file_prepare_standard_filemanager($course, 'overviewfiles', $overviewfilesoptions, $coursecontext, 'course', 'overviewfiles', 0); } - // Inject current aliases. - $aliases = $DB->get_records('role_names', array('contextid'=>$coursecontext->id)); - foreach($aliases as $alias) { - $course->{'role_'.$alias->roleid} = $alias->name; - } - // Populate course tags. $course->tags = core_tag_tag::get_item_tags_array('core', 'course', $course->id); diff --git a/course/edit_form.php b/course/edit_form.php index 897ee07b2c7..b42401b6cf9 100644 --- a/course/edit_form.php +++ b/course/edit_form.php @@ -374,22 +374,6 @@ class course_edit_form extends moodleform { $options[0] = get_string('none'); $mform->addElement('select', 'defaultgroupingid', get_string('defaultgrouping', 'group'), $options); - if ((empty($course->id) && guess_if_creator_will_have_course_capability('moodle/course:renameroles', $categorycontext)) - || (!empty($course->id) && has_capability('moodle/course:renameroles', $coursecontext))) { - // Customizable role names in this course. - $mform->addElement('header', 'rolerenaming', get_string('rolerenaming')); - $mform->addHelpButton('rolerenaming', 'rolerenaming'); - - if ($roles = get_all_roles()) { - $roles = role_fix_names($roles, null, ROLENAME_ORIGINAL); - $assignableroles = get_roles_for_contextlevels(CONTEXT_COURSE); - foreach ($roles as $role) { - $mform->addElement('text', 'role_' . $role->id, get_string('yourwordforx', '', $role->localname)); - $mform->setType('role_' . $role->id, PARAM_TEXT); - } - } - } - if (core_tag_tag::is_enabled('core', 'course') && ((empty($course->id) && guess_if_creator_will_have_course_capability('moodle/course:tag', $categorycontext)) || (!empty($course->id) && has_capability('moodle/course:tag', $coursecontext)))) { diff --git a/course/tests/behat/rename_roles.feature b/course/tests/behat/rename_roles.feature index 90503ef8748..c935bcb6b34 100644 --- a/course/tests/behat/rename_roles.feature +++ b/course/tests/behat/rename_roles.feature @@ -19,13 +19,11 @@ Feature: Rename roles within a course | teacher1 | C1 | editingteacher | | teacher2 | C1 | teacher | | student1 | C1 | student | - And I log in as "teacher1" - And I am on "Course 1" course homepage - When I navigate to "Settings" in current page administration + And I am on the "Course 1" "renameroles" page logged in as "teacher1" And I set the following fields to these values: | Your word for 'Non-editing teacher' | Tutor | | Your word for 'Student' | Learner | - And I press "Save and display" + And I press "Save" And I follow "Switch role to..." in the user menu Then "Tutor" "button" should exist And "Learner" "button" should exist @@ -42,12 +40,11 @@ Feature: Rename roles within a course And I press "Enrol users" And the "Assign role" select box should contain "Learner (Student)" And I click on "Cancel" "button" in the "Enrol users" "dialogue" - And I am on "Course 1" course homepage - And I navigate to "Settings" in current page administration + And I am on the "Course 1" "renameroles" page And I set the following fields to these values: | Your word for 'Non-editing teacher' | | | Your word for 'Student' | | - And I press "Save and display" + And I press "Save" And I follow "Switch role to..." in the user menu And I should see "Teacher" And "Student" "button" should exist diff --git a/course/tests/behat/role_renaming.feature b/course/tests/behat/role_renaming.feature index b650a347d84..4e527141864 100644 --- a/course/tests/behat/role_renaming.feature +++ b/course/tests/behat/role_renaming.feature @@ -18,14 +18,11 @@ Feature: Rename roles in a course | teacher1 | C1 | editingteacher | Scenario: Teacher can rename roles - Given I log in as "teacher1" - And I am on "Course 1" course homepage - And I navigate to "Settings" in current page administration - And I should see "Role renaming" + Given I am on the "Course 1" "renameroles" page logged in as "teacher1" When I set the following fields to these values: | Your word for 'Teacher' | Lecturer | | Your word for 'Student' | Learner | - And I press "Save and display" + And I press "Save" And I navigate to course participants Then I should see "Lecturer (Teacher)" in the "Teacher 1" "table_row" And I should see "Learner (Student)" in the "Student 1" "table_row" @@ -46,6 +43,5 @@ Feature: Rename roles in a course And I follow "Log out" When I log in as "teacher1" And I am on "Course 1" course homepage - And I navigate to "Settings" in current page administration + And I navigate to course participants Then I should not see "Role renaming" - And I should not see "Your word for 'Teacher'" diff --git a/enrol/classes/form/renameroles.php b/enrol/classes/form/renameroles.php new file mode 100644 index 00000000000..e93089678a1 --- /dev/null +++ b/enrol/classes/form/renameroles.php @@ -0,0 +1,53 @@ +. + +namespace core_enrol\form; + +/** + * Form to customise the course role names. + * + * @package core_enrol + * @copyright 2023 Ferran Recio + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class renameroles extends \moodleform { + + /** + * Form definition. + */ + public function definition() { + $mform = $this->_form; + + $courseid = $this->_customdata['id']; + $roles = $this->_customdata['roles'] ?? []; + + $formdata = new \stdClass(); + + $mform->addElement('hidden', 'id', $courseid); + $mform->setType('id', PARAM_INT); + + foreach ($roles as $role) { + $settingname = 'role_' . $role->id; + $mform->addElement('text', $settingname, get_string('yourwordforx', '', $role->localname)); + $mform->setType($settingname, PARAM_TEXT); + $formdata->{$settingname} = $role->coursealias; + } + + $mform->addElement('submit', 'submit', get_string('save')); + + $this->set_data($formdata); + } +} diff --git a/enrol/renameroles.php b/enrol/renameroles.php new file mode 100644 index 00000000000..6ba3436a8ba --- /dev/null +++ b/enrol/renameroles.php @@ -0,0 +1,67 @@ +. + +/** + * Customise the course role names. + * + * @package core_enrol + * @copyright 2023 Ferran Recio + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require('../config.php'); +require_once("$CFG->dirroot/course/lib.php"); + + +$id = required_param('id', PARAM_INT); +$action = optional_param('action', '', PARAM_ALPHANUMEXT); +$filter = optional_param('ifilter', 0, PARAM_INT); + +$course = $DB->get_record('course', ['id' => $id], '*', MUST_EXIST); +$context = core\context\course::instance($course->id, MUST_EXIST); + +require_login($course); +require_capability('moodle/course:renameroles', $context); + +if ($course->id == SITEID) { + redirect("$CFG->wwwroot/"); +} + +$PAGE->set_pagelayout('admin'); +$PAGE->set_url('/enrol/renameroles.php', ['id' => $course->id]); +$PAGE->set_pagelayout('standard'); +$PAGE->set_title(get_string('rolerenaming')); +$PAGE->set_heading($course->fullname); + +echo $OUTPUT->header(); +echo $OUTPUT->render_participants_tertiary_nav($course); +echo $OUTPUT->heading(get_string('rolerenaming')); + +echo $OUTPUT->container(get_string('rolerenaming_help')); + +$customdata = [ + 'id' => $course->id, + 'roles' => role_get_names($context, ROLENAME_ORIGINAL), +]; +$mform = new core_enrol\form\renameroles(null, $customdata); +if ($data = $mform->get_data()) { + save_local_role_names($course->id, (array)$data); + core\notification::add(get_string('rolerenaming_success'), core\notification::SUCCESS); +} + +$mform->display(); + +echo $OUTPUT->footer(); diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 02bbcf572b4..0dd4e56b9ea 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -1886,7 +1886,8 @@ $string['role'] = 'Role'; $string['roleassignments'] = 'Role assignments'; $string['rolemappings'] = 'Role mappings'; $string['rolerenaming'] = 'Role renaming'; -$string['rolerenaming_help'] = 'This setting allows the displayed names for roles used in the course to be changed. Only the displayed name is changed - role permissions are not affected. New role names will appear on the course participants page and elsewhere within the course. If the renamed role is one that the administrator has selected as a course manager role, then the new role name will also appear as part of the course listings.'; +$string['rolerenaming_help'] = 'Change the display names of roles in this course.'; +$string['rolerenaming_success'] = 'Course role names updated.'; $string['roles'] = 'Roles'; $string['rss'] = 'RSS'; $string['rssarticles'] = 'Number of RSS recent articles'; diff --git a/lib/classes/output/participants_action_bar.php b/lib/classes/output/participants_action_bar.php index cdbdc091a66..7ed7d8ef9e6 100644 --- a/lib/classes/output/participants_action_bar.php +++ b/lib/classes/output/participants_action_bar.php @@ -68,7 +68,8 @@ class participants_action_bar implements \renderable { return [ 'enrolments:enrol' => [ 'review', - 'manageinstances' + 'manageinstances', + 'renameroles', ], 'groups:group' => [ 'groups' @@ -81,7 +82,7 @@ class participants_action_bar implements \renderable { 'roleoverride', 'rolecheck', 'roleassign', - ] + ], ]; } diff --git a/lib/enrollib.php b/lib/enrollib.php index 611fe09c5d7..a5c74c4046b 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -476,6 +476,17 @@ function enrol_add_course_navigation(navigation_node $coursenode, $course) { if (!$url) { $instancesnode->trim_if_empty(); } + + if (has_capability('moodle/course:renameroles', $coursecontext)) { + $url = new moodle_url('/enrol/renameroles.php', array('id' => $course->id)); + $instancesnode->add( + get_string('rolerenaming'), + $url, + navigation_node::TYPE_SETTING, + null, + 'renameroles' + ); + } } // Manage groups in this course or even frontpage @@ -484,7 +495,10 @@ function enrol_add_course_navigation(navigation_node $coursenode, $course) { $usersnode->add(get_string('groups'), $url, navigation_node::TYPE_SETTING, null, 'groups', new pix_icon('i/group', '')); } - if (has_any_capability(array( 'moodle/role:assign', 'moodle/role:safeoverride','moodle/role:override', 'moodle/role:review'), $coursecontext)) { + if (has_any_capability( + [ 'moodle/role:assign', 'moodle/role:safeoverride', 'moodle/role:override', 'moodle/role:review'], + $coursecontext + )) { // Override roles if (has_capability('moodle/role:review', $coursecontext)) { $url = new moodle_url('/admin/roles/permissions.php', array('contextid'=>$coursecontext->id)); @@ -505,7 +519,7 @@ function enrol_add_course_navigation(navigation_node $coursenode, $course) { $url = new moodle_url('/admin/roles/check.php', array('contextid'=>$coursecontext->id)); $permissionsnode->add(get_string('checkpermissions', 'role'), $url, navigation_node::TYPE_SETTING, null, 'permissions', new pix_icon('i/checkpermissions', '')); } - } + } // Deal somehow with users that are not enrolled but still got a role somehow if ($course->id != SITEID) { diff --git a/lib/tests/behat/behat_navigation.php b/lib/tests/behat/behat_navigation.php index 2e9bd61da2b..e88374efcfa 100644 --- a/lib/tests/behat/behat_navigation.php +++ b/lib/tests/behat/behat_navigation.php @@ -882,6 +882,13 @@ class behat_navigation extends behat_base { $identifier . '" does not exist'); } return new moodle_url('/enrol/otherusers.php', ['id' => $courseid]); + case 'renameroles': + $courseid = $this->get_course_id($identifier); + if (!$courseid) { + throw new Exception('The specified course with shortname, fullname, or idnumber "' . + $identifier . '" does not exist'); + } + return new moodle_url('/enrol/renameroles.php', ['id' => $courseid]); } $parts = explode(' ', $type); From 0eaaf40f102d703cc22e5f749625ffd28a3d2e9b Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Thu, 18 May 2023 12:22:23 +0200 Subject: [PATCH 2/2] MDL-78175 output: new paragraph renderer method Many main php files still uses html_writer('p...) to render a simple paragraphs below or under other elements like forms. In many cases those texts could be part of a template but there are may scenarios were creating a full template for a simple paragraph is too much. However, the html_writer is too limiting as plugins cannot provide their own implementation. This new method in the renderer is a good in between solution. --- enrol/renameroles.php | 2 +- lib/outputrenderers.php | 16 ++++++++++++++++ lib/upgrade.txt | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/enrol/renameroles.php b/enrol/renameroles.php index 6ba3436a8ba..7181d368175 100644 --- a/enrol/renameroles.php +++ b/enrol/renameroles.php @@ -50,7 +50,7 @@ echo $OUTPUT->header(); echo $OUTPUT->render_participants_tertiary_nav($course); echo $OUTPUT->heading(get_string('rolerenaming')); -echo $OUTPUT->container(get_string('rolerenaming_help')); +echo $OUTPUT->paragraph(get_string('rolerenaming_help')); $customdata = [ 'id' => $course->id, diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index bc798b5d91a..d327479c48e 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -3263,6 +3263,22 @@ EOD; return $this->opencontainers->pop('box'); } + /** + * Outputs a paragraph. + * + * @param string $contents The contents of the paragraph + * @param string|null $classes A space-separated list of CSS classes + * @param string|null $id An optional ID + * @return string the HTML to output. + */ + public function paragraph(string $contents, ?string $classes = null, ?string $id = null): string { + return html_writer::tag( + 'p', + $contents, + ['id' => $id, 'class' => renderer_base::prepare_classes($classes)] + ); + } + /** * Outputs a container. * diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 7f6c47eec36..ad1a7299b2a 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -7,6 +7,7 @@ information provided here is intended especially for developers. - admin_setting_requiredpasswordunmask * The badges_get_oauth2_service_options() method has been deprecated, because it's not required anymore. It should no longer be used. +* New core_renderer::paragraph method to replace the overused html_writer::tag('p', ...) pattern. === 4.2 ===