Make sure models keep their original question when duplicating (#31687)

e2e/test/scenarios/models/reproductions/31309-duplicating-a-model-must-duplicate-question.cy.spec.js

+import {
+  getNotebookStep,
+  modal,
+  openQuestionActions,
+  popover,
+  restore,
+} from "e2e/support/helpers";
+
+import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
+import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
+
+const { ORDERS, ORDERS_ID, PEOPLE } = SAMPLE_DATABASE;
+
+const TEST_QUERY = {
+  "order-by": [["asc", ["field", "sum", { "base-type": "type/Float" }]]],
+  limit: 10,
+  filter: ["<", ["field", "sum", { "base-type": "type/Float" }], 100],
+  "source-query": {
+    "source-table": ORDERS_ID,
+    aggregation: [["sum", ["field", ORDERS.TOTAL, null]]],
+    breakout: [
+      [
+        "field",
+        PEOPLE.NAME,
+        { "base-type": "type/Text", "source-field": ORDERS.USER_ID },
+      ],
+    ],
+  },
+};
+
+describe("issue 31309", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+  });
+
+  it("should duplicate a model with its original aggregation and breakout", () => {
+    cy.createQuestion(
+      {
+        name: "model",
+        query: TEST_QUERY,
+        type: "query",
+        database: SAMPLE_DB_ID,
+        dataset: true,
+      },
+      {
+        visitQuestion: true,
+      },
+    );
+
+    openQuestionActions();
+    popover().findByText("Duplicate").click();
+
+    modal().within(() => {
+      cy.findByText("Duplicate").click();
+    });
+
+    modal().within(() => {
+      cy.findByText("Not now").click();
+    });
+
+    openQuestionActions();
+    popover().within(() => {
+      cy.findByText("Edit query definition").click();
+    });
+
+    cy.findByTestId("data-step-cell").findByText("Orders").should("exist");
+
+    cy.findByTestId("aggregate-step")
+      .findByText("Sum of Total")
+      .should("exist");
+
+    cy.findByTestId("breakout-step").findByText("User → Name").should("exist");
+
+    getNotebookStep("filter", { stage: 1, index: 0 })
+      .findByText("Sum of Total is less than 100")
+      .should("exist");
+
+    getNotebookStep("sort", { stage: 1, index: 0 })
+      .findByText("Sum of Total")
+      .should("exist");
+
+    getNotebookStep("limit", { stage: 1, index: 0 })
+      .findByDisplayValue("10")
+      .should("exist");
+  });
+});

frontend/src/metabase/query_builder/components/QueryModals.tsx

@@ -31,7 +31,12 @@ import PreviewQueryModal from "metabase/query_builder/components/view/PreviewQue
 import ConvertQueryModal from "metabase/query_builder/components/view/ConvertQueryModal";
 import QuestionMoveToast from "metabase/questions/components/QuestionMoveToast";
 import { Alert, Card, Collection, User } from "metabase-types/api";
-import { QueryBuilderMode, QueryBuilderUIControls } from "metabase-types/store";
+import {
+  QueryBuilderMode,
+  QueryBuilderUIControls,
+  State,
+} from "metabase-types/store";
+import { getQuestionWithParameters } from "metabase/query_builder/selectors";
 import StructuredQuery from "metabase-lib/queries/StructuredQuery";
 import Question from "metabase-lib/Question";
 import { UpdateQuestionOpts } from "../actions/core/updateQuestion";
@@ -40,6 +45,10 @@ const mapDispatchToProps = {
   setQuestionCollection: Questions.actions.setCollection,
 };
 
+const mapStateToProps = (state: State) => ({
+  questionWithParameters: getQuestionWithParameters(state) as Question,
+});
+
 type ModalType = typeof MODAL_TYPES[keyof typeof MODAL_TYPES];
 
 interface QueryModalsProps {
@@ -52,7 +61,8 @@ interface QueryModalsProps {
   updateQuestion: (question: Question, config?: UpdateQuestionOpts) => void;
   setQueryBuilderMode: (mode: QueryBuilderMode) => void;
   setUIControls: (opts: Partial<QueryBuilderUIControls>) => void;
-  originalQuestion: Question | null;
+  originalQuestion: Question;
+  questionWithParameters: Question;
   card: Card;
   onCreate: (question: Question) => void;
   onSave: (question: Question, config?: { rerunQuery: boolean }) => void;
@@ -95,6 +105,7 @@ class QueryModals extends Component<QueryModalsProps> {
       modal,
       modalContext,
       question,
+      questionWithParameters,
       initialCollectionId,
       onCloseModal,
       onOpenModal,
@@ -286,11 +297,12 @@ class QueryModals extends Component<QueryModalsProps> {
               }}
               copy={async formValues => {
                 const object = await this.props.onCreate(
-                  question
+                  questionWithParameters
                     .setDisplayName(formValues.name)
                     .setCollectionId(formValues.collection_id)
                     .setDescription(formValues.description || null),
                 );
+
                 return { payload: { object } };
               }}
               onClose={onCloseModal}
@@ -359,4 +371,4 @@ class QueryModals extends Component<QueryModalsProps> {
 }
 
 // eslint-disable-next-line import/no-default-export -- deprecated usage
-export default connect(null, mapDispatchToProps)(QueryModals);
+export default connect(mapStateToProps, mapDispatchToProps)(QueryModals);

frontend/src/metabase/query_builder/selectors.js

@@ -314,14 +314,21 @@ export const getLastRunQuestion = createSelector(
     card && metadata && new Question(card, metadata, parameterValues),
 );
 
+export const getQuestionWithParameters = createSelector(
+  [getCard, getMetadata, getParameterValues],
+  (card, metadata, parameterValues) => {
+    if (!card || !metadata) {
+      return;
+    }
+    return new Question(card, metadata, parameterValues);
+  },
+);
 export const getQuestion = createSelector(
-  [getMetadata, getCard, getParameterValues, getQueryBuilderMode],
-  (metadata, card, parameterValues, queryBuilderMode) => {
-    if (!metadata || !card) {
+  [getQuestionWithParameters, getQueryBuilderMode],
+  (question, queryBuilderMode) => {
+    if (!question) {
       return;
     }
-    const question = new Question(card, metadata, parameterValues);
-
     const isEditingModel = queryBuilderMode === "dataset";
     if (isEditingModel) {
       return question.lockDisplay();