From 42a58fee2620c9da27bcfa6ed558d75aba0f9318 Mon Sep 17 00:00:00 2001
From: Kamil Mielnik <kamil@kamilmielnik.com>
Date: Wed, 28 Feb 2024 18:47:02 +0700
Subject: [PATCH] Fix missing result metadata (#39201)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b4081ed192267621b716b1634d304f470f.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Tamás Benkő <tamas@metabase.com>
---
 ...9-37009-missing-result-metadata.cy.spec.js | 61 +++++++++++++++++++
 .../query_builder/actions/core/core.js        | 11 +++-
 .../query_builder/actions/querying.js         |  4 +-
 3 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 e2e/test/scenarios/models/reproductions/35039-37009-missing-result-metadata.cy.spec.js

diff --git a/e2e/test/scenarios/models/reproductions/35039-37009-missing-result-metadata.cy.spec.js b/e2e/test/scenarios/models/reproductions/35039-37009-missing-result-metadata.cy.spec.js
new file mode 100644
index 00000000000000..1ca9acfcd5f241
--- /dev/null
+++ b/e2e/test/scenarios/models/reproductions/35039-37009-missing-result-metadata.cy.spec.js
@@ -0,0 +1,61 @@
+import {
+  focusNativeEditor,
+  modal,
+  openQuestionActions,
+  popover,
+  restore,
+} from "e2e/support/helpers";
+
+describe("issues 35039 and 37009", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+  });
+
+  // This test follows #37009 repro steps because they are simpler than #35039 but still equivalent
+  it("should show columns available in the model (metabase#35039) (metabase#37009)", () => {
+    cy.visit("/model/new");
+    cy.findByTestId("new-model-options")
+      .findByText("Use a native query")
+      .click();
+
+    focusNativeEditor().type("select * from products -- where true=false");
+
+    cy.findByTestId("dataset-edit-bar").findByText("Save").click();
+    modal()
+      .last()
+      .within(() => {
+        cy.findByLabelText("Name").type("Model").realPress("Tab");
+        cy.findByText("Save").click();
+      });
+
+    openQuestionActions();
+    popover().findByText("Edit query definition").click();
+
+    focusNativeEditor().type(
+      "{backspace}{backspace}{backspace}{backspace}{backspace}",
+    );
+    cy.findByTestId("dataset-edit-bar").within(() => {
+      cy.findByText("Save changes").click();
+      cy.findByText("Saving…").should("not.exist");
+    });
+
+    cy.findByTestId("query-builder-main").within(() => {
+      cy.findByText("Doing science...").should("be.visible");
+      cy.findByText("Doing science...").should("not.exist");
+    });
+
+    cy.icon("notebook").click();
+    cy.findByTestId("fields-picker").click();
+    popover().within(() => {
+      cy.findByText("ID").should("exist");
+      cy.findByText("EAN").should("exist");
+      cy.findByText("TITLE").should("exist");
+      cy.findByText("CATEGORY").should("exist");
+      cy.findByText("VENDOR").should("exist");
+      cy.findByText("PRICE").should("exist");
+      cy.findByText("RATING").should("exist");
+      cy.findByText("CREATED_AT").should("exist");
+    });
+  });
+});
diff --git a/frontend/src/metabase/query_builder/actions/core/core.js b/frontend/src/metabase/query_builder/actions/core/core.js
index 397191700e3529..aa62043a9daa57 100644
--- a/frontend/src/metabase/query_builder/actions/core/core.js
+++ b/frontend/src/metabase/query_builder/actions/core/core.js
@@ -30,11 +30,11 @@ import {
   getCard,
   getIsResultDirty,
   getOriginalQuestion,
+  getParameters,
   getQuestion,
   getResultsMetadata,
   getTransformedSeries,
   isBasedOnExistingQuestion,
-  getParameters,
 } from "../../selectors";
 import { updateUrl } from "../navigation";
 import { zoomInRow } from "../object-detail";
@@ -248,10 +248,17 @@ export const apiUpdateQuestion = (question, { rerunQuery } = {}) => {
     const originalQuestion = getOriginalQuestion(getState());
     question = question || getQuestion(getState());
 
-    const resultsMetadata = getResultsMetadata(getState());
+    let resultsMetadata = getResultsMetadata(getState());
     const isResultDirty = getIsResultDirty(getState());
 
     if (question.isDataset()) {
+      if (!resultsMetadata) {
+        // Running the question will populate results metadata in redux store.
+        // Without it getSubmittableQuestion won't have all the necessary information.
+        await dispatch(runQuestionQuery());
+      }
+
+      resultsMetadata = getResultsMetadata(getState());
       resultsMetadata.columns = ModelIndexes.actions.cleanIndexFlags(
         resultsMetadata.columns,
       );
diff --git a/frontend/src/metabase/query_builder/actions/querying.js b/frontend/src/metabase/query_builder/actions/querying.js
index ab0af65a33763c..4df7cebdca81f2 100644
--- a/frontend/src/metabase/query_builder/actions/querying.js
+++ b/frontend/src/metabase/query_builder/actions/querying.js
@@ -121,7 +121,7 @@ export const runQuestionQuery = ({
 
     const queryTimer = startTimer();
 
-    apiRunQuestionQuery(question, {
+    const runQuestionPromise = apiRunQuestionQuery(question, {
       cancelDeferred: cancelQueryDeferred,
       ignoreCache: ignoreCache,
       isDirty: cardIsDirty,
@@ -140,6 +140,8 @@ export const runQuestionQuery = ({
       .catch(error => dispatch(queryErrored(startTime, error)));
 
     dispatch({ type: RUN_QUERY, payload: { cancelQueryDeferred } });
+
+    return runQuestionPromise;
   };
 };