From d29366636e806ce20c09d1516e0d7766c39805f1 Mon Sep 17 00:00:00 2001
From: Aleksandr Lesnenko <alxnddr@users.noreply.github.com>
Date: Tue, 5 Jul 2022 23:11:29 +0400
Subject: [PATCH] Fix saved question picker issues (#23704)
---
.../src/metabase-types/api/mocks/index.ts | 1 +
.../src/metabase-types/api/mocks/table.ts | 14 +++
frontend/src/metabase-types/api/table.ts | 2 +-
.../src/metabase/components/tree/TreeNode.tsx | 2 +-
.../SavedQuestionList.jsx | 37 ++++----
.../SavedQuestionPicker.jsx | 2 +-
.../SavedQuestionPicker.unit.spec.jsx | 95 +++++++++++++++++++
7 files changed, 132 insertions(+), 21 deletions(-)
create mode 100644 frontend/src/metabase-types/api/mocks/table.ts
create mode 100644 frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.unit.spec.jsx
diff --git a/frontend/src/metabase-types/api/mocks/index.ts b/frontend/src/metabase-types/api/mocks/index.ts
index deac792384d..eaa71fb13bc 100644
--- a/frontend/src/metabase-types/api/mocks/index.ts
+++ b/frontend/src/metabase-types/api/mocks/index.ts
@@ -5,6 +5,7 @@ export * from "./dashboard";
export * from "./database";
export * from "./dataset";
export * from "./models";
+export * from "./table";
export * from "./timeline";
export * from "./settings";
export * from "./user";
diff --git a/frontend/src/metabase-types/api/mocks/table.ts b/frontend/src/metabase-types/api/mocks/table.ts
new file mode 100644
index 00000000000..07bb37ae3d3
--- /dev/null
+++ b/frontend/src/metabase-types/api/mocks/table.ts
@@ -0,0 +1,14 @@
+import { Table } from "metabase-types/api";
+
+export const createMockTable = (opts?: Partial<Table>): Table => {
+ return {
+ id: 1,
+ db_id: 1,
+ display_name: "Table",
+ name: "table",
+ schema: "public",
+ description: null,
+ visibility_type: "normal",
+ ...opts,
+ };
+};
diff --git a/frontend/src/metabase-types/api/table.ts b/frontend/src/metabase-types/api/table.ts
index 7dade30cc15..003007194f9 100644
--- a/frontend/src/metabase-types/api/table.ts
+++ b/frontend/src/metabase-types/api/table.ts
@@ -13,7 +13,7 @@ export type VisibilityType =
| "cruft";
export interface Table {
- id: number;
+ id: number | string; // can be string for virtual questions (e.g. "card__17")
db_id: number;
db?: Database;
name: string;
diff --git a/frontend/src/metabase/components/tree/TreeNode.tsx b/frontend/src/metabase/components/tree/TreeNode.tsx
index 78dd543edb8..a61997a4995 100644
--- a/frontend/src/metabase/components/tree/TreeNode.tsx
+++ b/frontend/src/metabase/components/tree/TreeNode.tsx
@@ -75,7 +75,7 @@ const BaseTreeNode = React.memo(
<Icon {...iconProps} />
</IconContainer>
)}
- <NameContainer>{name}</NameContainer>
+ <NameContainer data-testid="tree-item-name">{name}</NameContainer>
</TreeNodeRoot>
);
}),
diff --git a/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionList.jsx b/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionList.jsx
index e956deb7ecc..1ac5e9d4ba1 100644
--- a/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionList.jsx
+++ b/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionList.jsx
@@ -57,24 +57,25 @@ function SavedQuestionList({
: schema.tables;
return (
<React.Fragment>
- {_.sortBy(tables, "display_name").map(t => (
- <SavedQuestionListItem
- id={t.id}
- isSelected={selectedId === t.id}
- key={t.id}
- size="small"
- name={t.display_name}
- icon={{
- name: isDatasets ? "model" : "table2",
- size: 16,
- }}
- onSelect={() => onSelect(t)}
- rightIcon={PLUGIN_MODERATION.getStatusIcon(
- t.moderated_status,
- )}
- />
- ))}
-
+ {tables
+ .sort((a, b) => a.display_name.localeCompare(b.display_name))
+ .map(t => (
+ <SavedQuestionListItem
+ id={t.id}
+ isSelected={selectedId === t.id}
+ key={t.id}
+ size="small"
+ name={t.display_name}
+ icon={{
+ name: isDatasets ? "model" : "table2",
+ size: 16,
+ }}
+ onSelect={() => onSelect(t)}
+ rightIcon={PLUGIN_MODERATION.getStatusIcon(
+ t.moderated_status,
+ )}
+ />
+ ))}
{tables.length === 0 ? emptyState : null}
</React.Fragment>
);
diff --git a/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.jsx b/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.jsx
index 5c6014e7894..885671eff41 100644
--- a/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.jsx
+++ b/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.jsx
@@ -69,8 +69,8 @@ function SavedQuestionPicker({
nonPersonalOrArchivedCollection,
);
- preparedCollections.push(...nonPersonalOrArchivedCollections);
preparedCollections.push(...userPersonalCollections);
+ preparedCollections.push(...nonPersonalOrArchivedCollections);
if (currentUser.is_superuser) {
const otherPersonalCollections = collections.filter(
diff --git a/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.unit.spec.jsx b/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.unit.spec.jsx
new file mode 100644
index 00000000000..947f79694f0
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.unit.spec.jsx
@@ -0,0 +1,95 @@
+import React from "react";
+import xhrMock from "xhr-mock";
+import {
+ renderWithProviders,
+ screen,
+ waitForElementToBeRemoved,
+} from "__support__/ui";
+import {
+ createMockCollection,
+ createMockTable,
+} from "metabase-types/api/mocks";
+import SavedQuestionPicker from "./SavedQuestionPicker";
+
+const CURRENT_USER = {
+ id: 1,
+ personal_collection_id: 222,
+ is_superuser: true,
+};
+
+const COLLECTIONS = {
+ PERSONAL: createMockCollection({
+ id: CURRENT_USER.personal_collection_id,
+ name: "My personal collection",
+ personal_owner_id: CURRENT_USER.id,
+ }),
+ REGULAR: createMockCollection({ id: 1, name: "Regular collection" }),
+};
+
+function mockCollectionTreeEndpoint() {
+ xhrMock.get("/api/collection/tree?tree=true", {
+ body: Object.values(COLLECTIONS),
+ });
+}
+
+function mockCollectionEndpoint() {
+ xhrMock.get("/api/database/-1337/schema/Everything%20else", {
+ body: [
+ createMockTable({
+ id: "card__1",
+ display_name: "B",
+ schema: "Everything else",
+ }),
+ createMockTable({
+ id: "card__2",
+ display_name: "a",
+ schema: "Everything else",
+ }),
+ createMockTable({
+ id: "card__3",
+ display_name: "A",
+ schema: "Everything else",
+ }),
+ ],
+ });
+}
+
+async function setup() {
+ mockCollectionTreeEndpoint();
+ mockCollectionEndpoint();
+ renderWithProviders(
+ <SavedQuestionPicker onSelect={jest.fn()} onBack={jest.fn()} />,
+ );
+ await waitForElementToBeRemoved(() => screen.queryAllByText("Loading..."));
+}
+
+describe("SavedQuestionPicker", () => {
+ beforeEach(() => {
+ xhrMock.setup();
+ window.HTMLElement.prototype.scrollIntoView = jest.fn();
+ });
+
+ afterEach(() => {
+ xhrMock.teardown();
+ });
+
+ it("shows the current user personal collection on the top after the root", async () => {
+ await setup();
+
+ expect(
+ screen.getAllByTestId("tree-item-name").map(node => node.innerHTML),
+ ).toEqual([
+ "Our analytics",
+ "Your personal collection",
+ "Regular collection",
+ ]);
+ });
+
+ it("sorts saved questions case-insensitive (metabase#23693)", async () => {
+ await setup();
+
+ expect(
+ screen.getAllByTestId("option-text").map(node => node.innerHTML),
+ ).toEqual(["a", "A", "B"]);
+ });
+});
--
GitLab