Merge branch 'wip-MDL-46921-master' of git://github.com/abgreeve/moodle

admin/user.php

@@ -46,6 +46,8 @@
 
     $returnurl = new moodle_url('/admin/user.php', array('sort' => $sort, 'dir' => $dir, 'perpage' => $perpage, 'page'=>$page));
 
+    // The $user variable is also used outside of these if statements.
+    $user = null;
     if ($confirmuser and confirm_sesskey()) {
         require_capability('moodle/user:update', $sitecontext);
         if (!$user = $DB->get_record('user', array('id'=>$confirmuser, 'mnethostid'=>$CFG->mnet_localhost_id))) {
@@ -158,8 +160,9 @@
     // Carry on with the user listing
     $context = context_system::instance();
     $extracolumns = get_extra_user_fields($context);
-    $columns = array_merge(array('firstname', 'lastname'), $extracolumns,
+    // Get all user name fields as an array.
-            array('city', 'country', 'lastaccess'));
+    $allusernamefields = get_all_user_name_fields(false, null, null, null, true);
+    $columns = array_merge($allusernamefields, $extracolumns, array('city', 'country', 'lastaccess'));
 
     foreach ($columns as $column) {
         $string[$column] = get_user_field_name($column);
@@ -183,22 +186,28 @@
         $$column = "<a href=\"user.php?sort=$column&amp;dir=$columndir\">".$string[$column]."</a>$columnicon";
     }
 
-    $override = new stdClass();
+    // We need to check that alternativefullnameformat is not set to '' or language.
-    $override->firstname = 'firstname';
+    // We don't need to check the fullnamedisplay setting here as the fullname function call further down has
-    $override->lastname = 'lastname';
+    // the override parameter set to true.
-    $fullnamelanguage = get_string('fullnamedisplay', '', $override);
+    $fullnamesetting = $CFG->alternativefullnameformat;
-    if (($CFG->fullnamedisplay == 'firstname lastname') or
+    // If we are using language or it is empty, then retrieve all of the user's names.
-        ($CFG->fullnamedisplay == 'firstname') or
+    if ($fullnamesetting == 'language' || empty($fullnamesetting)) {
-        ($CFG->fullnamedisplay == 'language' and $fullnamelanguage == 'firstname lastname' )) {
+        $fullnamesetting = implode(' ', $allusernamefields);
-        $fullnamedisplay = "$firstname / $lastname";
+    }
-        if ($sort == "name") { // If sort has already been set to something else then ignore.
+
-            $sort = "firstname";
+    // Order in string will ensure that the name columns are in the correct order.
-        }
+    $usernames = order_in_string($allusernamefields, $fullnamesetting);
-    } else { // ($CFG->fullnamedisplay == 'language' and $fullnamelanguage == 'lastname firstname').
+    $fullnamedisplay = array();
-        $fullnamedisplay = "$lastname / $firstname";
+    foreach ($usernames as $name) {
-        if ($sort == "name") { // This should give the desired sorting based on fullnamedisplay.
+        // Use the link from $$column for sorting on the user's name.
-            $sort = "lastname";
+        $fullnamedisplay[] = ${$name};
-        }
+    }
+    // All of the names are in one column. Put them into a string and separate them with a /.
+    $fullnamedisplay = implode(' / ', $fullnamedisplay);
+    // If $sort = name then it is the default for the setting and we should use the first name to sort by.
+    if ($sort == "name") {
+        // Use the first item in the array.
+        $sort = reset($usernames);
     }
 
     list($extrasql, $params) = $ufiltering->get_sql_filter();

enrol/locallib.php

@@ -243,16 +243,8 @@ class course_enrolment_manager {
                       JOIN {enrol} e ON (e.id = ue.enrolid)
                  LEFT JOIN {user_lastaccess} ul ON (ul.courseid = e.courseid AND ul.userid = u.id)
                  LEFT JOIN {groups_members} gm ON u.id = gm.userid
-                     WHERE $filtersql";
+                     WHERE $filtersql
-            if ($sort === 'firstname') {
+                  ORDER BY u.$sort $direction";
-                $sql .= " ORDER BY u.firstname $direction, u.lastname $direction";
-            } else if ($sort === 'lastname') {
-                $sql .= " ORDER BY u.lastname $direction, u.firstname $direction";
-            } else if ($sort === 'email') {
-                $sql .= " ORDER BY u.email $direction, u.lastname $direction, u.firstname $direction";
-            } else if ($sort === 'lastseen') {
-                $sql .= " ORDER BY ul.timeaccess $direction, u.lastname $direction, u.firstname $direction";
-            }
             $this->users[$key] = $DB->get_records_sql($sql, $params, $page*$perpage, $perpage);
         }
         return $this->users[$key];

enrol/renderer.php

@@ -420,8 +420,8 @@ class course_enrolment_table extends html_table implements renderable {
      * @static
      * @var array
      */
-    protected static $sortablefields = array('firstname', 'lastname', 'idnumber', 'email',
+    protected static $sortablefields = array('firstname', 'lastname', 'firstnamephonetic', 'lastnamephonetic', 'middlename',
-            'phone1', 'phone2', 'institution', 'department' );
+            'alternatename', 'idnumber', 'email', 'phone1', 'phone2', 'institution', 'department' );
 
     /**
      * Constructs the table
@@ -709,7 +709,8 @@ class course_enrolment_users_table extends course_enrolment_table {
      * @static
      * @var array
      */
-    protected static $sortablefields = array('firstname', 'lastname', 'email', 'lastaccess');
+    protected static $sortablefields = array('firstname', 'lastname', 'firstnamephonetic', 'lastnamephonetic', 'middlename',
+            'alternatename', 'email', 'lastaccess');
 }
 
 /**

enrol/users.php

@@ -174,11 +174,25 @@ if ($action) {
 
 
 $renderer = $PAGE->get_renderer('core_enrol');
-$userdetails = array (
+$userdetails = array('picture' => false);
-    'picture' => false,
+// Get all the user names in a reasonable default order.
-    'firstname' => get_string('firstname'),
+$allusernames = get_all_user_name_fields(false, null, null, null, true);
-    'lastname' => get_string('lastname'),
+// Initialise the variable for the user's names in the table header.
-);
+$usernameheader = null;
+// Get the alternative full name format for users with the viewfullnames capability.
+$fullusernames = $CFG->alternativefullnameformat;
+// If fullusernames is empty or accidentally set to language then fall back on the $allusernames set up.
+if ($fullusernames == 'language' || empty($fullusernames)) {
+    $usernameheader = $allusernames;
+} else {
+    // If everything is as expected then put them in the order specified by the alternative full name format setting.
+    $usernameheader = order_in_string($allusernames, $fullusernames);
+}
+
+// Loop through each name and return the language string.
+foreach ($usernameheader as $key => $username) {
+    $userdetails[$username] = get_string($username);
+}
 $extrafields = get_extra_user_fields($context);
 foreach ($extrafields as $field) {
     $userdetails[$field] = get_user_field_name($field);

lib/moodlelib.php

@@ -3672,9 +3672,12 @@ function fullname($user, $override=false) {
  * @param string $tableprefix table query prefix to use in front of each field.
  * @param string $prefix prefix added to the name fields e.g. authorfirstname.
  * @param string $fieldprefix sql field prefix e.g. id AS userid.
+ * @param bool $order moves firstname and lastname to the top of the array / start of the string.
  * @return array|string All name fields.
  */
-function get_all_user_name_fields($returnsql = false, $tableprefix = null, $prefix = null, $fieldprefix = null) {
+function get_all_user_name_fields($returnsql = false, $tableprefix = null, $prefix = null, $fieldprefix = null, $order = false) {
+    // This array is provided in this order because when called by fullname() (above) if firstname is before
+    // firstnamephonetic str_replace() will change the wrong placeholder.
     $alternatenames = array('firstnamephonetic' => 'firstnamephonetic',
                             'lastnamephonetic' => 'lastnamephonetic',
                             'middlename' => 'middlename',
@@ -3689,6 +3692,19 @@ function get_all_user_name_fields($returnsql = false, $tableprefix = null, $pref
         }
     }
 
+    // If we want the end result to have firstname and lastname at the front / top of the result.
+    if ($order) {
+        // Move the last two elements (firstname, lastname) off the array and put them at the top.
+        for ($i = 0; $i < 2; $i++) {
+            // Get the last element.
+            $lastelement = end($alternatenames);
+            // Remove it from the array.
+            unset($alternatenames[$lastelement]);
+            // Put the element back on the top of the array.
+            $alternatenames = array_merge(array($lastelement => $lastelement), $alternatenames);
+        }
+    }
+
     // Create an sql field snippet if requested.
     if ($returnsql) {
         if ($tableprefix) {

lib/tablelib.php

@@ -448,7 +448,8 @@ class flexible_table {
 
         if (($sortcol = optional_param($this->request[TABLE_VAR_SORT], '', PARAM_ALPHANUMEXT)) &&
                 $this->is_sortable($sortcol) && empty($this->sess->collapse[$sortcol]) &&
-                (isset($this->columns[$sortcol]) || in_array($sortcol, array('firstname', 'lastname')) && isset($this->columns['fullname']))) {
+                (isset($this->columns[$sortcol]) || in_array($sortcol, get_all_user_name_fields())
+                && isset($this->columns['fullname']))) {
 
             if (array_key_exists($sortcol, $this->sess->sortby)) {
                 // This key already exists somewhere. Change its sortorder and bring it to the top.
@@ -571,7 +572,7 @@ class flexible_table {
             if (isset($this->columns[$column])) {
                 continue; // This column is OK.
             }
-            if (in_array($column, array('firstname', 'lastname')) &&
+            if (in_array($column, get_all_user_name_fields()) &&
                     isset($this->columns['fullname'])) {
                 continue; // This column is OK.
             }
@@ -1186,7 +1187,7 @@ class flexible_table {
                 if ($nameformat == 'language') {
                     $nameformat = get_string('fullnamedisplay');
                 }
-                $requirednames = order_in_string(array('firstname', 'lastname'), $nameformat);
+                $requirednames = order_in_string(get_all_user_name_fields(), $nameformat);
 
                 if (!empty($requirednames)) {
                     if ($this->is_sortable($column)) {

lib/tests/moodlelib_test.php

@@ -2486,6 +2486,21 @@ class core_moodlelib_testcase extends advanced_testcase {
         // Additional name fields with an alias and a title - string.
         $teststring = 'u.firstnamephonetic AS authorfirstnamephonetic,u.lastnamephonetic AS authorlastnamephonetic,u.middlename AS authormiddlename,u.alternatename AS authoralternatename,u.firstname AS authorfirstname,u.lastname AS authorlastname';
         $this->assertEquals($teststring, get_all_user_name_fields(true, 'u', null, 'author'));
+
+        // Test the order parameter of the function.
+        // Returning an array.
+        $testarray = array('firstname' => 'firstname',
+                'lastname' => 'lastname',
+                'firstnamephonetic' => 'firstnamephonetic',
+                'lastnamephonetic' => 'lastnamephonetic',
+                'middlename' => 'middlename',
+                'alternatename' => 'alternatename'
+        );
+        $this->assertEquals($testarray, get_all_user_name_fields(false, null, null, null, true));
+
+        // Returning a string.
+        $teststring = 'firstname,lastname,firstnamephonetic,lastnamephonetic,middlename,alternatename';
+        $this->assertEquals($teststring, get_all_user_name_fields(true, null, null, null, true));
     }
 
     public function test_order_in_string() {

user/index.php

@@ -410,11 +410,9 @@ list($esql, $params) = get_enrolled_sql($context, null, $currentgroup, true);
 $joins = array("FROM {user} u");
 $wheres = array();
 
-$extrasql = get_extra_user_fields_sql($context, 'u', '', array(
-        'id', 'username', 'firstname', 'lastname', 'email', 'city', 'country',
-        'picture', 'lang', 'timezone', 'maildisplay', 'imagealt', 'lastaccess'));
-
 $mainuserfields = user_picture::fields('u', array('username', 'email', 'city', 'country', 'lang', 'timezone', 'maildisplay'));
+$alreadyretrievedfields = explode(',', $mainuserfields);
+$extrasql = get_extra_user_fields_sql($context, 'u', '', $alreadyretrievedfields);
 
 if ($isfrontpage) {
     $select = "SELECT $mainuserfields, u.lastaccess$extrasql";

user/tests/behat/table_sorting.feature

@@ -0,0 +1,55 @@
+@core
+Feature: Tables can be sorted by additional names
+  In order to sort fields by additional names
+  As a user
+  I need to browse to a page with users in a table.
+
+  Background:
+    Given the following "users" exist:
+    | username | firstname | lastname | middlename | alternatename | email | idnumber |
+    | student1 | Annie | Edison | Faith | Anne | student1@mail.com | s1 |
+    | student2 | George | Bradley | David | gman | student2@mail.com | s2 |
+    | student3 | Travis | Sutcliff | Peter | Mr T | student3@mail.com | s3 |
+    And I log in as "admin"
+    And I navigate to "User policies" node in "Site administration > Users > Permissions"
+    And I set the following administration settings values:
+    | Full name format | firstname middlename lastname |
+    | Alternative full name format | firstname middlename alternatename lastname |
+
+  @javascript
+  Scenario: All user names are show and sortable in the administration user list.
+    Given I navigate to "Browse list of users" node in "Site administration > Users > Accounts"
+    # And I should see "First name / Middle name / Alternate name / Surname"
+    Then the following should exist in the "users" table:
+    | First name / Middle name / Alternate name / Surname | Email address |
+    | Admin User | moodle@moodlemoodle.com |
+    | Annie Faith Anne Edison | student1@mail.com |
+    | George David gman Bradley | student2@mail.com |
+    | Travis Peter Mr T Sutcliff | student3@mail.com |
+    And "Admin User" "table_row" should appear before "Annie Faith Anne Edison" "table_row"
+    And "Annie Faith Anne Edison" "table_row" should appear before "George David gman Bradley" "table_row"
+    And "George David gman Bradley" "table_row" should appear before "Travis Peter Mr T Sutcliff" "table_row"
+    And I follow "Middle name"
+    And "George David gman Bradley" "table_row" should appear before "Annie Faith Anne Edison" "table_row"
+    And "Annie Faith Anne Edison" "table_row" should appear before "Travis Peter Mr T Sutcliff" "table_row"
+    And "Travis Peter Mr T Sutcliff" "table_row" should appear before "Admin User" "table_row"
+    And I follow "Middle name"
+    And "George David gman Bradley" "table_row" should appear after "Annie Faith Anne Edison" "table_row"
+    And "Annie Faith Anne Edison" "table_row" should appear after "Travis Peter Mr T Sutcliff" "table_row"
+    And "Travis Peter Mr T Sutcliff" "table_row" should appear after "Admin User" "table_row"
+    And I follow "Alternate name"
+    And "Annie Faith Anne Edison" "table_row" should appear before "George David gman Bradley" "table_row"
+    And "George David gman Bradley" "table_row" should appear before "Travis Peter Mr T Sutcliff" "table_row"
+    And "Travis Peter Mr T Sutcliff" "table_row" should appear before "Admin User" "table_row"
+    And I follow "Alternate name"
+    And "Annie Faith Anne Edison" "table_row" should appear after "George David gman Bradley" "table_row"
+    And "George David gman Bradley" "table_row" should appear after "Travis Peter Mr T Sutcliff" "table_row"
+    And "Travis Peter Mr T Sutcliff" "table_row" should appear after "Admin User" "table_row"
+    And I follow "Surname"
+    And "George David gman Bradley" "table_row" should appear before "Annie Faith Anne Edison" "table_row"
+    And "Annie Faith Anne Edison" "table_row" should appear before "Travis Peter Mr T Sutcliff" "table_row"
+    And "Travis Peter Mr T Sutcliff" "table_row" should appear before "Admin User" "table_row"
+    And I follow "Surname"
+    And "George David gman Bradley" "table_row" should appear after "Annie Faith Anne Edison" "table_row"
+    And "Annie Faith Anne Edison" "table_row" should appear after "Travis Peter Mr T Sutcliff" "table_row"
+    And "Travis Peter Mr T Sutcliff" "table_row" should appear after "Admin User" "table_row"