diff --git a/e2e/test/scenarios/filters/filter.cy.spec.js b/e2e/test/scenarios/filters/filter.cy.spec.js index 2bcb6219f21dc56b3a46c9beddb429eca93013f2..c45187ba39e71367e130d748d2360171acbb36da 100644 --- a/e2e/test/scenarios/filters/filter.cy.spec.js +++ b/e2e/test/scenarios/filters/filter.cy.spec.js @@ -841,6 +841,8 @@ describe("scenarios > question > filter", () => { describe("specific combination of filters can cause frontend reload or blank screen (metabase#16198)", () => { it("shouldn't display chosen category in a breadcrumb (metabase#16198-1)", () => { + const chosenCategory = "Gizmo"; + visitQuestionAdhoc({ dataset_query: { database: SAMPLE_DB_ID, @@ -848,13 +850,18 @@ describe("scenarios > question > filter", () => { "source-table": PRODUCTS_ID, filter: [ "and", - ["=", ["field", PRODUCTS.CATEGORY, null], "Gizmo"], + ["=", ["field", PRODUCTS.CATEGORY, null], chosenCategory], ["=", ["field", PRODUCTS.ID, null], 1], ], }, type: "query", }, }); + + cy.findByTestId("head-crumbs-container").should( + "not.contain", + chosenCategory, + ); }); it("adding an ID filter shouldn't cause page error and page reload (metabase#16198-2)", () => { diff --git a/frontend/src/metabase-lib/Question.ts b/frontend/src/metabase-lib/Question.ts index 984a89153761ec1abb7e73c13e092d91c808ad8a..9a56f42a22ef42b3e7f5c859d82555beabce3757 100644 --- a/frontend/src/metabase-lib/Question.ts +++ b/frontend/src/metabase-lib/Question.ts @@ -490,9 +490,9 @@ class Question { const hasSinglePk = table?.fields?.filter(field => field.isPK())?.length === 1; - const isStructured = !Lib.queryDisplayInfo(this.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(this.query()); - return isStructured && !Lib.hasClauses(query, -1) && hasSinglePk; + return !isNative && !Lib.hasClauses(query, -1) && hasSinglePk; } canAutoRun(): boolean { @@ -546,9 +546,9 @@ class Question { * of Question interface instead of Query interface makes it more convenient to also change the current visualization */ usesMetric(metricId): boolean { - const isStructured = !Lib.queryDisplayInfo(this.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(this.query()); return ( - isStructured && + !isNative && _.any( QUERY.getAggregations( this.legacyQuery({ useStructuredQuery: true }).legacyQuery({ @@ -561,9 +561,9 @@ class Question { } usesSegment(segmentId): boolean { - const isStructured = !Lib.queryDisplayInfo(this.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(this.query()); return ( - isStructured && + !isNative && QUERY.getFilters( this.legacyQuery({ useStructuredQuery: true }).legacyQuery({ useStructuredQuery: true, diff --git a/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js b/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js index 2dc86bf7c02a1ad5685425fe6f9eb7fbeb8e6245..3561244e18609d7ce09797175529b756cf5ad2ea 100644 --- a/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js +++ b/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js @@ -117,11 +117,11 @@ export function getDashboardDrillQuestionUrl(question, clicked) { clickBehavior, }); - const isTargetQuestionStructured = !Lib.queryDisplayInfo( + const isTargetQuestionNative = Lib.queryDisplayInfo( targetQuestion.query(), ).isNative; - return isTargetQuestionStructured + return !isTargetQuestionNative ? ML_Urls.getUrlWithParameters(targetQuestion, parameters, queryParams) : `${ML_Urls.getUrl(targetQuestion)}?${querystring.stringify(queryParams)}`; } diff --git a/frontend/src/metabase-lib/queries/utils/pivot.js b/frontend/src/metabase-lib/queries/utils/pivot.js index e03edff5ac0fbb2c7f9f475656fcbeb3c3c112af..ef253cb8132f5d47f5c120b8abdb68529d3dfeee 100644 --- a/frontend/src/metabase-lib/queries/utils/pivot.js +++ b/frontend/src/metabase-lib/queries/utils/pivot.js @@ -4,9 +4,9 @@ import { FieldDimension } from "metabase-lib/Dimension"; export function getPivotColumnSplit(question) { const setting = question.setting("pivot_table.column_split"); - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); const breakout = - (isStructured && + (!isNative && question.legacyQuery({ useStructuredQuery: true }).breakouts()) || []; const { rows: pivot_rows, columns: pivot_cols } = _.mapObject( diff --git a/frontend/src/metabase-lib/urls.ts b/frontend/src/metabase-lib/urls.ts index 4e20a6e5f6338b0122ef728dda51d85d0ef3de48..ab7a7dff29687915772b2bee001592bbf9b31a0d 100644 --- a/frontend/src/metabase-lib/urls.ts +++ b/frontend/src/metabase-lib/urls.ts @@ -55,9 +55,9 @@ export function getUrlWithParameters( const includeDisplayIsLocked = true; const { isEditable } = Lib.queryDisplayInfo(question.query()); - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); - if (isStructured) { + if (!isNative) { let questionWithParameters = question.setParameters(parameters); if (isEditable) { diff --git a/frontend/src/metabase/parameters/utils/mapping-options.js b/frontend/src/metabase/parameters/utils/mapping-options.js index 8a58f72637b6661869a01c53754a6f6f8bce709b..c33d23ce85067a3c9b9205e5c07f981b141c37da 100644 --- a/frontend/src/metabase/parameters/utils/mapping-options.js +++ b/frontend/src/metabase/parameters/utils/mapping-options.js @@ -97,9 +97,9 @@ export function getParameterMappingOptions( } const question = new Question(card, metadata); - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); const options = []; - if (isStructured || question.isDataset()) { + if (!isNative || question.isDataset()) { // treat the dataset/model question like it is already composed so that we can apply // dataset/model-specific metadata to the underlying dimension options const query = question.isDataset() diff --git a/frontend/src/metabase/query_builder/actions/core/core.js b/frontend/src/metabase/query_builder/actions/core/core.js index 3cc57b47d1957749c78d168a664a81ffc8295d52..e1824ee0d9c934b2ab6a34f2628b7bf1652d1e2f 100644 --- a/frontend/src/metabase/query_builder/actions/core/core.js +++ b/frontend/src/metabase/query_builder/actions/core/core.js @@ -248,9 +248,9 @@ export const apiUpdateQuestion = (question, { rerunQuery } = {}) => { ); } - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); - if (isStructured) { + if (!isNative) { rerunQuery = rerunQuery ?? isResultDirty; } diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts index dbfa7723cbd2415e806f4dcb6b106999a4ca9a8a..537a1365bcaebd2f1729d6d7011ba5aa44a84cc2 100644 --- a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts +++ b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts @@ -377,8 +377,8 @@ async function handleQBInit( }); if (uiControls.queryBuilderMode !== "notebook") { - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; - if (question.canRun() && (question.isSaved() || isStructured)) { + const { isNative } = Lib.queryDisplayInfo(question.query()); + if (question.canRun() && (question.isSaved() || !isNative)) { // Timeout to allow Parameters widget to set parameterValues setTimeout( () => dispatch(runQuestionQuery({ shouldUpdateUrl: false })), diff --git a/frontend/src/metabase/query_builder/actions/navigation.js b/frontend/src/metabase/query_builder/actions/navigation.js index eaa5fabef9e6a8ddb264fe06e00e6dff64089727..584a9a53fa99fdd7fa24b2284d3026a3fbf749d1 100644 --- a/frontend/src/metabase/query_builder/actions/navigation.js +++ b/frontend/src/metabase/query_builder/actions/navigation.js @@ -138,10 +138,10 @@ export const updateUrl = createThunkAction( (!isAdHocModel && question.isDirtyComparedTo(originalQuestion)); } - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); // prevent clobbering of hash when there are fake parameters on the question // consider handling this in a more general way, somehow - if (isStructured && question.parameters().length > 0) { + if (!isNative && question.parameters().length > 0) { dirty = true; } diff --git a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx index 2d9562e20d7243042f6b75235a637a6428043110..eca7fbd9c550f2ca7c0de689b3fc8a990565b5ee 100644 --- a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx +++ b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx @@ -221,9 +221,9 @@ function DatasetEditor(props) { const isEditingMetadata = datasetEditorTab === "metadata"; const initialEditorHeight = useMemo(() => { - const isStructured = !Lib.queryDisplayInfo(dataset.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(dataset.query()); - if (isStructured) { + if (!isNative) { return INITIAL_NOTEBOOK_EDITOR_HEIGHT; } return calcInitialEditorHeight({ @@ -416,8 +416,8 @@ function DatasetEditor(props) { ); const canSaveChanges = useMemo(() => { - const isStructured = !Lib.queryDisplayInfo(dataset.query()).isNative; - const isEmpty = isStructured + const { isNative } = Lib.queryDisplayInfo(dataset.query()); + const isEmpty = !isNative ? Lib.databaseID(dataset.query()) == null : dataset.legacyQuery().isEmpty(); diff --git a/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx b/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx index 7d4630aede09f2996b37c8594184a5cfd764ca9d..7e099f9fd90eaacb0109b333c1d9a15c7bf870fd 100644 --- a/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx @@ -93,9 +93,9 @@ const Notebook = ({ className, updateQuestion, ...props }: NotebookProps) => { function getSourceQuestionId(question: Question) { const query = question.query(); - const isStructured = !Lib.queryDisplayInfo(query).isNative; + const { isNative } = Lib.queryDisplayInfo(query); - if (isStructured) { + if (!isNative) { const sourceTableId = Lib.sourceTableOrCardId(query); if (isVirtualCardId(sourceTableId)) { diff --git a/frontend/src/metabase/query_builder/components/view/ConvertQueryButton/ConvertQueryButton.tsx b/frontend/src/metabase/query_builder/components/view/ConvertQueryButton/ConvertQueryButton.tsx index e31ee2f7b6ce9a4f790edf126601acafabe504c0..3e690c09c24fee58fe73366e09e22dfd0b9a5dbe 100644 --- a/frontend/src/metabase/query_builder/components/view/ConvertQueryButton/ConvertQueryButton.tsx +++ b/frontend/src/metabase/query_builder/components/view/ConvertQueryButton/ConvertQueryButton.tsx @@ -46,9 +46,9 @@ ConvertQueryButton.shouldRender = ({ question, queryBuilderMode, }: ConvertQueryButtonOpts) => { - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); return ( - isStructured && + !isNative && question.database()?.native_permissions === "write" && queryBuilderMode === "notebook" ); diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx b/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx index 5f7f3b75ebb4ca462bf163500bae84320b5fa767..fe673b757fa9aea2b23794df95a9597f91b4b536 100644 --- a/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx +++ b/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx @@ -175,7 +175,7 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) { const parts = []; const query = question.query(); const metadata = question.metadata(); - const isStructured = !Lib.queryDisplayInfo(query).isNative; + const { isNative } = Lib.queryDisplayInfo(query); const database = metadata.database(Lib.databaseID(query)); if (database) { @@ -186,7 +186,7 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) { }); } - const table = isStructured + const table = !isNative ? metadata.table(Lib.sourceTableOrCardId(query)) : question.legacyQuery().table(); if (table && table.hasSchema()) { @@ -201,7 +201,7 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) { if (table) { const hasTableLink = subHead || isObjectDetail; - if (!isStructured) { + if (isNative) { return { name: table.displayName(), link: hasTableLink ? getTableURL() : "", diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDescription.jsx b/frontend/src/metabase/query_builder/components/view/QuestionDescription.jsx index 70895148040df3ca45758c9da1bbfe56b286446d..cd326676781f861bd43cb9367aad9a50b8d94ee6 100644 --- a/frontend/src/metabase/query_builder/components/view/QuestionDescription.jsx +++ b/frontend/src/metabase/query_builder/components/view/QuestionDescription.jsx @@ -13,9 +13,9 @@ const QuestionDescription = ({ onClick, }) => { const query = question.query(); - const isStructured = !Lib.queryDisplayInfo(query).isNative; + const { isNative } = Lib.queryDisplayInfo(query); - if (isStructured) { + if (!isNative) { const stageIndex = -1; const aggregations = Lib.aggregations(query, stageIndex); const breakouts = Lib.breakouts(query, stageIndex); diff --git a/frontend/src/metabase/query_builder/components/view/QuestionRowCount/QuestionRowCount.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/QuestionRowCount/QuestionRowCount.unit.spec.tsx index b07efb552267a08d8dcece9b7193c38cdeab1b84..da1ab8dde885abaa9f8c9120ce9255ab40dbdb0a 100644 --- a/frontend/src/metabase/query_builder/components/view/QuestionRowCount/QuestionRowCount.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/view/QuestionRowCount/QuestionRowCount.unit.spec.tsx @@ -46,8 +46,8 @@ type SetupOpts = { function patchQuestion(question: Question) { const query = question.query(); - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; - if (isStructured) { + const { isNative } = Lib.queryDisplayInfo(question.query()); + if (!isNative) { const [sampleColumn] = Lib.orderableColumns(query, 0); const nextQuery = Lib.orderBy(query, 0, sampleColumn); return question.setDatasetQuery(Lib.toLegacyQuery(nextQuery)); diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx index cdaa3174e31b01dbf222a22960d90145e37d0d13..81dd7a743bac79c73942e8eea180da14bc75da88 100644 --- a/frontend/src/metabase/query_builder/components/view/View.jsx +++ b/frontend/src/metabase/query_builder/components/view/View.jsx @@ -192,9 +192,9 @@ class View extends Component { getRightSidebar = () => { const { question } = this.props; - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); - return isStructured + return !isNative ? this.getRightSidebarForStructuredQuery() : this.getRightSidebarForNativeQuery(); }; @@ -203,10 +203,10 @@ class View extends Component { const { question } = this.props; const query = question.query(); const legacyQuery = question.legacyQuery({ useStructuredQuery: true }); - const isStructured = !Lib.queryDisplayInfo(query).isNative; + const { isNative } = Lib.queryDisplayInfo(query); const isNewQuestion = - isStructured && + !isNative && Lib.sourceTableOrCardId(query) === null && !legacyQuery.sourceQuery(); @@ -333,10 +333,10 @@ class View extends Component { const query = question.query(); const legacyQuery = question.legacyQuery({ useStructuredQuery: true }); - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); const isNewQuestion = - isStructured && + !isNative && Lib.sourceTableOrCardId(query) === null && !legacyQuery.sourceQuery(); @@ -373,7 +373,7 @@ class View extends Component { <QueryBuilderViewRoot className="QueryBuilder"> {isHeaderVisible && this.renderHeader()} <QueryBuilderContentContainer> - {isStructured && ( + {!isNative && ( <QueryViewNotebook isNotebookContainerOpen={isNotebookContainerOpen} {...this.props} diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js index 482170deed8e1f2069bbe19b2ac3aaf7fcf66b40..84c528d93428d9299e7ee52d80813dca51a81b27 100644 --- a/frontend/src/metabase/query_builder/selectors.js +++ b/frontend/src/metabase/query_builder/selectors.js @@ -639,11 +639,11 @@ export const getShouldShowUnsavedChangesWarning = createSelector( return isSavedQuestionChanged; } - const isOriginalQuestionStructured = + const isOriginalQuestionNative = originalQuestion && - !Lib.queryDisplayInfo(originalQuestion.query()).isNative; + Lib.queryDisplayInfo(originalQuestion.query()).isNative; - if (isOriginalQuestionStructured) { + if (!isOriginalQuestionNative) { return uiControls.isModifiedFromNotebook; } diff --git a/frontend/src/metabase/query_builder/utils.ts b/frontend/src/metabase/query_builder/utils.ts index d8557a4bb80759a757600c8f0a3dc93893ce472a..bf4afe47120df260582552848ca3e0b7a2b7743a 100644 --- a/frontend/src/metabase/query_builder/utils.ts +++ b/frontend/src/metabase/query_builder/utils.ts @@ -82,9 +82,8 @@ export const isNavigationAllowed = ({ const { hash, pathname } = destination; const { isNative } = Lib.queryDisplayInfo(question.query()); - const isStructured = !isNative; - const runModelPathnames = isStructured + const runModelPathnames = !isNative ? ["/model", "/model/notebook"] : ["/model"]; const isRunningModel = @@ -118,7 +117,7 @@ export const isNavigationAllowed = ({ * https://github.com/metabase/metabase/issues/34686 * */ - if (!isNewQuestion && isStructured) { + if (!isNewQuestion && !isNative) { const isRunningQuestion = ["/question", "/question/notebook"].includes(pathname) && hash.length > 0; const allowedPathnames = validSlugs.flatMap(slug => [ diff --git a/frontend/src/metabase/visualizations/components/LeafletMap.jsx b/frontend/src/metabase/visualizations/components/LeafletMap.jsx index e176c4203a4f33760e4f0379a1e75ece9a35cb43..05df88ee730f4afaf957e656f6c38b9fd25f4769 100644 --- a/frontend/src/metabase/visualizations/components/LeafletMap.jsx +++ b/frontend/src/metabase/visualizations/components/LeafletMap.jsx @@ -158,9 +158,9 @@ export default class LeafletMap extends Component { }); const question = new Question(card, metadata); - const isStructured = !Lib.queryDisplayInfo(question.query()).isNative; + const { isNative } = Lib.queryDisplayInfo(question.query()); - if (isStructured) { + if (!isNative) { const query = question.query(); const stageIndex = -1; const filterBounds = { diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/ChartSettingTableColumns.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/ChartSettingTableColumns.tsx index 6ff7760bd283e73e9ce6b9583a4c1e33a1761227..46e686fcf53b3c822f6d2cc0b273a45ea0855c00 100644 --- a/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/ChartSettingTableColumns.tsx +++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/ChartSettingTableColumns.tsx @@ -39,10 +39,9 @@ export const ChartSettingTableColumns = ({ [question, onChange], ); - const isStructured = - question && !Lib.queryDisplayInfo(question.query()).isNative; + const isNative = question && Lib.queryDisplayInfo(question.query()).isNative; - if (isStructured) { + if (question && !isNative) { const query = question.query(); return (