Fix 30727: Sort database tables by name in the data reference view (#30751)

* Clean up table references

* Sort tables

* De-crazy the return chain

* Remove superfluous argument

`index` is not needed prop in `ListItem` as of #30650

* Explicitly say that we pass the `table` to the `ListItem`

* Fix sort order

First sort by table name, then by the table schema!

* Make sure tables are sorted in the test

frontend/src/metabase/reference/databases/TableList.jsx

@@ -3,6 +3,7 @@ import React, { Component } from "react";
 import PropTypes from "prop-types";
 import { connect } from "react-redux";
 import { t } from "ttag";
+import _ from "underscore";
 
 import S from "metabase/components/List/List.css";
 import R from "metabase/reference/Reference.css";
@@ -41,12 +42,12 @@ const mapDispatchToProps = {
   ...metadataActions,
 };
 
-const createListItem = entity => (
+const createListItem = table => (
   <ListItem
-    key={entity.id}
-    name={entity.display_name || entity.name}
-    description={entity.description}
-    url={`/reference/databases/${entity.db_id}/tables/${entity.id}`}
+    key={table.id}
+    name={table.display_name || table.name}
+    description={table.description}
+    url={`/reference/databases/${table.db_id}/tables/${table.id}`}
     icon="table2"
   />
 );
@@ -59,26 +60,24 @@ export const separateTablesBySchema = (
   tables,
   createSchemaSeparator,
   createListItem,
-) =>
-  Object.values(tables)
-    .sort((table1, table2) =>
-      table1.schema_name > table2.schema_name
-        ? 1
-        : table1.schema_name === table2.schema_name
-        ? 0
-        : -1,
-    )
-    .map((table, index, sortedTables) => {
-      if (!table || !table.id || !table.name) {
-        return;
-      }
-      // add schema header for first element and if schema is different from previous
-      const previousTableId = Object.keys(sortedTables)[index - 1];
-      return index === 0 ||
-        sortedTables[previousTableId].schema_name !== table.schema_name
-        ? [createSchemaSeparator(table), createListItem(table, index)]
-        : createListItem(table, index);
-    });
+) => {
+  const sortedTables = _.chain(tables)
+    .sortBy(t => t.name)
+    .sortBy(t => t.schema_name)
+    .value();
+
+  return sortedTables.map((table, index, sortedTables) => {
+    if (!table || !table.id || !table.name) {
+      return;
+    }
+    // add schema header for first element and if schema is different from previous
+    const previousTableId = Object.keys(sortedTables)[index - 1];
+    return index === 0 ||
+      sortedTables[previousTableId].schema_name !== table.schema_name
+      ? [createSchemaSeparator(table), createListItem(table)]
+      : createListItem(table);
+  });
+};
 
 class TableList extends Component {
   static propTypes = {
@@ -100,8 +99,10 @@ class TableList extends Component {
       loading,
     } = this.props;
 
+    const tables = Object.values(entities);
+
     return (
-      <div style={style} className="full">
+      <div style={style} className="full" data-testid="table-list">
         <ReferenceHeader
           name={t`Tables in ${database.name}`}
           type="tables"
@@ -112,21 +113,21 @@ class TableList extends Component {
           error={loadingError}
         >
           {() =>
-            Object.keys(entities).length > 0 ? (
+            tables.length > 0 ? (
               <div className="wrapper wrapper--trim">
                 <List>
                   {!hasSingleSchema
                     ? separateTablesBySchema(
-                        entities,
+                        tables,
                         createSchemaSeparator,
                         createListItem,
                       )
-                    : Object.values(entities).map(
-                        (entity, index) =>
-                          entity &&
-                          entity.id &&
-                          entity.name &&
-                          createListItem(entity, index),
+                    : _.sortBy(tables, "name").map(
+                        table =>
+                          table &&
+                          table.id &&
+                          table.name &&
+                          createListItem(table),
                       )}
                 </List>
               </div>

frontend/src/metabase/reference/utils.unit.spec.js

@@ -59,14 +59,14 @@ describe("Reference utils.js", () => {
   });
 
   describe("tablesToSchemaSeparatedTables()", () => {
-    it("should add schema separator to appropriate locations", () => {
+    it("should add schema separator to appropriate locations and sort tables by name", () => {
       const tables = {
-        1: { id: 1, name: "table1", schema_name: "foo" },
-        2: { id: 2, name: "table2", schema_name: "bar" },
-        3: { id: 3, name: "table3", schema_name: "boo" },
-        4: { id: 4, name: "table4", schema_name: "bar" },
-        5: { id: 5, name: "table5", schema_name: "foo" },
-        6: { id: 6, name: "table6", schema_name: "bar" },
+        1: { id: 1, name: "Toucan", schema_name: "foo" },
+        2: { id: 2, name: "Elephant", schema_name: "bar" },
+        3: { id: 3, name: "Giraffe", schema_name: "boo" },
+        4: { id: 4, name: "Wombat", schema_name: "bar" },
+        5: { id: 5, name: "Anaconda", schema_name: "foo" },
+        6: { id: 6, name: "Buffalo", schema_name: "bar" },
       };
 
       const createSchemaSeparator = table => table.schema_name;
@@ -79,12 +79,12 @@ describe("Reference utils.js", () => {
       );
 
       expect(schemaSeparatedTables).toEqual([
-        ["bar", { id: 2, name: "table2", schema_name: "bar" }],
-        { id: 4, name: "table4", schema_name: "bar" },
-        { id: 6, name: "table6", schema_name: "bar" },
-        ["boo", { id: 3, name: "table3", schema_name: "boo" }],
-        ["foo", { id: 1, name: "table1", schema_name: "foo" }],
-        { id: 5, name: "table5", schema_name: "foo" },
+        ["bar", { id: 6, name: "Buffalo", schema_name: "bar" }],
+        { id: 2, name: "Elephant", schema_name: "bar" },
+        { id: 4, name: "Wombat", schema_name: "bar" },
+        ["boo", { id: 3, name: "Giraffe", schema_name: "boo" }],
+        ["foo", { id: 5, name: "Anaconda", schema_name: "foo" }],
+        { id: 1, name: "Toucan", schema_name: "foo" },
       ]);
     });
   });