MDL-70266 quiz overrides: respect show user identity setting
authorTim Hunt <T.J.Hunt@open.ac.uk>
Fri, 11 Dec 2020 17:57:44 +0000 (17:57 +0000)
committerTim Hunt <T.J.Hunt@open.ac.uk>
Thu, 7 Jan 2021 10:38:39 +0000 (10:38 +0000)
mod/quiz/override_form.php
mod/quiz/overridedelete.php
mod/quiz/overrides.php
mod/quiz/tests/behat/quiz_user_override.feature

index 7561c13..2036cae 100644 (file)
@@ -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) {
index c4b35c4..9da9c8e 100644 (file)
@@ -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);
index 30b5fee..fafcb4f 100644 (file)
@@ -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 = '<a href="' . $groupurl->out(true,
-                        ['group' => $override->groupid]) . '" >' . $override->name . '</a>';
+        $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 = '<a href="' . $userurl->out(true,
-                        ['id' => $override->userid]) . '" >' . fullname($override) . '</a>';
-    }
-    if (!$active) {
-        $usergroupstr .= '*';
+        $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;
+        }
     }
-    $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) {
index 5e6ecda..db450d3 100644 (file)
@@ -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: