Skip to content

Commit

Permalink
MDL-71070 reportbuilder: consistent user fullname columns sorting.
Browse files Browse the repository at this point in the history 
Report columns that display the fullname of users should behave
in a consistent manner with current functionality of tables, in
regards to sorting by each individual component of a name (first
name, middle name, surname, etc).

To facilitate this, instead of a hard-coded assumption that such
columns are always named 'fullname', table classes can now define
additional columns containing a users name.
  • Loading branch information
@paulholden
paulholden committed Aug 3, 2021
1 parent 7edcf36 commit a4a3721
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 39 deletions.
88 changes: 49 additions & 39 deletions lib/tablelib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ class flexible_table {
var $baseurl = NULL;
var $request = array();

/** @var string[] Columns that are expected to contain a users fullname. */
protected $userfullnamecolumns = ['fullname'];

/**
* @var bool Whether or not to store table properties in the user_preferences table.
*/
Expand Down Expand Up @@ -599,6 +602,17 @@ public function get_sql_sort() {
return self::construct_order_by($this->get_sort_columns(), $this->column_textsort);
}

/**
* Whether the current table contains any fullname columns
*
* @return bool
*/
private function contains_fullname_columns(): bool {
$fullnamecolumns = array_intersect_key($this->columns, array_flip($this->userfullnamecolumns));

return !empty($fullnamecolumns);
}

/**
* Get the columns to sort by, in the form required by {@link construct_order_by()}.
* @return array column name => SORT_... constant.
Expand All @@ -611,13 +625,11 @@ public function get_sort_columns() {
if (empty($this->prefs['sortby'])) {
return array();
}

foreach ($this->prefs['sortby'] as $column => $notused) {
if (isset($this->columns[$column])) {
continue; // This column is OK.
}
if (in_array($column, \core_user\fields::get_name_fields()) &&
isset($this->columns['fullname'])) {
if (in_array($column, \core_user\fields::get_name_fields()) && $this->contains_fullname_columns()) {
continue; // This column is OK.
}
// This column is not OK.
Expand Down Expand Up @@ -656,7 +668,7 @@ function get_sql_where() {
$conditions = array();
$params = array();

if (isset($this->columns['fullname'])) {
if ($this->contains_fullname_columns()) {
static $i = 0;
$i++;

Expand Down Expand Up @@ -973,8 +985,7 @@ function print_initials_bar() {
$ilast = '';
}

if ((!empty($ifirst) || !empty($ilast) ||$this->use_initials)
&& isset($this->columns['fullname'])) {
if ((!empty($ifirst) || !empty($ilast) || $this->use_initials) && $this->contains_fullname_columns()) {
$prefixfirst = $this->request[TABLE_VAR_IFIRST];
$prefixlast = $this->request[TABLE_VAR_ILAST];
echo $OUTPUT->initials_bar($ifirst, 'firstinitial', get_string('firstname'), $prefixfirst, $this->baseurl);
Expand Down Expand Up @@ -1231,44 +1242,43 @@ function print_headers() {
}
switch ($column) {

case 'fullname':
// Check the full name display for sortable fields.
if (has_capability('moodle/site:viewfullnames', $this->get_context())) {
$nameformat = $CFG->alternativefullnameformat;
} else {
$nameformat = $CFG->fullnamedisplay;
}
case 'userpic':
// do nothing, do not display sortable links
break;

if ($nameformat == 'language') {
$nameformat = get_string('fullnamedisplay');
}
default:

$requirednames = order_in_string(\core_user\fields::get_name_fields(), $nameformat);
if (array_search($column, $this->userfullnamecolumns) !== false) {
// Check the full name display for sortable fields.
if (has_capability('moodle/site:viewfullnames', $this->get_context())) {
$nameformat = $CFG->alternativefullnameformat;
} else {
$nameformat = $CFG->fullnamedisplay;
}

if (!empty($requirednames)) {
if ($this->is_sortable($column)) {
// Done this way for the possibility of more than two sortable full name display fields.
$this->headers[$index] = '';
foreach ($requirednames as $name) {
$sortname = $this->sort_link(get_string($name),
$name, $primarysortcolumn === $name, $primarysortorder);
$this->headers[$index] .= $sortname . ' / ';
}
$helpicon = '';
if (isset($this->helpforheaders[$index])) {
$helpicon = $OUTPUT->render($this->helpforheaders[$index]);
}
$this->headers[$index] = substr($this->headers[$index], 0, -3). $helpicon;
if ($nameformat == 'language') {
$nameformat = get_string('fullnamedisplay');
}
}
break;

case 'userpic':
// do nothing, do not display sortable links
break;
$requirednames = order_in_string(\core_user\fields::get_name_fields(), $nameformat);

default:
if ($this->is_sortable($column)) {
if (!empty($requirednames)) {
if ($this->is_sortable($column)) {
// Done this way for the possibility of more than two sortable full name display fields.
$this->headers[$index] = '';
foreach ($requirednames as $name) {
$sortname = $this->sort_link(get_string($name),
$name, $primarysortcolumn === $name, $primarysortorder);
$this->headers[$index] .= $sortname . ' / ';
}
$helpicon = '';
if (isset($this->helpforheaders[$index])) {
$helpicon = $OUTPUT->render($this->helpforheaders[$index]);
}
$this->headers[$index] = substr($this->headers[$index], 0, -3) . $helpicon;
}
}
} else if ($this->is_sortable($column)) {
$helpicon = '';
if (isset($this->helpforheaders[$index])) {
$helpicon = $OUTPUT->render($this->helpforheaders[$index]);
Expand Down Expand Up @@ -1328,7 +1338,7 @@ protected function set_sorting_preferences(): void {
$isvalidsort = $sortby && $this->is_sortable($sortby);
$isvalidsort = $isvalidsort && empty($this->prefs['collapse'][$sortby]);
$isrealcolumn = isset($this->columns[$sortby]);
$isfullnamefield = isset($this->columns['fullname']) && in_array($sortby, $usernamefields);
$isfullnamefield = $this->contains_fullname_columns() && in_array($sortby, $usernamefields);

return $isvalidsort && ($isrealcolumn || $isfullnamefield);
}, ARRAY_FILTER_USE_KEY);
Expand Down
5 changes: 5 additions & 0 deletions reportbuilder/classes/table/system_report_table.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public function __construct(string $uniqueid, array $parameters = []) {

$columnheaders[$column->get_column_alias()] = $column->get_title();

// Specify whether column should behave as a user fullname column.
if (preg_match('/^user:fullname.*$/', $column->get_unique_identifier())) {
$this->userfullnamecolumns[] = $column->get_column_alias();
}

// Add each columns fields, joins and params to our report.
$fields = array_merge($fields, $column->get_fields());
$joins = array_merge($joins, $column->get_joins());
Expand Down

0 comments on commit a4a3721

Please sign in to comment.