[E2E] Refactor question commands to function helpers (#40236) (#40287)

* Extract question function helpers

* Move question helpers

* Remove cy.archiveQuestion

* Replace cy.createQuestion usages in TS files

* Format code in JSDocs

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>

e2e/support/commands/api/question.ts

-import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
-import type {
-  Card,
-  DatasetQuery,
-  NativeQuery,
-  StructuredQuery,
-} from "metabase-types/api";
+import { createNativeQuestion, createQuestion } from "e2e/support/helpers";
 
 declare global {
   namespace Cypress {
     interface Chainable {
-      archiveQuestion(
-        id: Card["id"],
-      ): Cypress.Chainable<Cypress.Response<Card>>;
-      createQuestion(
-        questionDetails: StructuredQuestionDetails,
-        options?: Options,
-      ): Cypress.Chainable<Cypress.Response<Card>>;
-      createNativeQuestion(
-        questionDetails: NativeQuestionDetails,
-        options?: Options,
-      ): Cypress.Chainable<Cypress.Response<Card>>;
-    }
-  }
-}
-
-type QuestionDetails = {
-  dataset_query: DatasetQuery;
-  /**
-   * Defaults to "test question".
-   */
-  name?: Card["name"];
-  description?: Card["description"];
-  /**
-   * Entity type.
-   * Defaults to "question".
-   */
-  type?: Card["type"];
-  /**
-   * Defaults to "table".
-   */
-  display?: Card["display"];
-  parameters?: Card["parameters"];
-  visualization_settings?: Card["visualization_settings"];
-  /**
-   * Parent collection in which to store this question.
-   */
-  collection_id?: Card["collection_id"];
-  /**
-   * Used on the frontend to determine whether the question is pinned or not.
-   */
-  collection_position?: Card["collection_position"];
-  embedding_params?: Card["embedding_params"];
-  /**
-   * Defaults to false.
-   */
-  enable_embedding?: Card["enable_embedding"];
-};
-
-type StructuredQuestionDetails = Omit<QuestionDetails, "dataset_query"> & {
-  /**
-   * Defaults to SAMPLE_DB_ID.
-   */
-  database?: DatasetQuery["database"];
-  query: StructuredQuery;
-};
-
-type NativeQuestionDetails = Omit<QuestionDetails, "dataset_query"> & {
-  /**
-   * Defaults to SAMPLE_DB_ID.
-   */
-  database?: DatasetQuery["database"];
-  native: NativeQuery;
-};
-
-type Options = {
-  /**
-   * Whether to visit the question in order to load its metadata.
-   * Defaults to false.
-   */
-  loadMetadata?: boolean;
-  /**
-   * Whether to visit the question after the creation or not.
-   * Defaults to false.
-   */
-  visitQuestion?: boolean;
-  /**
-   * Whether to wrap a question id, to make it available outside of this scope.
-   * Defaults to false.
-   */
-  wrapId?: boolean;
-  /**
-   * Alias a question id in order to use it later with `cy.get("@" + alias).
-   * Defaults to "questionId".
-   */
-  idAlias?: string;
-  /**
-   * We need distinctive endpoint aliases for cases where we have multiple questions or nested questions.
-   * Defaults to "cardQuery".
-   */
-  interceptAlias?: string;
-};
-
-Cypress.Commands.add("archiveQuestion", (id: Card["id"]) => {
-  cy.log(`Archiving a question with id: ${id}`);
-
-  return cy.request("PUT", `/api/card/${id}`, {
-    archived: true,
-  });
-});
-
-Cypress.Commands.add(
-  "createQuestion",
-  (questionDetails: StructuredQuestionDetails, options?: Options) => {
-    const { database = SAMPLE_DB_ID, name, query } = questionDetails;
-
-    if (!query) {
-      throw new Error('"query" attribute missing in questionDetails');
-    }
-
-    logAction("Create a QB question", name);
-
-    return question(
-      {
-        ...questionDetails,
-        dataset_query: { type: "query", query, database },
-      },
-      options,
-    );
-  },
-);
-
-Cypress.Commands.add(
-  "createNativeQuestion",
-  (questionDetails: NativeQuestionDetails, options?: Options) => {
-    const { database = SAMPLE_DB_ID, name, native } = questionDetails;
-
-    if (!native) {
-      throw new Error('"native" attribute missing in questionDetails');
-    }
-
-    logAction("Create a native question", name);
-
-    return question(
-      {
-        ...questionDetails,
-        dataset_query: { type: "native", native, database },
-      },
-      options,
-    );
-  },
-);
-
-const question = (
-  {
-    name = "test question",
-    description,
-    dataset_query,
-    type = "question",
-    display = "table",
-    parameters,
-    visualization_settings = {},
-    collection_id,
-    collection_position,
-    embedding_params,
-    enable_embedding = false,
-  }: QuestionDetails,
-  {
-    loadMetadata = false,
-    visitQuestion = false,
-    wrapId = false,
-    idAlias = "questionId",
-    interceptAlias = "cardQuery",
-  }: Options = {},
-) => {
-  return cy
-    .request("POST", "/api/card", {
-      name,
-      description,
-      dataset_query,
-      display,
-      parameters,
-      visualization_settings,
-      collection_id,
-      collection_position,
-    })
-    .then(({ body }) => {
       /**
-       * Optionally, if you need question's id later in the test, outside the scope of this function,
-       * you can use it like this:
-       *
-       * `cy.get("@questionId").then(id=> {
-       *   doSomethingWith(id);
-       * })
+       * @deprecated Use function helper instead, i.e.
+       * ```
+       * import { createQuestion } from "e2e/support/helpers"
+       * ```
        */
-      if (wrapId) {
-        cy.wrap(body.id).as(idAlias);
-      }
-
-      if (type === "model" || enable_embedding) {
-        cy.request("PUT", `/api/card/${body.id}`, {
-          type,
-          enable_embedding,
-          embedding_params,
-        });
-      }
+      createQuestion: typeof createQuestion;
 
-      if (loadMetadata || visitQuestion) {
-        if (type === "model") {
-          cy.intercept("POST", "/api/dataset").as("dataset");
-          cy.visit(`/model/${body.id}`);
-          cy.wait("@dataset"); // Wait for `result_metadata` to load
-        } else {
-          // We need to use the wildcard because endpoint for pivot tables has the following format: `/api/card/pivot/${id}/query`
-          cy.intercept("POST", `/api/card/**/${body.id}/query`).as(
-            interceptAlias,
-          );
-          cy.visit(`/question/${body.id}`);
-          cy.wait("@" + interceptAlias); // Wait for `result_metadata` to load
-        }
-      }
-    });
-};
-
-const logAction = (
-  /**
-   * A title used to log the Cypress action/request that follows it.
-   */
-  title: string,
-  /**
-   * Optional question name.
-   */
-  questionName?: string,
-) => {
-  const fullTitle = `${title}: ${questionName}`;
-  const message = questionName ? fullTitle : title;
+      /**
+       * @deprecated Use function helper instead, i.e.
+       * ```
+       * import { createNativeQuestion } from "e2e/support/helpers"
+       * ```
+       */
+      createNativeQuestion: typeof createNativeQuestion;
+    }
+  }
+}
 
-  cy.log(message);
-};
+Cypress.Commands.add("createQuestion", createQuestion);
+Cypress.Commands.add("createNativeQuestion", createNativeQuestion);

e2e/support/helpers/e2e-question-helpers.ts

+import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
+import type {
+  Card,
+  DatasetQuery,
+  NativeQuery,
+  StructuredQuery,
+} from "metabase-types/api";
+
+type QuestionDetails = {
+  dataset_query: DatasetQuery;
+  /**
+   * Defaults to "test question".
+   */
+  name?: Card["name"];
+  description?: Card["description"];
+  /**
+   * Entity type.
+   * Defaults to "question".
+   */
+  type?: Card["type"];
+  /**
+   * Defaults to "table".
+   */
+  display?: Card["display"];
+  parameters?: Card["parameters"];
+  visualization_settings?: Card["visualization_settings"];
+  /**
+   * Parent collection in which to store this question.
+   */
+  collection_id?: Card["collection_id"];
+  /**
+   * Used on the frontend to determine whether the question is pinned or not.
+   */
+  collection_position?: Card["collection_position"];
+  embedding_params?: Card["embedding_params"];
+  /**
+   * Defaults to false.
+   */
+  enable_embedding?: Card["enable_embedding"];
+};
+
+export type StructuredQuestionDetails = Omit<
+  QuestionDetails,
+  "dataset_query"
+> & {
+  /**
+   * Defaults to SAMPLE_DB_ID.
+   */
+  database?: DatasetQuery["database"];
+  query: StructuredQuery;
+};
+
+export type NativeQuestionDetails = Omit<QuestionDetails, "dataset_query"> & {
+  /**
+   * Defaults to SAMPLE_DB_ID.
+   */
+  database?: DatasetQuery["database"];
+  native: NativeQuery;
+};
+
+type Options = {
+  /**
+   * Whether to visit the question in order to load its metadata.
+   * Defaults to false.
+   */
+  loadMetadata?: boolean;
+  /**
+   * Whether to visit the question after the creation or not.
+   * Defaults to false.
+   */
+  visitQuestion?: boolean;
+  /**
+   * Whether to wrap a question id, to make it available outside of this scope.
+   * Defaults to false.
+   */
+  wrapId?: boolean;
+  /**
+   * Alias a question id in order to use it later with `cy.get("@" + alias).
+   * Defaults to "questionId".
+   */
+  idAlias?: string;
+  /**
+   * We need distinctive endpoint aliases for cases where we have multiple questions or nested questions.
+   * Defaults to "cardQuery".
+   */
+  interceptAlias?: string;
+};
+
+export const createQuestion = (
+  questionDetails: StructuredQuestionDetails,
+  options?: Options,
+): Cypress.Chainable<Cypress.Response<Card>> => {
+  const { database = SAMPLE_DB_ID, name, query } = questionDetails;
+
+  if (!query) {
+    throw new Error('"query" attribute missing in questionDetails');
+  }
+
+  logAction("Create a QB question", name);
+
+  return question(
+    {
+      ...questionDetails,
+      dataset_query: { type: "query", query, database },
+    },
+    options,
+  );
+};
+
+export const createNativeQuestion = (
+  questionDetails: NativeQuestionDetails,
+  options?: Options,
+): Cypress.Chainable<Cypress.Response<Card>> => {
+  const { database = SAMPLE_DB_ID, name, native } = questionDetails;
+
+  if (!native) {
+    throw new Error('"native" attribute missing in questionDetails');
+  }
+
+  logAction("Create a native question", name);
+
+  return question(
+    {
+      ...questionDetails,
+      dataset_query: { type: "native", native, database },
+    },
+    options,
+  );
+};
+
+export const archiveQuestion = (
+  id: Card["id"],
+): Cypress.Chainable<Cypress.Response<Card>> => {
+  cy.log(`Archiving a question with id: ${id}`);
+
+  return cy.request("PUT", `/api/card/${id}`, {
+    archived: true,
+  });
+};
+
+const question = (
+  {
+    name = "test question",
+    description,
+    dataset_query,
+    type = "question",
+    display = "table",
+    parameters,
+    visualization_settings = {},
+    collection_id,
+    collection_position,
+    embedding_params,
+    enable_embedding = false,
+  }: QuestionDetails,
+  {
+    loadMetadata = false,
+    visitQuestion = false,
+    wrapId = false,
+    idAlias = "questionId",
+    interceptAlias = "cardQuery",
+  }: Options = {},
+) => {
+  return cy
+    .request("POST", "/api/card", {
+      name,
+      description,
+      dataset_query,
+      display,
+      parameters,
+      visualization_settings,
+      collection_id,
+      collection_position,
+    })
+    .then(({ body }) => {
+      /**
+       * Optionally, if you need question's id later in the test, outside the scope of this function,
+       * you can use it like this:
+       *
+       * `cy.get("@questionId").then(id=> {
+       *   doSomethingWith(id);
+       * })
+       */
+      if (wrapId) {
+        cy.wrap(body.id).as(idAlias);
+      }
+
+      if (type === "model" || enable_embedding) {
+        cy.request("PUT", `/api/card/${body.id}`, {
+          type,
+          enable_embedding,
+          embedding_params,
+        });
+      }
+
+      if (loadMetadata || visitQuestion) {
+        if (type === "model") {
+          cy.intercept("POST", "/api/dataset").as("dataset");
+          cy.visit(`/model/${body.id}`);
+          cy.wait("@dataset"); // Wait for `result_metadata` to load
+        } else {
+          // We need to use the wildcard because endpoint for pivot tables has the following format: `/api/card/pivot/${id}/query`
+          cy.intercept("POST", `/api/card/**/${body.id}/query`).as(
+            interceptAlias,
+          );
+          cy.visit(`/question/${body.id}`);
+          cy.wait("@" + interceptAlias); // Wait for `result_metadata` to load
+        }
+      }
+    });
+};
+
+const logAction = (
+  /**
+   * A title used to log the Cypress action/request that follows it.
+   */
+  title: string,
+  /**
+   * Optional question name.
+   */
+  questionName?: string,
+) => {
+  const fullTitle = `${title}: ${questionName}`;
+  const message = questionName ? fullTitle : title;
+
+  cy.log(message);
+};

e2e/support/helpers/index.js

@@ -26,6 +26,7 @@ export * from "./e2e-bi-basics-helpers";
 export * from "./e2e-boolean-helpers";
 export * from "./e2e-embedding-helpers";
 export * from "./e2e-permissions-helpers";
+export * from "./e2e-question-helpers";
 export * from "./e2e-request-helpers";
 export * from "./e2e-visual-tests-helpers";
 export * from "./e2e-users-helpers";

e2e/test/scenarios/collections/archive.cy.spec.js

@@ -3,7 +3,7 @@ import {
   FIRST_COLLECTION_ID,
   READ_ONLY_PERSONAL_COLLECTION_ID,
 } from "e2e/support/cypress_sample_instance_data";
-import { restore } from "e2e/support/helpers";
+import { archiveQuestion, restore } from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID, PEOPLE_ID } = SAMPLE_DATABASE;
 
@@ -47,7 +47,7 @@ describe("scenarios > collections > archive", () => {
         ],
       },
     }).then(({ body: { id } }) => {
-      cy.archiveQuestion(id);
+      archiveQuestion(id);
     });
 
     cy.visit("/archive");

e2e/test/scenarios/models/reproductions/25884-34349-null-description-in-column-metadata.cy.spec.ts

 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
-import { openQuestionActions, popover, restore } from "e2e/support/helpers";
+import {
+  createQuestion,
+  openQuestionActions,
+  popover,
+  restore,
+} from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE;
 
