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 0b51201 commit d26b782
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 7 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");
});
});
});
15 changes: 9 additions & 6 deletions frontend/src/metabase/query_builder/actions/core/core.js
Expand Up @@ -30,11 +30,11 @@ import {
getCard,
getIsResultDirty,
getOriginalQuestion,
getParameters,
getQuestion,
getResultsMetadata,
isBasedOnExistingQuestion,
getParameters,
getSubmittableQuestion,
isBasedOnExistingQuestion,
} from "../../selectors";
import { updateUrl } from "../navigation";
import { zoomInRow } from "../object-detail";
Expand Down Expand Up @@ -235,14 +235,17 @@ export const apiUpdateQuestion = (question, { rerunQuery } = {}) => {
const originalQuestion = getOriginalQuestion(getState());
question = question || getQuestion(getState());

const resultsMetadata = getResultsMetadata(getState());
const isResultDirty = getIsResultDirty(getState());
const isModel = question.type() === "model";

if (isModel) {
resultsMetadata.columns = ModelIndexes.actions.cleanIndexFlags(
resultsMetadata.columns,
);
const resultsMetadata = getResultsMetadata(getState());

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());
}
}

const { isNative } = Lib.queryDisplayInfo(question.query());
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
7 changes: 7 additions & 0 deletions frontend/src/metabase/query_builder/selectors.js
Expand Up @@ -17,6 +17,7 @@ import {
import { getComputedSettingsForSeries } from "metabase/visualizations/lib/settings/visualization";

import Databases from "metabase/entities/databases";
import { ModelIndexes } from "metabase/entities/model-indexes";
import Timelines from "metabase/entities/timelines";

import { getAlerts } from "metabase/alert/selectors";
Expand Down Expand Up @@ -1053,6 +1054,12 @@ export const getSubmittableQuestion = (state, question) => {
const resultsMetadata = getResultsMetadata(state);
const isResultDirty = getIsResultDirty(state);

if (question.type() === "model" && resultsMetadata) {
resultsMetadata.columns = ModelIndexes.actions.cleanIndexFlags(
resultsMetadata.columns,
);
}

let submittableQuestion = question;

if (series) {
Expand Down

0 comments on commit d26b782

Please sign in to comment.