Skip to content
Snippets Groups Projects
Unverified Commit a5dd4444 authored by Alexander Polyankin's avatar Alexander Polyankin Committed by GitHub
Browse files

Remove unused code from QuestionDataSource (#37691)

parent 1b80c36f
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx
+ 15
− 65
View file @ a5dd4444
......@@ -12,10 +12,9 @@ import Tooltip from "metabase/core/components/Tooltip";
import TableInfoPopover from "metabase/components/MetadataInfo/TableInfoPopover";
import {
isVirtualCardId,
getQuestionIdFromVirtualTableId,
isVirtualCardId,
} from "metabase-lib/metadata/utils/saved-questions";
import * as ML_Urls from "metabase-lib/urls";
import { HeadBreadcrumbs } from "./HeaderBreadcrumbs";
import { TablesDivider } from "./QuestionDataSource.styled";
......@@ -23,8 +22,6 @@ import { TablesDivider } from "./QuestionDataSource.styled";
QuestionDataSource.propTypes = {
question: PropTypes.object,
originalQuestion: PropTypes.object,
subHead: PropTypes.bool,
isObjectDetail: PropTypes.bool,
};
function isMaybeBasedOnDataset(question) {
......@@ -33,17 +30,13 @@ function isMaybeBasedOnDataset(question) {
return isVirtualCardId(sourceTableId);
}
function QuestionDataSource({ question, originalQuestion, subHead, ...props }) {
function QuestionDataSource({ question, originalQuestion, ...props }) {
if (!question) {
return null;
}
const variant = subHead ? "subhead" : "head";
if (!question.isStructured() || !isMaybeBasedOnDataset(question)) {
return (
<DataSourceCrumbs question={question} variant={variant} {...props} />
);
return <DataSourceCrumbs question={question} {...props} />;
}
const query = question.query();
......@@ -51,13 +44,7 @@ function QuestionDataSource({ question, originalQuestion, subHead, ...props }) {
const sourceQuestionId = getQuestionIdFromVirtualTableId(sourceTableId);
if (originalQuestion?.id() === sourceQuestionId) {
return (
<SourceDatasetBreadcrumbs
model={originalQuestion}
variant={variant}
{...props}
/>
);
return <SourceDatasetBreadcrumbs model={originalQuestion} {...props} />;
}
return (
......@@ -76,18 +63,11 @@ function QuestionDataSource({ question, originalQuestion, subHead, ...props }) {
<SourceDatasetBreadcrumbs
model={sourceQuestion}
collection={collection}
variant={variant}
{...props}
/>
);
}
return (
<DataSourceCrumbs
question={question}
variant={variant}
{...props}
/>
);
return <DataSourceCrumbs question={question} {...props} />;
}}
</Collections.Loader>
)}
......@@ -97,17 +77,13 @@ function QuestionDataSource({ question, originalQuestion, subHead, ...props }) {
DataSourceCrumbs.propTypes = {
question: PropTypes.object,
variant: PropTypes.oneOf(["head", "subhead"]),
isObjectDetail: PropTypes.bool,
};
function DataSourceCrumbs({ question, variant, isObjectDetail, ...props }) {
function DataSourceCrumbs({ question, ...props }) {
const parts = getDataSourceParts({
question,
subHead: variant === "subhead",
isObjectDetail,
});
return <HeadBreadcrumbs parts={parts} variant={variant} {...props} />;
return <HeadBreadcrumbs parts={parts} {...props} />;
}
SourceDatasetBreadcrumbs.propTypes = {
......@@ -155,10 +131,10 @@ function SourceDatasetBreadcrumbs({ model, collection, ...props }) {
);
}
QuestionDataSource.shouldRender = ({ question, isObjectDetail }) =>
getDataSourceParts({ question, isObjectDetail }).length > 0;
QuestionDataSource.shouldRender = ({ question }) =>
getDataSourceParts({ question }).length > 0;
function getDataSourceParts({ question, subHead, isObjectDetail }) {
function getDataSourceParts({ question }) {
if (!question) {
return [];
}
......@@ -178,7 +154,7 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) {
const database = question.database();
if (database) {
parts.push({
icon: !subHead ? "database" : undefined,
icon: "database",
name: database.displayName(),
href: database.id >= 0 && Urls.browseDatabase(database),
});
......@@ -196,11 +172,9 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) {
}
if (table) {
const hasTableLink = subHead || isObjectDetail;
if (!isStructuredQuery) {
return {
name: table.displayName(),
link: hasTableLink ? getTableURL() : "",
};
}
......@@ -209,14 +183,7 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) {
...query.joins().map(j => j.joinedTable()),
].filter(Boolean);
parts.push(
<QuestionTableBadges
tables={allTables}
subHead={subHead}
hasLink={hasTableLink}
isLast={!isObjectDetail}
/>,
);
parts.push(<QuestionTableBadges tables={allTables} />);
}
return parts.filter(part => isValidElement(part) || part.name || part.icon);
......@@ -224,20 +191,11 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) {
QuestionTableBadges.propTypes = {
tables: PropTypes.arrayOf(PropTypes.object).isRequired,
hasLink: PropTypes.bool,
subHead: PropTypes.bool,
isLast: PropTypes.bool,
};
function QuestionTableBadges({ tables, subHead, hasLink, isLast }) {
const badgeInactiveColor = isLast && !subHead ? "text-dark" : "text-light";
function QuestionTableBadges({ tables }) {
const parts = tables.map(table => (
<HeadBreadcrumbs.Badge
key={table.id}
to={hasLink ? getTableURL(table) : ""}
inactiveColor={badgeInactiveColor}
>
<HeadBreadcrumbs.Badge key={table.id} to={""} inactiveColor="text-dark">
<TableInfoPopover table={table} placement="bottom-start">
<span>{table.displayName()}</span>
</TableInfoPopover>
......@@ -247,19 +205,11 @@ function QuestionTableBadges({ tables, subHead, hasLink, isLast }) {
return (
<HeadBreadcrumbs
parts={parts}
variant={subHead ? "subhead" : "head"}
variant="head"
divider={<TablesDivider>+</TablesDivider>}
data-testid="question-table-badges"
/>
);
}
function getTableURL(table) {
if (isVirtualCardId(table.id)) {
const cardId = getQuestionIdFromVirtualTableId(table.id);
return Urls.question({ id: cardId, name: table.displayName() });
}
return ML_Urls.getUrl(table.newQuestion());
}
export default QuestionDataSource;
frontend/src/metabase/query_builder/components/view/QuestionDataSource.unit.spec.js
+ 14
− 94
View file @ a5dd4444
......@@ -7,21 +7,20 @@ import {
} from "metabase-types/api/mocks";
import {
ORDERS_ID,
createSampleDatabase,
ORDERS,
SAMPLE_DB_ID,
PRODUCTS,
ORDERS_ID,
PEOPLE,
PRODUCTS_ID,
PEOPLE_ID,
createSampleDatabase,
PRODUCTS,
PRODUCTS_ID,
SAMPLE_DB_ID,
} from "metabase-types/api/mocks/presets";
import { createMockMetadata } from "__support__/metadata";
import { setupCardEndpoints } from "__support__/server-mocks/card";
import { renderWithProviders, screen } from "__support__/ui";
import * as Urls from "metabase/lib/urls";
import Question from "metabase-lib/Question";
import * as ML_Urls from "metabase-lib/urls";
import QuestionDataSource from "./QuestionDataSource";
const MULTI_SCHEMA_DB_ID = 2;
......@@ -59,24 +58,6 @@ const SAVED_QUESTION = {
collection_id: null,
};
const ORDERS_QUERY = {
type: "query",
database: SAMPLE_DB_ID,
query: { "source-table": ORDERS_ID },
};
const PRODUCTS_QUERY = {
type: "query",
database: SAMPLE_DB_ID,
query: { "source-table": PRODUCTS_ID },
};
const PEOPLE_QUERY = {
type: "query",
database: SAMPLE_DB_ID,
query: { "source-table": PEOPLE_ID },
};
const QUERY_IN_MULTI_SCHEMA_DB = {
type: "query",
database: MULTI_SCHEMA_DB_ID,
......@@ -199,18 +180,6 @@ function getSavedNativeQuestion(overrides) {
});
}
function getAdHocOrdersQuestion() {
return getAdHocQuestion({ dataset_query: ORDERS_QUERY });
}
function getAdHocProductsQuestion() {
return getAdHocQuestion({ dataset_query: PRODUCTS_QUERY });
}
function getAdHocPeopleQuestion() {
return getAdHocQuestion({ dataset_query: PEOPLE_QUERY });
}
function getNestedQuestionTableMock(isMultiSchemaDB) {
const dbId = isMultiSchemaDB ? MULTI_SCHEMA_DB_ID : SAMPLE_DB_ID;
const metadata = getMetadata();
......@@ -399,7 +368,7 @@ describe("QuestionDataSource", () => {
describe(questionType, () => {
it("displays database name", () => {
setup({ question });
const node = screen.queryByText(question.database().displayName());
const node = screen.getByText(question.database().displayName());
expect(node).toBeInTheDocument();
expect(node.closest("a")).toHaveAttribute(
"href",
......@@ -433,33 +402,9 @@ describe("QuestionDataSource", () => {
describe(questionType, () => {
it("displays table name", () => {
setup({ question });
const node = screen.queryByText(
new RegExp(question.table().displayName()),
);
expect(node).toBeInTheDocument();
expect(node.closest("a")).not.toBeInTheDocument();
});
it("displays table link in subhead variant", () => {
setup({ question, subHead: true });
const node = screen.queryByText(
new RegExp(question.table().displayName()),
);
expect(node.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(question.table().newQuestion()),
);
});
it("displays table link in object detail view", () => {
setup({ question, isObjectDetail: true });
const node = screen.queryByText(
new RegExp(question.table().displayName()),
);
expect(node.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(question.table().newQuestion()),
);
expect(
screen.getByText(new RegExp(question.table().displayName())),
).toBeInTheDocument();
});
});
});
......@@ -497,19 +442,11 @@ describe("QuestionDataSource", () => {
it("displays 2 joined tables (metabase#17961)", () => {
setup({ question, subHead: true });
const orders = screen.queryByText(/Orders/);
const products = screen.queryByText(/Products/);
const orders = screen.getByText(/Orders/);
const products = screen.getByText(/Products/);
expect(orders).toBeInTheDocument();
expect(orders.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(getAdHocOrdersQuestion()),
);
expect(products).toBeInTheDocument();
expect(products.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(getAdHocProductsQuestion()),
);
});
});
});
......@@ -523,26 +460,9 @@ describe("QuestionDataSource", () => {
describe(questionType, () => {
it("displays > 2 joined tables (metabase#17961)", () => {
setup({ question, subHead: true });
const orders = screen.queryByText(/Orders/);
const products = screen.queryByText(/Products/);
const people = screen.queryByText(/People/);
expect(orders).toBeInTheDocument();
expect(orders.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(getAdHocOrdersQuestion()),
);
expect(products).toBeInTheDocument();
expect(products.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(getAdHocProductsQuestion()),
);
expect(people).toBeInTheDocument();
expect(people.closest("a")).toHaveAttribute(
"href",
ML_Urls.getUrl(getAdHocPeopleQuestion()),
);
expect(screen.getByText(/Orders/)).toBeInTheDocument();
expect(screen.getByText(/Products/)).toBeInTheDocument();
expect(screen.getByText(/People/)).toBeInTheDocument();
});
});
});
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment