Skip to content

Commit

Permalink
Fix missing result metadata (#39201)
Browse files Browse the repository at this point in the history
* 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 5ad3a8b.

* 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>
  • Loading branch information
kamilmielnik and metamben committed Feb 28, 2024
1 parent c6a236b commit 42a58fe
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 3 deletions.
@@ -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");
});
});
});
11 changes: 9 additions & 2 deletions frontend/src/metabase/query_builder/actions/core/core.js
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
);
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/metabase/query_builder/actions/querying.js
Expand Up @@ -121,7 +121,7 @@ export const runQuestionQuery = ({

const queryTimer = startTimer();

apiRunQuestionQuery(question, {
const runQuestionPromise = apiRunQuestionQuery(question, {
cancelDeferred: cancelQueryDeferred,
ignoreCache: ignoreCache,
isDirty: cardIsDirty,
Expand All @@ -140,6 +140,8 @@ export const runQuestionQuery = ({
.catch(error => dispatch(queryErrored(startTime, error)));

dispatch({ type: RUN_QUERY, payload: { cancelQueryDeferred } });

return runQuestionPromise;
};
};

Expand Down

0 comments on commit 42a58fe

Please sign in to comment.