From 81c3639c05540b6005808dc7fbc3e70066f1179d Mon Sep 17 00:00:00 2001
From: Cal Herries <39073188+calherries@users.noreply.github.com>
Date: Wed, 19 Oct 2022 19:14:43 +0100
Subject: [PATCH] Remove unused CardTagEditor code (#26008)
* Remove CardTagEditor
* Remove unused cardTagRegexFromId
---
.../metabase-lib/lib/queries/NativeQuery.ts | 19 ---
.../template_tags/CardTagEditor.jsx | 146 ------------------
.../template_tags/TagEditorSidebar.jsx | 32 ++--
.../lib/queries/NativeQuery.unit.spec.js | 38 -----
4 files changed, 10 insertions(+), 225 deletions(-)
delete mode 100644 frontend/src/metabase/query_builder/components/template_tags/CardTagEditor.jsx
diff --git a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
index 9d6366f4c29..4a7e029d627 100644
--- a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
+++ b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
@@ -67,15 +67,9 @@ export function recognizeTemplateTags(queryText: string): string[] {
return _.uniq(tagNames);
}
-// needs to match logically with `cardTagRegexFromId`
// matches '#123-foo-bar' and '#123' but not '#123foo'
const CARD_TAG_NAME_REGEX: RegExp = /^#([0-9]*)(-[a-z0-9-]*)?$/;
-// needs to match logically with `CARD_TAG_NAME_REGEX`
-function cardTagRegexFromId(cardId: number): RegExp {
- return new RegExp(`{{\\s*#${cardId}(-[a-z0-9-]*)?\\s*}}`, "g");
-}
-
function tagRegex(tagName: string): RegExp {
return new RegExp(`{{\\s*${tagName}\\s*}}`, "g");
}
@@ -91,19 +85,6 @@ function replaceTagName(
return query.setQueryText(queryText);
}
-// replaces template tag with given cardId with a new tag name
-// the new tag name could reference a completely different card
-export function replaceCardTagNameById(
- query: NativeQuery,
- cardId: number,
- newTagName: string,
-): NativeQuery {
- const queryText = query
- .queryText()
- .replace(cardTagRegexFromId(cardId), `{{${newTagName}}}`);
- return query.setQueryText(queryText);
-}
-
export function cardIdFromTagName(name: string): number | null {
const match = name.match(CARD_TAG_NAME_REGEX);
return parseInt(match?.[1]) || null;
diff --git a/frontend/src/metabase/query_builder/components/template_tags/CardTagEditor.jsx b/frontend/src/metabase/query_builder/components/template_tags/CardTagEditor.jsx
deleted file mode 100644
index 82ac9805cc9..00000000000
--- a/frontend/src/metabase/query_builder/components/template_tags/CardTagEditor.jsx
+++ /dev/null
@@ -1,146 +0,0 @@
-/* eslint-disable react/prop-types */
-import React, { Component } from "react";
-import { Link } from "react-router";
-import { t } from "ttag";
-import cx from "classnames";
-
-import Icon from "metabase/components/Icon";
-import QuestionPicker from "metabase/containers/QuestionPicker";
-import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
-import SelectButton from "metabase/core/components/SelectButton";
-import LoadingSpinner from "metabase/components/LoadingSpinner";
-
-import Questions from "metabase/entities/questions";
-import * as Urls from "metabase/lib/urls";
-import { formatDateTimeWithUnit } from "metabase/lib/formatting";
-import MetabaseSettings from "metabase/lib/settings";
-import { replaceCardTagNameById } from "metabase-lib/lib/queries/NativeQuery";
-
-class CardTagEditor extends Component {
- handleQuestionSelection = id => {
- const { question, query, setDatasetQuery } = this.props;
- const selectedQuestion = query.metadata().question(id);
- setDatasetQuery(
- replaceCardTagNameById(
- query,
- question ? question.id : "",
- `#${selectedQuestion.slug()}`,
- ).datasetQuery(),
- );
- this._popover && this._popover.close();
- };
-
- getQuestionUrl() {
- const { tag, question } = this.props;
- return Urls.question(question || { id: tag["card-id"] });
- }
-
- errorMessage() {
- const { error, question, query } = this.props;
-
- if (
- question &&
- // If this question was loaded by a search endpoint before fetching, it
- // might not have a database_id set yet.
- question.database_id != null &&
- question.database_id !== query.databaseId()
- ) {
- return t`This question can't be used because it's based on a different database.`;
- }
- if (error) {
- return error.status === 404
- ? t`Couldn't find a saved question with that ID number.`
- : error.data;
- }
- return null;
- }
-
- triggerElement() {
- const { tag, question } = this.props;
- return (
- <SelectButton>
- {tag["card-id"] == null ? (
- <span className="text-medium">{t`Pick a question or a model`}</span>
- ) : this.errorMessage() ? (
- <span className="text-medium">{t`Pick a different question or a model`}</span>
- ) : question ? (
- question.name
- ) : (
- // we only hit this on the initial render before we fetch
- t`Loading…`
- )}
- </SelectButton>
- );
- }
-
- render() {
- const {
- tag: { "card-id": cardId },
- loading,
- question,
- } = this.props;
-
- return (
- <div className="px3 py4 border-top">
- <h3 className="text-heavy text-brand mb1">
- {cardId == null ? (
- t`Question #…`
- ) : (
- <Link to={this.getQuestionUrl()}>
- {question?.dataset ? t`Model #${cardId}` : t`Question #${cardId}`}
- </Link>
- )}
- </h3>
- {loading ? (
- <LoadingSpinner />
- ) : (
- <PopoverWithTrigger
- ref={ref => (this._popover = ref)}
- triggerElement={this.triggerElement()}
- verticalAttachments={["top", "bottom"]}
- horizontalAttachments={["right", "left"]}
- pinInitialAttachment
- >
- <QuestionPicker
- className="p2"
- value={question && question.id}
- onChange={this.handleQuestionSelection}
- />
- </PopoverWithTrigger>
- )}
- {this.errorMessage() && (
- <p className="text-error bg-light p2 mt2 mb0">
- {this.errorMessage()}
- </p>
- )}
- {question && !this.errorMessage() && (
- <div className="bg-light text-medium text-small py1 px2 mt1">
- {question.collection && (
- <div className="flex align-center">
- <Icon name="all" size={12} mr={1} /> {question.collection.name}
- </div>
- )}
- <div
- className={cx("flex align-center", { mt1: question.collection })}
- >
- <Icon name="calendar" size={12} mr={1} />{" "}
- {t`Last edited ${formatDate(question.updated_at)}`}
- </div>
- </div>
- )}
- </div>
- );
- }
-}
-
-export default Questions.load({
- id: (state, { tag }) => tag["card-id"],
- loadingAndErrorWrapper: false,
- dispatchApiErrorEvent: false,
-})(CardTagEditor);
-
-// This formats a timestamp as a date using any custom formatting options.
-function formatDate(value) {
- const options = MetabaseSettings.get("custom-formatting")["type/Temporal"];
- return formatDateTimeWithUnit(value, "day", options);
-}
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx
index de4b39a8868..c740f5b0881 100644
--- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx
+++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx
@@ -10,7 +10,6 @@ import SidebarContent from "metabase/query_builder/components/SidebarContent";
import * as MetabaseAnalytics from "metabase/lib/analytics";
import NativeQuery from "metabase-lib/lib/queries/NativeQuery";
import TagEditorParam from "./TagEditorParam";
-import CardTagEditor from "./CardTagEditor";
import TagEditorHelp from "./TagEditorHelp";
export default class TagEditorSidebar extends React.Component {
@@ -85,8 +84,6 @@ export default class TagEditorSidebar extends React.Component {
databaseFields={databaseFields}
database={database}
databases={databases}
- query={query}
- setDatasetQuery={setDatasetQuery}
setTemplateTag={setTemplateTag}
setParameterValue={setParameterValue}
/>
@@ -110,31 +107,22 @@ const SettingsPane = ({
databaseFields,
database,
databases,
- query,
- setDatasetQuery,
setTemplateTag,
setParameterValue,
}) => (
<div>
{tags.map(tag => (
<div key={tags.name}>
- {tag.type === "card" ? (
- <CardTagEditor
- query={query}
- setDatasetQuery={setDatasetQuery}
- tag={tag}
- />
- ) : (
- <TagEditorParam
- tag={tag}
- parameter={parametersById[tag.id]}
- databaseFields={databaseFields}
- database={database}
- databases={databases}
- setTemplateTag={setTemplateTag}
- setParameterValue={setParameterValue}
- />
- )}
+ <TagEditorParam
+ tag={tag}
+ key={tags.name}
+ parameter={parametersById[tag.id]}
+ databaseFields={databaseFields}
+ database={database}
+ databases={databases}
+ setTemplateTag={setTemplateTag}
+ setParameterValue={setParameterValue}
+ />
</div>
))}
</div>
diff --git a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
index f9c386aec74..2b74ba1323a 100644
--- a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
@@ -7,7 +7,6 @@ import {
} from "__support__/sample_database_fixture";
import NativeQuery, {
- replaceCardTagNameById,
recognizeTemplateTags,
cardIdFromTagName,
updateCardTemplateTagNames,
@@ -268,43 +267,6 @@ describe("NativeQuery", () => {
expect(q.templateTags().map(v => v["card-id"])).toEqual([1, 2, 1]);
});
});
- describe("replaceCardTagNameById", () => {
- it("should update the query text", () => {
- const query = makeQuery().setQueryText("SELECT * from {{ #123 }}");
- const newQuery = replaceCardTagNameById(
- query,
- "123",
- "#456-a-card-name",
- );
- expect(newQuery.queryText()).toBe("SELECT * from {{#456-a-card-name}}");
- const tags = newQuery.templateTags();
- expect(tags.length).toBe(1);
- const [{ "card-id": cardId, type, name }] = tags;
- expect(cardId).toEqual(456);
- expect(type).toEqual("card");
- expect(name).toEqual("#456-a-card-name");
- });
-
- it("should perform multiple updates", () => {
- const query = makeQuery().setQueryText(
- "{{#123}} {{foo}} {{#1234}} {{ #123-original-card-name }}",
- );
- const newQuery = replaceCardTagNameById(
- query,
- "123",
- "#456-a-card-name",
- );
- expect(newQuery.queryText()).toBe(
- "{{#456-a-card-name}} {{foo}} {{#1234}} {{#456-a-card-name}}",
- );
- });
-
- it("should replace a blank id", () => {
- const query = makeQuery().setQueryText("{{#}} {{#123}}");
- const newQuery = replaceCardTagNameById(query, "", "#456-a-card-name");
- expect(newQuery.queryText()).toBe("{{#456-a-card-name}} {{#123}}");
- });
- });
});
describe("variables", () => {
it("should return empty array if there are no tags", () => {
--
GitLab