diff --git a/mod/quiz/override_form.php b/mod/quiz/override_form.php index 7561c136358..2036cae81a9 100644 --- a/mod/quiz/override_form.php +++ b/mod/quiz/override_form.php @@ -123,11 +123,12 @@ class quiz_override_form extends moodleform { } } else { // User override. + $extrauserfields = get_extra_user_fields($this->context); if ($this->userid) { // There is already a userid, so freeze the selector. - $user = $DB->get_record('user', array('id'=>$this->userid)); + $user = $DB->get_record('user', ['id' => $this->userid]); $userchoices = array(); - $userchoices[$this->userid] = fullname($user); + $userchoices[$this->userid] = $this->display_user_name($user, $extrauserfields); $mform->addElement('select', 'userid', get_string('overrideuser', 'quiz'), $userchoices); $mform->freeze('userid'); @@ -142,14 +143,13 @@ class quiz_override_form extends moodleform { } // Get the list of appropriate users, depending on whether and how groups are used. + $userfields = user_picture::fields('u', $extrauserfields, 'userid'); if ($accessallgroups) { $users = get_users_by_capability($this->context, 'mod/quiz:attempt', - 'u.id, u.email, ' . get_all_user_name_fields(true, 'u'), - $sort); + $userfields, $sort); } else if ($groups = groups_get_activity_allowed_groups($cm)) { $users = get_users_by_capability($this->context, 'mod/quiz:attempt', - 'u.id, u.email, ' . get_all_user_name_fields(true, 'u'), - $sort, '', '', array_keys($groups)); + $userfields, $sort, '', '', array_keys($groups)); } // Filter users based on any fixed restrictions (groups, profile). @@ -162,17 +162,9 @@ class quiz_override_form extends moodleform { print_error('usersnone', 'quiz', $link); } - $userchoices = array(); - $canviewemail = in_array('email', get_extra_user_fields($this->context)); + $userchoices = []; foreach ($users as $id => $user) { - if (empty($invalidusers[$id]) || (!empty($override) && - $id == $override->userid)) { - if ($canviewemail) { - $userchoices[$id] = fullname($user) . ', ' . $user->email; - } else { - $userchoices[$id] = fullname($user); - } - } + $userchoices[$id] = $this->display_user_name($user, $extrauserfields); } unset($users); @@ -228,7 +220,27 @@ class quiz_override_form extends moodleform { $mform->addGroup($buttonarray, 'buttonbar', '', array(' '), false); $mform->closeHeaderBefore('buttonbar'); + } + /** + * Get a user's name and identity ready to display. + * + * @param stdClass $user a user object. + * @param array $extrauserfields from get_extra_user_fields. + * @return string User's name, with extra info, for display. + */ + protected function display_user_name(stdClass $user, array $extrauserfields) { + $username = fullname($user); + $namefields = []; + foreach ($extrauserfields as $field) { + if (isset($user->$field) && $user->$field !== '') { + $namefields[] = $user->$field; + } + } + if ($namefields) { + $username .= ' (' . implode(', ', $namefields) . ')'; + } + return $username; } public function validation($data, $files) { diff --git a/mod/quiz/overridedelete.php b/mod/quiz/overridedelete.php index c4b35c4bcb3..9da9c8ebcb2 100644 --- a/mod/quiz/overridedelete.php +++ b/mod/quiz/overridedelete.php @@ -92,13 +92,24 @@ echo $OUTPUT->header(); echo $OUTPUT->heading(format_string($quiz->name, true, array('context' => $context))); if ($override->groupid) { - $group = $DB->get_record('groups', array('id' => $override->groupid), 'id, name'); + $group = $DB->get_record('groups', ['id' => $override->groupid], 'id, name'); $confirmstr = get_string("overridedeletegroupsure", "quiz", $group->name); } else { $namefields = get_all_user_name_fields(true); - $user = $DB->get_record('user', array('id' => $override->userid), - 'id, ' . $namefields); - $confirmstr = get_string("overridedeleteusersure", "quiz", fullname($user)); + $user = $DB->get_record('user', ['id' => $override->userid]); + + $username = fullname($user); + $namefields = []; + foreach (get_extra_user_fields($context) as $field) { + if (isset($user->$field) && $user->$field !== '') { + $namefields[] = $user->$field; + } + } + if ($namefields) { + $username .= ' (' . implode(', ', $namefields) . ')'; + } + + $confirmstr = get_string('overridedeleteusersure', 'quiz', $username); } echo $OUTPUT->confirm($confirmstr, $confirmurl, $cancelurl); diff --git a/mod/quiz/overrides.php b/mod/quiz/overrides.php index 30b5fee311c..fafcb4fa308 100644 --- a/mod/quiz/overrides.php +++ b/mod/quiz/overrides.php @@ -45,10 +45,10 @@ if (!$canedit) { } $quizgroupmode = groups_get_activity_groupmode($cm); -$accessallgroups = ($quizgroupmode == NOGROUPS) || has_capability('moodle/site:accessallgroups', $context); +$showallgroups = ($quizgroupmode == NOGROUPS) || has_capability('moodle/site:accessallgroups', $context); // Get the course groups that the current user can access. -$groups = $accessallgroups ? groups_get_all_groups($cm->course) : groups_get_activity_allowed_groups($cm); +$groups = $showallgroups ? groups_get_all_groups($cm->course) : groups_get_activity_allowed_groups($cm); // Default mode is "group", unless there are no groups. if ($mode != "user" and $mode != "group") { @@ -83,6 +83,8 @@ if (!empty($orphaned)) { } $overrides = []; +$colclasses = []; +$headers = []; // Fetch all overrides. if ($groupmode) { @@ -101,46 +103,60 @@ if ($groupmode) { $overrides = $DB->get_records_sql($sql, $params); } + } else { // User overrides. - $colname = get_string('user'); + $colclasses[] = 'colname'; + $headers[] = get_string('user'); + $extrauserfields = get_extra_user_fields($context); + foreach ($extrauserfields as $field) { + $colclasses[] = 'col' . $field; + $headers[] = get_user_field_name($field); + } + list($sort, $params) = users_order_by_sql('u'); $params['quizid'] = $quiz->id; + $userfields = user_picture::fields('u', $extrauserfields, 'userid'); - if ($accessallgroups) { - $sql = 'SELECT o.*, ' . get_all_user_name_fields(true, 'u') . ' - FROM {quiz_overrides} o - JOIN {user} u ON o.userid = u.id - WHERE o.quiz = :quizid - ORDER BY ' . $sort; + if ($showallgroups) { + $groupsjoin = ''; + $groupswhere = ''; - $overrides = $DB->get_records_sql($sql, $params); } else if ($groups) { list($insql, $inparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $groupsjoin = 'JOIN {groups_members} gm ON u.id = gm.userid'; + $groupswhere = ' AND gm.groupid ' . $insql; $params += $inparams; - $sql = 'SELECT o.*, ' . get_all_user_name_fields(true, 'u') . ' - FROM {quiz_overrides} o - JOIN {user} u ON o.userid = u.id - JOIN {groups_members} gm ON u.id = gm.userid - WHERE o.quiz = :quizid AND gm.groupid ' . $insql . ' - ORDER BY ' . $sort; - - $overrides = $DB->get_records_sql($sql, $params); + } else { + // User cannot see any data. + $groupsjoin = ''; + $groupswhere = ' AND 1 = 2'; } + + $overrides = $DB->get_records_sql(" + SELECT o.*, $userfields + FROM {quiz_overrides} o + JOIN {user} u ON o.userid = u.id + $groupsjoin + WHERE o.quiz = :quizid + $groupswhere + ORDER BY $sort + ", $params); } // Initialise table. $table = new html_table(); -$table->headspan = [1, 2, 1]; -$table->colclasses = ['colname', 'colsetting', 'colvalue', 'colaction']; -$table->head = [ - $colname, - get_string('overrides', 'quiz'), -]; -if ($canedit) { - $table->head[] = get_string('action'); -} +$table->colclasses = $colclasses; +$table->colclasses[] = 'colsetting'; +$table->colclasses[] = 'colvalue'; +$table->colclasses[] = 'colaction'; +$table->headspan = array_fill(0, count($headers), 1); +$table->headspan[] = 2; +$table->headspan[] = 1; +$table->head = $headers; +$table->head[] = get_string('overrides', 'quiz'); +$table->head[] = get_string('action'); $userurl = new moodle_url('/user/view.php', []); $groupurl = new moodle_url('/group/overview.php', ['id' => $cm->course]); @@ -203,19 +219,28 @@ foreach ($overrides as $override) { } // Prepare the information about who this override applies to. + $extranamebit = $active ? '' : '*'; + $usercells = []; if ($groupmode) { - $usergroupstr = '' . $override->name . ''; + $groupcell = new html_table_cell(); + $groupcell->rowspan = count($fields); + $groupcell->text = html_writer::link(new moodle_url($groupurl, ['group' => $override->groupid]), + $override->name . $extranamebit); + $usercells[] = $groupcell; } else { - $usergroupstr = '' . fullname($override) . ''; + $usercell = new html_table_cell(); + $usercell->rowspan = count($fields); + $usercell->text = html_writer::link(new moodle_url($groupurl, ['id' => $override->userid]), + fullname($override) . $extranamebit); + $usercells[] = $usercell; + + foreach ($extrauserfields as $field) { + $usercell = new html_table_cell(); + $usercell->rowspan = count($fields); + $usercell->text = $override->$field; + $usercells[] = $usercell; + } } - if (!$active) { - $usergroupstr .= '*'; - } - $usergroupcell = new html_table_cell(); - $usergroupcell->rowspan = count($fields); - $usergroupcell->text = $usergroupstr; // Prepare the actions. if ($canedit) { @@ -250,7 +275,7 @@ foreach ($overrides as $override) { } if ($i == 0) { - $row->cells[] = $usergroupcell; + $row->cells = $usercells; } $labelcell = new html_table_cell(); @@ -302,7 +327,7 @@ if ($canedit) { } else { $users = []; // See if there are any students in the quiz. - if ($accessallgroups) { + if ($showallgroups) { $users = get_users_by_capability($context, 'mod/quiz:attempt', 'u.id'); $nousermessage = get_string('usersnone', 'quiz'); } else if ($groups) { diff --git a/mod/quiz/tests/behat/quiz_user_override.feature b/mod/quiz/tests/behat/quiz_user_override.feature index 5e6ecda87f8..db450d3bf46 100644 --- a/mod/quiz/tests/behat/quiz_user_override.feature +++ b/mod/quiz/tests/behat/quiz_user_override.feature @@ -30,13 +30,13 @@ Feature: Quiz user override And I navigate to "User overrides" in current page administration And I press "Add user override" And I set the following fields to these values: - | Override user | Student1 | - | id_timeclose_enabled | 1 | - | timeclose[day] | 1 | - | timeclose[month] | January | - | timeclose[year] | 2020 | - | timeclose[hour] | 08 | - | timeclose[minute] | 00 | + | Override user | Student One (student1@example.com) | + | id_timeclose_enabled | 1 | + | timeclose[day] | 1 | + | timeclose[month] | January | + | timeclose[year] | 2020 | + | timeclose[hour] | 08 | + | timeclose[minute] | 00 | And I press "Save" Then I should see "Wednesday, 1 January 2020, 8:00" @@ -44,9 +44,11 @@ Feature: Quiz user override And I set the following fields to these values: | timeclose[year] | 2030 | And I press "Save" - And I should see "Tuesday, 1 January 2030, 8:00" + And I should see "Tuesday, 1 January 2030, 8:00" in the "Student One" "table_row" + And I should see "student1@example.com" in the "Student One" "table_row" - And I click on "Delete" "link" + And I click on "Delete" "link" in the "Student One" "table_row" + And I should see "Are you sure you want to delete the override for user Student One (student1@example.com)?" And I press "Continue" And I should not see "Student One" @@ -58,14 +60,33 @@ Feature: Quiz user override When I am on the "Test quiz" "mod_quiz > User overrides" page logged in as "teacher" And I press "Add user override" And I set the following fields to these values: - | Override user | Student1 | - | Attempts allowed | 1 | + | Override user | Student One (student1@example.com) | + | Attempts allowed | 1 | And I press "Save" Then I should see "This override is inactive" And "Edit" "icon" should exist in the "Student One" "table_row" And "copy" "icon" should exist in the "Student One" "table_row" And "Delete" "icon" should exist in the "Student One" "table_row" + @javascript + Scenario: Teacher without 'See full user identity in lists' can see and edit overrides + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:viewuseridentity | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | course | idnumber | visible | + | quiz | Test quiz | C1 | quiz1 | 0 | + When I am on the "Test quiz" "mod_quiz > User overrides" page logged in as "teacher" + And I press "Add user override" + And I set the following fields to these values: + | Override user | Student One | + | Attempts allowed | 1 | + And I press "Save" + And I should not see "student1@example.com" + And "Edit" "icon" should exist in the "Student One" "table_row" + And "copy" "icon" should exist in the "Student One" "table_row" + And "Delete" "icon" should exist in the "Student One" "table_row" + Scenario: A teacher without accessallgroups permission should only be able to add user override for users that he/she shares groups with, when the activity's group mode is to "separate groups" Given the following "groups" exist: @@ -85,8 +106,8 @@ Feature: Quiz user override | quiz | Test quiz | C1 | quiz1 | 1 | When I am on the "Test quiz" "mod_quiz > User overrides" page logged in as "teacher" And I press "Add user override" - Then the "Override user" select box should contain "Student One, student1@example.com" - And the "Override user" select box should not contain "Student Two, student2@example.com" + Then the "Override user" select box should contain "Student One (student1@example.com)" + And the "Override user" select box should not contain "Student Two (student2@example.com)" Scenario: Override user in an activity with group mode set to "separate groups" as a teacher who is not a member in any group, and does not have accessallgroups permission Given the following "groups" exist: