Make Browse models table sortable (#42113)

* squash

* Reset change

* Fix name column width

enterprise/frontend/src/metabase-enterprise/content_verification/utils.unit.spec.ts

-import type { CollectionEssentials, SearchResult } from "metabase-types/api";
+import type { CollectionEssentials } from "metabase-types/api";
 import { createMockModelResult } from "metabase-types/api/mocks";
 
 import { availableModelFilters, sortCollectionsByVerification } from "./utils";
@@ -24,7 +24,7 @@ describe("Utilities related to content verification", () => {
     expect(sorted[1].name).toBe("Collection Alpha - unverified");
   });
   it("include a constant that defines a filter for only showing verified models", () => {
-    const models: SearchResult[] = [
+    const models = [
       createMockModelResult({
         name: "A verified model",
         moderated_status: "verified",

frontend/src/metabase-types/api/mocks/search.ts

 import _ from "underscore";
 
 import type {
-  SearchResult,
+  ModelResult,
   SearchResponse,
+  SearchResult,
   SearchScore,
 } from "metabase-types/api";
 
@@ -81,5 +82,7 @@ export const createMockSearchResults = ({
   };
 };
 
-export const createMockModelResult = (model: Partial<SearchResult>) =>
-  createMockSearchResult({ model: "dataset", ...model });
+export const createMockModelResult = (
+  model: Partial<ModelResult>,
+): ModelResult =>
+  createMockSearchResult({ ...model, model: "dataset" }) as ModelResult;

frontend/src/metabase-types/api/search.ts

@@ -139,3 +139,6 @@ export type SearchRequest = {
   collection?: CollectionId;
   namespace?: "snippets";
 } & PaginationRequest;
+
+/** Model retrieved through the search endpoint */
+export type ModelResult = SearchResult<number, "dataset">;

frontend/src/metabase/browse/components/BrowseModels.tsx

@@ -7,6 +7,7 @@ import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
 import { color } from "metabase/lib/colors";
 import { PLUGIN_CONTENT_VERIFICATION } from "metabase/plugins";
 import { Box, Flex, Group, Icon, Stack, Title } from "metabase/ui";
+import type { ModelResult, SearchRequest } from "metabase-types/api";
 
 import { filterModels, type ActualModelFilters } from "../utils";
 
@@ -62,16 +63,20 @@ export const BrowseModels = () => {
 export const BrowseModelsBody = ({
   actualModelFilters,
 }: {
+  /** Mapping of filter names to true if the filter is active
+   * or false if it is inactive */
   actualModelFilters: ActualModelFilters;
 }) => {
-  const { data, error, isLoading } = useSearchQuery({
-    models: ["dataset"],
-    filter_items_in_personal_collection: "exclude",
+  const query: SearchRequest = {
+    models: ["dataset"], // 'model' in the sense of 'type of thing'
     model_ancestors: true,
-  });
+    filter_items_in_personal_collection: "exclude",
+  };
+
+  const { data, error, isLoading } = useSearchQuery(query);
 
   const models = useMemo(() => {
-    const unfilteredModels = data?.data ?? [];
+    const unfilteredModels = (data?.data as ModelResult[]) ?? [];
     const filteredModels = filterModels(
       unfilteredModels || [],
       actualModelFilters,
@@ -85,7 +90,7 @@ export const BrowseModelsBody = ({
       <LoadingAndErrorWrapper
         error={error}
         loading={isLoading}
-        style={{ display: "flex", flex: 1 }}
+        style={{ flex: 1 }}
       />
     );
   }
@@ -94,7 +99,7 @@ export const BrowseModelsBody = ({
     return (
       <Stack spacing="md" mb="lg">
         <ModelExplanationBanner />
-        <ModelsTable items={models} />
+        <ModelsTable models={models} />
       </Stack>
     );
   }

frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.tsx

@@ -16,10 +16,9 @@ import {
   Breadcrumb,
   CollectionBreadcrumbsWrapper,
 } from "./CollectionBreadcrumbsWithTooltip.styled";
+import { pathSeparatorChar } from "./constants";
 import type { RefProp } from "./types";
-import { getBreadcrumbMaxWidths } from "./utils";
-
-const separatorCharacter = "/";
+import { getBreadcrumbMaxWidths, getCollectionPathString } from "./utils";
 
 export const CollectionBreadcrumbsWithTooltip = ({
   collection,
@@ -31,9 +30,7 @@ export const CollectionBreadcrumbsWithTooltip = ({
   const collections = (
     (collection.effective_ancestors as CollectionEssentials[]) || []
   ).concat(collection);
-  const pathString = collections
-    .map(coll => getCollectionName(coll))
-    .join(` ${separatorCharacter} `);
+  const pathString = getCollectionPathString(collection);
   const ellipsifyPath = collections.length > 2;
   const shownCollections = ellipsifyPath
     ? [collections[0], collections[collections.length - 1]]
@@ -149,6 +146,6 @@ Ellipsis.displayName = "Ellipsis";
 
 const PathSeparator = () => (
   <Text color="text-light" mx="xs" py={1}>
-    {separatorCharacter}
+    {pathSeparatorChar}
   </Text>
 );

frontend/src/metabase/browse/components/ModelsTable.tsx

+import { useState } from "react";
 import { t } from "ttag";
 
 import EntityItem from "metabase/components/EntityItem";
+import {
+  SortableColumnHeader,
+  type SortingOptions,
+} from "metabase/components/ItemsTable/BaseItemsTable";
 import {
   ColumnHeader,
   ItemCell,
@@ -10,22 +15,24 @@ import {
   TableColumn,
   TBody,
 } from "metabase/components/ItemsTable/BaseItemsTable.styled";
-import { Columns } from "metabase/components/ItemsTable/Columns";
+import { Columns, SortDirection } from "metabase/components/ItemsTable/Columns";
 import type { ResponsiveProps } from "metabase/components/ItemsTable/utils";
 import { color } from "metabase/lib/colors";
+import { useSelector } from "metabase/lib/redux";
 import * as Urls from "metabase/lib/urls";
+import { getLocale } from "metabase/setup/selectors";
 import { Icon, type IconProps } from "metabase/ui";
-import type { Card, SearchResult } from "metabase-types/api";
+import type { ModelResult } from "metabase-types/api";
 
 import { trackModelClick } from "../analytics";
 import { getCollectionName, getIcon } from "../utils";
 
 import { CollectionBreadcrumbsWithTooltip } from "./CollectionBreadcrumbsWithTooltip";
 import { EllipsifiedWithMarkdown } from "./EllipsifiedWithMarkdown";
-import { getModelDescription } from "./utils";
+import { getModelDescription, sortModels } from "./utils";
 
 export interface ModelsTableProps {
-  items: SearchResult[];
+  models: ModelResult[];
 }
 
 const descriptionProps: ResponsiveProps = {
@@ -38,12 +45,22 @@ const collectionProps: ResponsiveProps = {
   containerName: "ItemsTableContainer",
 };
 
-export const ModelsTable = ({ items }: ModelsTableProps) => {
+export const ModelsTable = ({ models }: ModelsTableProps) => {
+  const locale = useSelector(getLocale);
+  const localeCode: string | undefined = locale?.code;
+
+  const [sortingOptions, setSortingOptions] = useState<SortingOptions>({
+    sort_column: "name",
+    sort_direction: SortDirection.Asc,
+  });
+
+  const sortedModels = sortModels(models, sortingOptions, localeCode);
+
   return (
     <Table>
       <colgroup>
         {/* <col> for Name column */}
-        <TableColumn style={{ width: "10rem" }} />
+        <TableColumn style={{ width: "200px" }} />
 
         {/* <col> for Description column */}
         <TableColumn {...descriptionProps} />
@@ -55,60 +72,66 @@ export const ModelsTable = ({ items }: ModelsTableProps) => {
       </colgroup>
       <thead>
         <tr>
-          <Columns.Name.Header />
+          <Columns.Name.Header
+            sortingOptions={sortingOptions}
+            onSortingOptionsChange={setSortingOptions}
+          />
           <ColumnHeader {...descriptionProps}>{t`Description`}</ColumnHeader>
-          <ColumnHeader {...collectionProps}>{t`Collection`}</ColumnHeader>
+          <SortableColumnHeader
+            name="collection"
+            sortingOptions={sortingOptions}
+            onSortingOptionsChange={setSortingOptions}
+            {...collectionProps}
+          >
+            {t`Collection`}
+          </SortableColumnHeader>
           <Columns.RightEdge.Header />
         </tr>
       </thead>
       <TBody>
-        {items.map((item: SearchResult) => (
-          <TBodyRow item={item} key={`${item.model}-${item.id}`} />
+        {sortedModels.map((model: ModelResult) => (
+          <TBodyRow model={model} key={`${model.model}-${model.id}`} />
         ))}
       </TBody>
     </Table>
   );
 };
 
-const TBodyRow = ({ item }: { item: SearchResult }) => {
-  const icon = getIcon(item);
-  if (item.model === "card") {
-    icon.color = color("text-light");
-  }
-
-  const containerName = `collections-path-for-${item.id}`;
+const TBodyRow = ({ model }: { model: ModelResult }) => {
+  const icon = getIcon(model);
+  const containerName = `collections-path-for-${model.id}`;
 
   return (
     <tr>
       {/* Name */}
       <NameCell
-        item={item}
+        model={model}
         icon={icon}
         onClick={() => {
-          trackModelClick(item.id);
+          trackModelClick(model.id);
         }}
       />
 
       {/* Description */}
       <ItemCell {...descriptionProps}>
         <EllipsifiedWithMarkdown>
-          {getModelDescription(item) || ""}
+          {getModelDescription(model) || ""}
         </EllipsifiedWithMarkdown>
       </ItemCell>
 
       {/* Collection */}
       <ItemCell
         data-testid={`path-for-collection: ${
-          item.collection
-            ? getCollectionName(item.collection)
+          model.collection
+            ? getCollectionName(model.collection)
             : t`Untitled collection`
         }`}
         {...collectionProps}
       >
-        {item.collection && (
+        {model.collection && (
           <CollectionBreadcrumbsWithTooltip
             containerName={containerName}
-            collection={item.collection}
+            collection={model.collection}
           />
         )}
       </ItemCell>
@@ -120,29 +143,27 @@ const TBodyRow = ({ item }: { item: SearchResult }) => {
 };
 
 const NameCell = ({
-  item,
+  model,
   testIdPrefix = "table",
   onClick,
   icon,
 }: {
-  item: SearchResult;
+  model: ModelResult;
   testIdPrefix?: string;
   onClick?: () => void;
   icon: IconProps;
 }) => {
+  const { id, name } = model;
   return (
     <ItemNameCell data-testid={`${testIdPrefix}-name`}>
-      <ItemLink
-        to={Urls.model(item as unknown as Partial<Card>)}
-        onClick={onClick}
-      >
+      <ItemLink to={Urls.model({ id, name })} onClick={onClick}>
         <Icon
           size={16}
           {...icon}
           color={color("brand")}
           style={{ flexShrink: 0 }}
         />
-        <EntityItem.Name name={item.name} variant="list" />
+        <EntityItem.Name name={model.name} variant="list" />
       </ItemLink>
     </ItemNameCell>
   );

frontend/src/metabase/browse/components/constants.tsx

+export const pathSeparatorChar = "/";

frontend/src/metabase/browse/components/utils.tsx

 import { t } from "ttag";
 
-import type { CollectionEssentials, SearchResult } from "metabase-types/api";
+import type { SortingOptions } from "metabase/components/ItemsTable/BaseItemsTable";
+import { SortDirection } from "metabase/components/ItemsTable/Columns";
+import type {
+  CollectionEssentials,
+  ModelResult,
+  SearchResult,
+} from "metabase-types/api";
 
 import { getCollectionName } from "../utils";
 
+import { pathSeparatorChar } from "./constants";
+
 export const getBreadcrumbMaxWidths = (
   collections: CollectionEssentials["effective_ancestors"],
   totalUnitsOfWidthAvailable: number,
@@ -37,3 +45,55 @@ export const getModelDescription = (item: SearchResult) => {
     return item.description;
   }
 };
+
+export const getCollectionPathString = (collection: CollectionEssentials) => {
+  const ancestors: CollectionEssentials[] =
+    collection.effective_ancestors || [];
+  const collections = ancestors.concat(collection);
+  const pathString = collections
+    .map(coll => getCollectionName(coll))
+    .join(` ${pathSeparatorChar} `);
+  return pathString;
+};
+
+const getValueForSorting = (
+  model: ModelResult,
+  sort_column: keyof ModelResult,
+): string => {
+  if (sort_column === "collection") {
+    return getCollectionPathString(model.collection);
+  } else {
+    return model[sort_column];
+  }
+};
+
+export const isValidSortColumn = (
+  sort_column: string,
+): sort_column is keyof ModelResult => {
+  return ["name", "collection"].includes(sort_column);
+};
+
+export const sortModels = (
+  models: ModelResult[],
+  sortingOptions: SortingOptions,
+  localeCode: string = "en",
+) => {
+  const { sort_column, sort_direction } = sortingOptions;
+
+  if (!isValidSortColumn(sort_column)) {
+    console.error("Invalid sort column", sort_column);
+    return models;
+  }
+
+  return [...models].sort((a, b) => {
+    const aValue = getValueForSorting(a, sort_column);
+    const bValue = getValueForSorting(b, sort_column);
+    const [firstValue, secondValue] =
+      sort_direction === SortDirection.Asc
+        ? [aValue, bValue]
+        : [bValue, aValue];
+    return firstValue.localeCompare(secondValue, localeCode, {
+      sensitivity: "base",
+    });
+  });
+};

frontend/src/metabase/browse/components/utils.unit.spec.tsx

+import { SortDirection } from "metabase/components/ItemsTable/Columns";
+import {
+  createMockCollection,
+  createMockModelResult,
+} from "metabase-types/api/mocks";
+
+import { getCollectionPathString, sortModels } from "./utils";
+
+describe("getCollectionPathString", () => {
+  it("should return path for collection without ancestors", () => {
+    const collection = createMockCollection({
+      id: 0,
+      name: "Documents",
+      effective_ancestors: [],
+    });
+    const pathString = getCollectionPathString(collection);
+    expect(pathString).toBe("Documents");
+  });
+
+  it("should return path for collection with multiple ancestors", () => {
+    const ancestors = [
+      createMockCollection({ name: "Home" }),
+      createMockCollection({ name: "User" }),
+      createMockCollection({ name: "Files" }),
+    ];
+    const collection = createMockCollection({
+      name: "Documents",
+      effective_ancestors: ancestors,
+    });
+    const pathString = getCollectionPathString(collection);
+    expect(pathString).toBe("Home / User / Files / Documents");
+  });
+});
+
+describe("sortModels", () => {
+  const mockSearchResults = [
+    createMockModelResult({
+      name: "A",
+      // This model has collection path X / Y / Z
+      collection: createMockCollection({
+        name: "Z",
+        effective_ancestors: [
+          createMockCollection({ name: "X" }),
+          createMockCollection({ name: "Y" }),
+        ],
+      }),
+    }),
+    createMockModelResult({
+      name: "C",
+      collection: createMockCollection({ name: "Z" }),
+    }),
+    createMockModelResult({
+      name: "B",
+      // This model has collection path D / E / F
+      collection: createMockCollection({
+        name: "F",
+        effective_ancestors: [
+          createMockCollection({ name: "D" }),
+          createMockCollection({ name: "E" }),
+        ],
+      }),
+    }),
+  ];
+
+  it("should sort by name in ascending order", () => {
+    const sortingOptions = {
+      sort_column: "name",
+      sort_direction: SortDirection.Asc,
+    };
+    const sorted = sortModels(mockSearchResults, sortingOptions);
+    expect(sorted?.map(model => model.name)).toEqual(["A", "B", "C"]);
+  });
+
+  it("should sort by name in descending order", () => {
+    const sortingOptions = {
+      sort_column: "name",
+      sort_direction: SortDirection.Desc,
+    };
+    const sorted = sortModels(mockSearchResults, sortingOptions);
+    expect(sorted?.map(model => model.name)).toEqual(["C", "B", "A"]);
+  });
+
+  it("should sort by collection path in ascending order", () => {
+    const sortingOptions = {
+      sort_column: "collection",
+      sort_direction: SortDirection.Asc,
+    };
+    const sorted = sortModels(mockSearchResults, sortingOptions);
+    expect(sorted?.map(model => model.name)).toEqual(["B", "A", "C"]);
+  });
+
+  it("should sort by collection path in descending order", () => {
+    const sortingOptions = {
+      sort_column: "collection",
+      sort_direction: SortDirection.Desc,
+    };
+    const sorted = sortModels(mockSearchResults, sortingOptions);
+    expect(sorted?.map(model => model.name)).toEqual(["C", "A", "B"]);
+  });
+});

frontend/src/metabase/browse/constants.ts

 export const RELOAD_INTERVAL = 2000;
-
-export const BROWSE_MODELS_LOCALSTORAGE_KEY = "browseModelsViewPreferences";

frontend/src/metabase/browse/utils.ts

@@ -5,20 +5,11 @@ import _ from "underscore";
 import {
   canonicalCollectionId,
   coerceCollectionId,
-  isInstanceAnalyticsCollection,
   isRootCollection,
-  isValidCollectionId,
 } from "metabase/collections/utils";
 import { entityForObject } from "metabase/lib/schema";
-import { PLUGIN_CONTENT_VERIFICATION } from "metabase/plugins";
 import type { IconName } from "metabase/ui";
-import type {
-  CollectionEssentials,
-  SearchResult,
-  CollectionId,
-} from "metabase-types/api";
-
-import { BROWSE_MODELS_LOCALSTORAGE_KEY } from "./constants";
+import type { CollectionEssentials, ModelResult } from "metabase-types/api";
 
 export const getCollectionName = (collection: CollectionEssentials) => {
   if (isRootCollection(collection)) {
@@ -33,63 +24,10 @@ export const getCollectionIdForSorting = (collection: CollectionEssentials) => {
   return coerceCollectionId(canonicalCollectionId(collection.id));
 };
 
-/** Group models by collection */
-export const groupModels = (
-  models: SearchResult[],
-  locale: string | undefined,
-) => {
-  const groupedModels = _.groupBy(models, model =>
-    getCollectionIdForSorting(model.collection),
-  );
-  const groupsOfModels: SearchResult[][] = Object.values(groupedModels);
-  const sortGroupsByCollection = (a: SearchResult[], b: SearchResult[]) => {
-    const collection1 = a[0].collection;
-    const collection2 = b[0].collection;
-
-    // Sort instance analytics collection to the end
-    const collection1IsInstanceAnalyticsCollection =
-      isInstanceAnalyticsCollection(collection1);
-    const collection2IsInstanceAnalyticsCollection =
-      isInstanceAnalyticsCollection(collection2);
-    if (
-      collection1IsInstanceAnalyticsCollection &&
-      !collection2IsInstanceAnalyticsCollection
-    ) {
-      return 1;
-    }
-    if (
-      collection2IsInstanceAnalyticsCollection &&
-      !collection1IsInstanceAnalyticsCollection
-    ) {
-      return -1;
-    }
-
-    const sortValueFromPlugin =
-      PLUGIN_CONTENT_VERIFICATION.sortCollectionsByVerification(
-        collection1,
-        collection2,
-      );
-    if (sortValueFromPlugin) {
-      return sortValueFromPlugin;
-    }
-
-    const name1 = getCollectionName(collection1);
-    const name2 = getCollectionName(collection2);
-    return name1.localeCompare(name2, locale);
-  };
-  groupsOfModels.sort(sortGroupsByCollection);
-  return groupsOfModels;
-};
-
-export type BrowseTabId = "models" | "databases";
-
-export const isValidBrowseTab = (value: unknown): value is BrowseTabId =>
-  value === "models" || value === "databases";
-
 export type AvailableModelFilters = Record<
   string,
   {
-    predicate: (value: SearchResult) => boolean;
+    predicate: (value: ModelResult) => boolean;
     activeByDefault: boolean;
   }
 >;
@@ -99,43 +37,12 @@ export type ModelFilterControlsProps = {
   setActualModelFilters: Dispatch<SetStateAction<ActualModelFilters>>;
 };
 
-export const sortModels = (
-  a: SearchResult,
-  b: SearchResult,
-  localeCode?: string,
-) => {
-  const sortValueFromPlugin =
-    PLUGIN_CONTENT_VERIFICATION.sortModelsByVerification(a, b);
-  if (sortValueFromPlugin) {
-    return sortValueFromPlugin;
-  }
-
-  if (a.name && !b.name) {
-    return -1;
-  }
-  if (!a.name && !b.name) {
-    return 0;
-  }
-  if (!a.name && b.name) {
-    return 1;
-  }
-  if (a.name && !b.name) {
-    return -1;
-  }
-  if (!a.name && !b.name) {
-    return 0;
-  }
-  const nameA = a.name.toLowerCase();
-  const nameB = b.name.toLowerCase();
-  return nameA.localeCompare(nameB, localeCode);
-};
-
 /** Mapping of filter names to true if the filter is active
  * or false if it is inactive */
 export type ActualModelFilters = Record<string, boolean>;
 
 export const filterModels = (
-  unfilteredModels: SearchResult[],
+  unfilteredModels: ModelResult[],
   actualModelFilters: ActualModelFilters,
   availableModelFilters: AvailableModelFilters,
 ) => {
@@ -149,51 +56,6 @@ export const filterModels = (
   );
 };
 
-type CollectionPrefs = Partial<Record<CollectionId, ModelVisibilityPrefs>>;
-
-type ModelVisibilityPrefs = {
-  expanded: boolean;
-  showAll: boolean;
-};
-
-const isRecordWithCollectionIdKeys = (
-  prefs: unknown,
-): prefs is Record<CollectionId, any> =>
-  !!prefs &&
-  typeof prefs === "object" &&
-  !Array.isArray(prefs) &&
-  Object.keys(prefs).every(isValidCollectionId);
-
-const isValidModelVisibilityPrefs = (
-  value: unknown,
-): value is ModelVisibilityPrefs =>
-  typeof value === "object" &&
-  value !== null &&
-  Object.keys(value).includes("expanded") &&
-  Object.keys(value).includes("showAll") &&
-  Object.values(value).every(_.isBoolean);
-
-const isValidCollectionPrefs = (prefs: unknown): prefs is CollectionPrefs =>
-  isRecordWithCollectionIdKeys(prefs) &&
-  Object.values(prefs).every(isValidModelVisibilityPrefs);
-
-export const getCollectionViewPreferences = (): CollectionPrefs => {
-  try {
-    const collectionPrefs = JSON.parse(
-      localStorage.getItem(BROWSE_MODELS_LOCALSTORAGE_KEY) ?? "{}",
-    );
-
-    if (isValidCollectionPrefs(collectionPrefs)) {
-      return collectionPrefs;
-    }
-
-    return {};
-  } catch (err) {
-    console.error(err);
-    return {};
-  }
-};
-
 export const getIcon = (item: unknown): { name: IconName; color: string } => {
   const entity = entityForObject(item);
   return entity?.objectSelectors?.getIcon?.(item) || { name: "folder" };

frontend/src/metabase/browse/utils.unit.spec.ts

 import { defaultRootCollection } from "metabase/admin/permissions/pages/CollectionPermissionsPage/tests/setup";
-import type { SearchResult } from "metabase-types/api";
+import type { SearchResult, ModelResult } from "metabase-types/api";
 import {
   createMockCollection,
   createMockModelResult,
@@ -16,7 +16,7 @@ const collectionZulu = createMockCollection({ id: 4, name: "Zulu" });
 const collectionAngstrom = createMockCollection({ id: 5, name: "Ångström" });
 const collectionOzgur = createMockCollection({ id: 6, name: "Özgür" });
 
-const mockModels: SearchResult[] = [
+const mockModels: ModelResult[] = [
   {
     id: 0,
     name: "Model 0",

frontend/src/metabase/collections/components/CollectionContent/CollectionContentView.tsx

@@ -18,10 +18,8 @@ import type {
 } from "metabase/collections/types";
 import { isPersonalCollectionChild } from "metabase/collections/utils";
 import { ItemsTable } from "metabase/components/ItemsTable";
-import {
-  Sort,
-  type SortingOptions,
-} from "metabase/components/ItemsTable/BaseItemsTable";
+import type { SortingOptions } from "metabase/components/ItemsTable/BaseItemsTable";
+import { SortDirection } from "metabase/components/ItemsTable/Columns";
 import PaginationControls from "metabase/components/PaginationControls";
 import ItemsDragLayer from "metabase/containers/dnd/ItemsDragLayer";
 import CS from "metabase/css/core/index.css";
@@ -97,7 +95,7 @@ export const CollectionContentView = ({
   const [unpinnedItemsSorting, setUnpinnedItemsSorting] =
     useState<SortingOptions>({
       sort_column: "name",
-      sort_direction: Sort.Asc,
+      sort_direction: SortDirection.Asc,
     });
 
   const [

frontend/src/metabase/components/ItemsTable/BaseItemsTable.tsx

 import {
   useCallback,
+  useMemo,
   type HTMLAttributes,
   type PropsWithChildren,
 } from "react";
@@ -24,12 +25,12 @@ import {
   Table,
   TBody,
 } from "./BaseItemsTable.styled";
-import { Columns } from "./Columns";
+import { Columns, SortDirection } from "./Columns";
 import type { ResponsiveProps } from "./utils";
 
 export type SortingOptions = {
   sort_column: string;
-  sort_direction: "asc" | "desc";
+  sort_direction: SortDirection;
 };
 
 export type SortableColumnHeaderProps = {
@@ -38,36 +39,38 @@ export type SortableColumnHeaderProps = {
   onSortingOptionsChange?: (newSortingOptions: SortingOptions) => void;
 } & PropsWithChildren<Partial<HTMLAttributes<HTMLDivElement>>>;
 
-export enum Sort {
-  Asc = "asc",
-  Desc = "desc",
-}
-
 export const SortableColumnHeader = ({
-  name = "",
-  sortingOptions = {
-    sort_column: "",
-    sort_direction: Sort.Asc,
-  },
+  name,
+  sortingOptions,
   onSortingOptionsChange,
   children,
   hideAtContainerBreakpoint,
   containerName,
   ...props
 }: SortableColumnHeaderProps & ResponsiveProps) => {
-  const isSortable = !!onSortingOptionsChange;
-  const isSortingThisColumn = sortingOptions.sort_column === name;
+  const isSortable = !!onSortingOptionsChange && !!name;
+  const isSortingThisColumn = sortingOptions?.sort_column === name;
   const direction = isSortingThisColumn
-    ? sortingOptions.sort_direction
-    : Sort.Desc;
+    ? sortingOptions?.sort_direction
+    : SortDirection.Desc;
 
-  const onSortingControlClick = () => {
-    const nextDirection = direction === Sort.Asc ? Sort.Desc : Sort.Asc;
-    onSortingOptionsChange?.({
-      sort_column: name,
-      sort_direction: nextDirection,
-    });
-  };
+  const onSortingControlClick = useMemo(() => {
+    if (!isSortable) {
+      return undefined;
+    }
+    const handler = () => {
+      const nextDirection =
+        direction === SortDirection.Asc
+          ? SortDirection.Desc
+          : SortDirection.Asc;
+      const newSortingOptions = {
+        sort_column: name,
+        sort_direction: nextDirection,
+      };
+      onSortingOptionsChange?.(newSortingOptions);
+    };
+    return handler;
+  }, [direction, isSortable, name, onSortingOptionsChange]);
 
   return (
     <ColumnHeader
@@ -84,7 +87,7 @@ export const SortableColumnHeader = ({
         {children}
         {isSortable && (
           <SortingIcon
-            name={direction === Sort.Asc ? "chevronup" : "chevrondown"}
+            name={direction === SortDirection.Asc ? "chevronup" : "chevrondown"}
           />
         )}
       </SortingControlContainer>

frontend/src/metabase/components/ItemsTable/BaseItemsTable.unit.spec.tsx

@@ -14,6 +14,7 @@ import { createMockCollection } from "metabase-types/api/mocks";
 
 import type { BaseItemsTableProps } from "./BaseItemsTable";
 import { BaseItemsTable } from "./BaseItemsTable";
+import { SortDirection } from "./Columns";
 
 const timestamp = "2021-06-03T19:46:52.128";
 
@@ -64,7 +65,10 @@ describe("BaseItemsTable", () => {
         component={() => (
           <BaseItemsTable
             items={items}
-            sortingOptions={{ sort_column: "name", sort_direction: "asc" }}
+            sortingOptions={{
+              sort_column: "name",
+              sort_direction: SortDirection.Asc,
+            }}
             onSortingOptionsChange={jest.fn()}
             {...props}
           />

frontend/src/metabase/components/ItemsTable/Columns.tsx

@@ -290,3 +290,8 @@ const getLastEditedBy = (lastEditInfo?: Edit) => {
   const name = getFullName(lastEditInfo);
   return name || lastEditInfo.email;
 };
+
+export enum SortDirection {
+  Asc = "asc",
+  Desc = "desc",
+}