@@ -13,7 +18,7 @@ describe("issues 25884 and 34349", () => {
   });
 
   it("should show empty description input for columns without description in metadata (metabase#25884, metabase#34349)", () => {
-    cy.createQuestion(
+    createQuestion(
       {
         type: "model",
         query: {

e2e/test/scenarios/models/reproductions/29943-custom-column-in-metadata.cy.spec.ts

 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
-import { openQuestionActions, popover, restore } from "e2e/support/helpers";
+import {
+  createQuestion,
+  openQuestionActions,
+  popover,
+  restore,
+} from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE;
 
@@ -11,7 +16,7 @@ describe("issue 29943", () => {
   });
 
   it("selects the right column when clicking a column header (metabase#29943)", () => {
-    cy.createQuestion(
+    createQuestion(
       {
         type: "model",
         query: {

e2e/test/scenarios/models/reproductions/35711-metadata-in-models-with-custom-fields.cy.spec.ts

 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
-import { openQuestionActions, popover, restore } from "e2e/support/helpers";
+import {
+  createQuestion,
+  openQuestionActions,
+  popover,
+  restore,
+} from "e2e/support/helpers";
 import type { FieldReference } from "metabase-types/api";
 
 const { ORDERS_ID, ORDERS } = SAMPLE_DATABASE;
@@ -19,7 +24,7 @@ describe("issue 35711", () => {
   });
 
   it("can edit metadata of a model with a custom column (metabase#35711)", () => {
-    cy.createQuestion(
+    createQuestion(
       {
         type: "model",
         query: {

e2e/test/scenarios/permissions/reproductions/11994-dirty-question.cy.spec.ts

 import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
-import { restore, visitQuestion } from "e2e/support/helpers";
+import { createQuestion, restore, visitQuestion } from "e2e/support/helpers";
 import type {
   ConcreteFieldReference,
   StructuredQuery,
@@ -35,7 +35,7 @@ describe("issue 11994", () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();
-    cy.createQuestion(
+    createQuestion(
       {
         database: SAMPLE_DB_ID,
         query: QUERY,
@@ -61,7 +61,7 @@ describe("issue 11994", () => {
       },
       { wrapId: true, idAlias: "pivotQuestionId" },
     );
-    cy.createQuestion(
+    createQuestion(
       {
         database: SAMPLE_DB_ID,
         query: QUERY,