Skip to content
Snippets Groups Projects
Commit 7cc72cd8 authored by Paul Rosenzweig's avatar Paul Rosenzweig Committed by Tom Robinson
Browse files

Don't run native queries with invalid template tags (#10946)

* don't run native queries with invalid template tags

* fix test
parent 4d8a54e1
No related branches found
No related tags found
No related merge requests found
...@@ -63,7 +63,11 @@ export default class NativeQuery extends AtomicQuery { ...@@ -63,7 +63,11 @@ export default class NativeQuery extends AtomicQuery {
} }
canRun() { canRun() {
return this.hasData() && this.queryText().length > 0; return (
this.hasData() &&
this.queryText().length > 0 &&
this.allTemplateTagsAreValid()
);
} }
isEmpty() { isEmpty() {
...@@ -226,6 +230,12 @@ export default class NativeQuery extends AtomicQuery { ...@@ -226,6 +230,12 @@ export default class NativeQuery extends AtomicQuery {
templateTagsMap(): TemplateTags { templateTagsMap(): TemplateTags {
return getIn(this.datasetQuery(), ["native", "template-tags"]) || {}; return getIn(this.datasetQuery(), ["native", "template-tags"]) || {};
} }
allTemplateTagsAreValid(): boolean {
return this.templateTags().every(
// field filters require a field
t => !(t.type === "dimension" && t.dimension == null),
);
}
setDatasetQuery(datasetQuery: DatasetQuery): NativeQuery { setDatasetQuery(datasetQuery: DatasetQuery): NativeQuery {
return new NativeQuery(this._originalQuestion, datasetQuery); return new NativeQuery(this._originalQuestion, datasetQuery);
...@@ -282,7 +292,7 @@ export default class NativeQuery extends AtomicQuery { ...@@ -282,7 +292,7 @@ export default class NativeQuery extends AtomicQuery {
id: Utils.uuid(), id: Utils.uuid(),
name: tagName, name: tagName,
display_name: humanize(tagName), display_name: humanize(tagName),
type: null, type: "text",
}; };
} }
} }
......
...@@ -160,7 +160,12 @@ export default class TagEditorParam extends Component { ...@@ -160,7 +160,12 @@ export default class TagEditorParam extends Component {
{tag.type === "dimension" && ( {tag.type === "dimension" && (
<div className="pb1"> <div className="pb1">
<h5 className="pb1 text-normal">{t`Field to map to`}</h5> <h5 className="pb1 text-normal">
{t`Field to map to`}
{tag.dimension == null && (
<span className="text-error mx1">(required)</span>
)}
</h5>
{(!hasSelectedDimensionField || {(!hasSelectedDimensionField ||
(hasSelectedDimensionField && fieldMetadataLoaded)) && ( (hasSelectedDimensionField && fieldMetadataLoaded)) && (
...@@ -179,41 +184,43 @@ export default class TagEditorParam extends Component { ...@@ -179,41 +184,43 @@ export default class TagEditorParam extends Component {
</div> </div>
)} )}
<div className="pb1"> {hasSelectedDimensionField && (
<h5 className="pb1 text-normal">{t`Filter widget type`}</h5> <div className="pb1">
<Select <h5 className="pb1 text-normal">{t`Filter widget type`}</h5>
className="border-med bg-white block" <Select
value={tag["widget-type"]} className="border-med bg-white block"
onChange={e => value={tag["widget-type"]}
this.setParameterAttribute("widget-type", e.target.value) onChange={e =>
} this.setParameterAttribute("widget-type", e.target.value)
isInitiallyOpen={!tag["widget-type"] && hasWidgetOptions} }
placeholder={t`Select…`} isInitiallyOpen={!tag["widget-type"] && hasWidgetOptions}
> placeholder={t`Select…`}
{[{ name: "None", type: undefined }] >
.concat(widgetOptions) {[{ name: "None", type: undefined }]
.map(widgetOption => ( .concat(widgetOptions)
<Option key={widgetOption.type} value={widgetOption.type}> .map(widgetOption => (
{widgetOption.name} <Option key={widgetOption.type} value={widgetOption.type}>
</Option> {widgetOption.name}
))} </Option>
</Select> ))}
{hasSelectedDimensionField && !hasWidgetOptions && ( </Select>
<p className="pb1"> {!hasWidgetOptions && (
{t`There aren't any filter widgets for this type of field yet.`}{" "} <p className="pb1">
<Link {t`There aren't any filter widgets for this type of field yet.`}{" "}
to={MetabaseSettings.docsUrl( <Link
"users-guide/13-sql-parameters", to={MetabaseSettings.docsUrl(
"the-field-filter-variable-type", "users-guide/13-sql-parameters",
)} "the-field-filter-variable-type",
target="_blank" )}
className="link" target="_blank"
> className="link"
{t`Learn more`} >
</Link> {t`Learn more`}
</p> </Link>
)} </p>
</div> )}
</div>
)}
<div className="flex align-center pb1"> <div className="flex align-center pb1">
<h5 className="text-normal mr1">{t`Required?`}</h5> <h5 className="text-normal mr1">{t`Required?`}</h5>
......
import { assocIn } from "icepick";
import { import {
SAMPLE_DATASET, SAMPLE_DATASET,
MONGO_DATABASE, MONGO_DATABASE,
...@@ -170,5 +172,29 @@ describe("NativeQuery", () => { ...@@ -170,5 +172,29 @@ describe("NativeQuery", () => {
expect(tagMaps["max_price"].display_name).toEqual("Max price"); expect(tagMaps["max_price"].display_name).toEqual("Max price");
}); });
}); });
describe("Invalid template tags prevent the query from running", () => {
let q = makeQuery().setQueryText("SELECT * from ORDERS where {{foo}}");
expect(q.canRun()).toBe(true);
// set template tag's type to dimension without setting field id
q = q.setDatasetQuery(
assocIn(
q.datasetQuery(),
["native", "template-tags", "foo", "type"],
"dimension",
),
);
expect(q.canRun()).toBe(false);
// now set the field
q = q.setDatasetQuery(
assocIn(
q.datasetQuery(),
["native", "template-tags", "foo", "dimension"],
["field-id", 123],
),
);
expect(q.canRun()).toBe(true);
});
}); });
}); });
...@@ -37,6 +37,7 @@ import { ...@@ -37,6 +37,7 @@ import {
UPDATE_EMBEDDING_PARAMS, UPDATE_EMBEDDING_PARAMS,
UPDATE_ENABLE_EMBEDDING, UPDATE_ENABLE_EMBEDDING,
UPDATE_TEMPLATE_TAG, UPDATE_TEMPLATE_TAG,
SET_IS_SHOWING_TEMPLATE_TAGS_EDITOR,
} from "metabase/query_builder/actions"; } from "metabase/query_builder/actions";
import NativeQueryEditor from "metabase/query_builder/components/NativeQueryEditor"; import NativeQueryEditor from "metabase/query_builder/components/NativeQueryEditor";
import { delay } from "metabase/lib/promise"; import { delay } from "metabase/lib/promise";
...@@ -157,8 +158,11 @@ describe("public/embedded", () => { ...@@ -157,8 +158,11 @@ describe("public/embedded", () => {
"select count(*) from products where {{category}}", "select count(*) from products where {{category}}",
); );
await store.waitForActions([SET_IS_SHOWING_TEMPLATE_TAGS_EDITOR]);
const tagEditorSidebar = app.find(TagEditorSidebar); const tagEditorSidebar = app.find(TagEditorSidebar);
click(tagEditorSidebar.find("SelectButton"));
const fieldFilterVarType = tagEditorSidebar const fieldFilterVarType = tagEditorSidebar
.find(".ColumnarSelector-row") .find(".ColumnarSelector-row")
.at(3); .at(3);
......
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