Merge pull request metabase#7574 from metabase/sort-users-list

Sort the /admin/people list by last_name & first_name
yajuve · May 9, 2018 · 36bf50e · 36bf50e
2 parents c488057 + 92e7974
commit 36bf50e
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 6 deletions.
diff --git a/frontend/src/metabase/admin/people/containers/PeopleListingApp.jsx b/frontend/src/metabase/admin/people/containers/PeopleListingApp.jsx
@@ -2,7 +2,6 @@
 import React, { Component } from "react";
 import PropTypes from "prop-types";
 import { Link } from "react-router";
-import _ from "underscore";
 import { connect } from "react-redux";
 
 import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper.jsx";
@@ -30,7 +29,7 @@ export const MODAL_RESET_PASSWORD_EMAIL = "MODAL_RESET_PASSWORD_EMAIL";
 export const MODAL_USER_ADDED_WITH_INVITE = "MODAL_USER_ADDED_WITH_INVITE";
 export const MODAL_USER_ADDED_WITH_PASSWORD = "MODAL_USER_ADDED_WITH_PASSWORD";
 
-import { getUsers, getModal, getGroups } from "../selectors";
+import { getSortedUsers, getModal, getGroups } from "../selectors";
 import {
   createUser,
   deleteUser,
@@ -48,7 +47,7 @@ import {
 
 const mapStateToProps = (state, props) => {
   return {
-    users: getUsers(state, props),
+    users: getSortedUsers(state, props),
     modal: getModal(state, props),
     user: state.currentUser,
     groups: getGroups(state, props),
@@ -402,8 +401,6 @@ export default class PeopleListingApp extends Component {
     let { modal, users, groups } = this.props;
     let { error } = this.state;
 
-    users = _.values(users).sort((a, b) => b.date_joined - a.date_joined);
-
     return (
       <LoadingAndErrorWrapper loading={!users} error={error}>
         {() => (

diff --git a/frontend/src/metabase/admin/people/selectors.js b/frontend/src/metabase/admin/people/selectors.js
@@ -23,3 +23,19 @@ export const getUsers = createSelector(
           .value(),
     })),
 );
+
+// sort the users list by last_name, ignore case or diacritical marks. If last names are the same then compare by first
+// name
+const compareNames = (a, b) =>
+  a.localeCompare(b, undefined, { sensitivty: "base" });
+
+export const getSortedUsers = createSelector(
+  [getUsers],
+  users =>
+    users &&
+    _.values(users).sort(
+      (a, b) =>
+        compareNames(a.last_name, b.last_name) ||
+        compareNames(a.first_name, b.first_name),
+    ),
+);
diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj
@@ -23,7 +23,9 @@
   "Fetch a list of all active `Users` for the admin People page."
   []
   (db/select [User :id :first_name :last_name :email :is_superuser :google_auth :ldap_auth :last_login]
-    :is_active true))
+    :is_active true
+    {:order-by [[:%lower.last_name :asc]
+                [:%lower.first_name :asc]]}))
 
 (defn- reactivate-user! [existing-user first-name last-name]
   (when-not (:is_active existing-user)