Skip to content
Snippets Groups Projects
Unverified Commit ae9d22cb authored by Kamil Mielnik's avatar Kamil Mielnik Committed by GitHub
Browse files

Show new syncing tables as greyed out in data reference pane (#33076)

* Use MetabaseApi where possible in entities/tables.js

* Mock syncing state

* Disable table link if sync is not complete

* Disable model link if sync is not complete

* Gray out the disabled link by inheriting body color

* Revert "Disable model link if sync is not complete"

This reverts commit 5423834bc6638638f0e3c2d2b51d044e099bea62.

* Revert "Mock syncing state"

This reverts commit 4546f7655e95310a59bb21f3022a8ebad8479e23.

* Export DatabaseTablesPaneProps

* Use SearchResult type instead of any

* Add a unit test boilerplate

* Implement test cases for various initial_sync_status for tables

* Update names

* Add explanatory comments and functions
parent 11f8b036
No related branches found
No related tags found
No related merge requests found
......@@ -21,7 +21,7 @@ import Metrics from "metabase/entities/metrics";
import Segments from "metabase/entities/segments";
import Questions from "metabase/entities/questions";
import { GET, PUT } from "metabase/lib/api";
import { PUT } from "metabase/lib/api";
import {
getMetadata,
getMetadataUnfiltered,
......@@ -31,11 +31,9 @@ import {
getQuestionVirtualTableId,
} from "metabase-lib/metadata/utils/saved-questions";
const listTables = GET("/api/table");
const listTablesForDatabase = async (...args) =>
// HACK: no /api/database/:dbId/tables endpoint
(await GET("/api/database/:dbId/metadata")(...args)).tables;
const listTablesForSchema = GET("/api/database/:dbId/schema/:schemaName");
(await MetabaseApi.db_metadata(...args)).tables;
const updateFieldOrder = PUT("/api/table/:id/fields/order");
const updateTables = PUT("/api/table");
......@@ -57,11 +55,11 @@ const Tables = createEntity({
api: {
list: async (params, ...args) => {
if (params.dbId != null && params.schemaName != null) {
return listTablesForSchema(params, ...args);
return MetabaseApi.db_schema_tables(params, ...args);
} else if (params.dbId != null) {
return listTablesForDatabase(params, ...args);
} else {
return listTables(params, ...args);
return MetabaseApi.table_list(params, ...args);
}
},
},
......
......@@ -2,6 +2,7 @@ import { useMemo } from "react";
import { ngettext, msgid } from "ttag";
import _ from "underscore";
import { SearchResult } from "metabase-types/api";
import Search from "metabase/entities/search";
import SidebarContent from "metabase/query_builder/components/SidebarContent";
import type { State } from "metabase-types/store";
......@@ -18,15 +19,15 @@ import {
} from "./NodeList.styled";
import { PaneContent } from "./Pane.styled";
interface DatabaseTablesPaneProps {
export interface DatabaseTablesPaneProps {
onBack: () => void;
onClose: () => void;
onItemClick: (type: string, item: unknown) => void;
database: Database;
searchResults: any[]; // TODO: /api/search is yet to be typed
searchResults: SearchResult[];
}
const DatabaseTablesPane = ({
export const DatabaseTablesPane = ({
database,
onItemClick,
searchResults,
......@@ -97,7 +98,10 @@ const DatabaseTablesPane = ({
<ul>
{tables.map(table => (
<li key={table.id}>
<NodeListItemLink onClick={() => onItemClick("table", table)}>
<NodeListItemLink
disabled={table.initial_sync_status !== "complete"}
onClick={() => onItemClick("table", table)}
>
<NodeListItemIcon name="table" />
<NodeListItemName>{table.table_name}</NodeListItemName>
</NodeListItemLink>
......
import userEvent from "@testing-library/user-event";
import { createMockMetadata } from "__support__/metadata";
import { renderWithProviders, screen } from "__support__/ui";
import { getNextId } from "__support__/utils";
import {
createMockDatabase,
createMockSearchResult,
} from "metabase-types/api/mocks";
import { checkNotNull } from "metabase/core/utils/types";
import type { DatabaseTablesPaneProps } from "./DatabaseTablesPane";
import { DatabaseTablesPane } from "./DatabaseTablesPane";
const database = createMockDatabase();
const metadata = createMockMetadata({
databases: [database],
});
const incompleteTableSearchResult = createMockSearchResult({
id: getNextId(),
table_name: "Incomplete result",
model: "table",
initial_sync_status: "incomplete",
});
const abortedTableSearchResult = createMockSearchResult({
id: getNextId(),
table_name: "Aborted result",
model: "table",
initial_sync_status: "aborted",
});
const completeTableSearchResult = createMockSearchResult({
id: getNextId(),
table_name: "Complete result",
model: "table",
initial_sync_status: "complete",
});
const defaultProps = {
database: checkNotNull(metadata.database(database.id)),
searchResults: [],
onBack: jest.fn(),
onClose: jest.fn(),
onItemClick: jest.fn(),
};
const setup = (options?: Partial<DatabaseTablesPaneProps>) => {
return renderWithProviders(
<DatabaseTablesPane {...defaultProps} {...options} />,
);
};
describe("DatabaseTablesPane", () => {
it("should show tables with initial_sync_status='incomplete' as non-interactive (disabled)", () => {
setup({
searchResults: [incompleteTableSearchResult],
});
const textElement = screen.getByText(
checkNotNull(incompleteTableSearchResult.table_name),
);
expect(textElement).toBeInTheDocument();
expectToBeDisabled(textElement);
});
it("should show tables with initial_sync_status='aborted' as non-interactive (disabled)", () => {
setup({
searchResults: [abortedTableSearchResult],
});
const textElement = screen.getByText(
checkNotNull(abortedTableSearchResult.table_name),
);
expect(textElement).toBeInTheDocument();
expectToBeDisabled(textElement);
});
it("should show tables with initial_sync_status='complete' as interactive (enabled)", () => {
setup({
searchResults: [completeTableSearchResult],
});
const textElement = screen.getByText(
checkNotNull(completeTableSearchResult.table_name),
);
expect(textElement).toBeInTheDocument();
expectToBeEnabled(textElement);
});
});
/**
* We're dealing with <a> here, which are presented as disabled thanks to:
* - not having "href" attribute
* - using "pointer-events: none"
*
* Due to this "expect().toBeDisabled()" and "expect().toBeEnabled()" won't work as expected.
*
* Clicking the element allows us to detect interactiveness (being enabled/disabled) with certainty.
*/
function expectToBeDisabled(element: Element) {
expect(() => {
userEvent.click(element);
}).toThrow();
}
/**
* @see expectToBeDisabled
*/
function expectToBeEnabled(element: Element) {
expect(() => {
userEvent.click(element);
}).not.toThrow();
}
import { css } from "@emotion/react";
import styled from "@emotion/styled";
import { Icon } from "metabase/core/components/Icon";
......@@ -20,7 +21,11 @@ export const NodeListItemIcon = styled(Icon)`
width: ${space(2)};
`;
export const NodeListItemLink = styled.a`
interface NodeListItemLinkProps {
disabled?: boolean;
}
export const NodeListItemLink = styled.a<NodeListItemLinkProps>`
border-radius: 8px;
display: flex;
align-items: center;
......@@ -35,6 +40,18 @@ export const NodeListItemLink = styled.a`
:hover {
background-color: ${color("bg-medium")};
}
${props =>
props.disabled &&
css`
pointer-events: none;
opacity: 0.4;
color: inherit;
${NodeListItemIcon} {
color: inherit;
}
`};
`;
export const NodeListContainer = styled.ul`
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment