Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing result metadata #39201

Merged
merged 14 commits into from
Feb 28, 2024
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import {
focusNativeEditor,
main,
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 changes...").should("not.exist");
});

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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export const runQuestionQuery = ({

const queryTimer = startTimer();

apiRunQuestionQuery(question, {
const promise = 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 promise;
};
};

Expand Down
7 changes: 7 additions & 0 deletions frontend/src/metabase/query_builder/selectors.js
Original file line number Diff line number Diff line change
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
Loading