Global E2E Config: Solve circular dependency issue in helpers (#20741)

* Add `SAMPLE_DB_TABLES` reference

* Add notes to the cypress data references

* Ensure table ids are correct

Early warning mechanism.

* Use SAMPLE_DB_TABLES in e2e helpers and custom commands

* Use `SAMPLE_DB_ID` in the previously missed helper

frontend/test/__support__/e2e/commands/permissions/sandboxTable.js

-import { USER_GROUPS } from "__support__/e2e/cypress_data";
+import { USER_GROUPS, SAMPLE_DB_TABLES } from "__support__/e2e/cypress_data";
+
+const { STATIC_ORDERS_ID } = SAMPLE_DB_TABLES;
 
 const { COLLECTION_GROUP } = USER_GROUPS;
 
@@ -8,7 +10,7 @@ Cypress.Commands.add(
     attribute_remappings = {},
     card_id = null,
     group_id = COLLECTION_GROUP,
-    table_id = 2,
+    table_id = STATIC_ORDERS_ID,
   } = {}) => {
     // Extract the name of the table, as well as `schema` and `db_id` that we'll need later on for `cy.updatePermissionsSchemas()`
     cy.request("GET", "/api/table").then(({ body: tables }) => {

frontend/test/__support__/e2e/cypress_data.js

+/**
+ * We are keeping the references to most commonly used ids and objects in this file.
+ *
+ * Please note that these ids are hard coded and might change if sample database changes in the future!
+ * For that reason, we have some sanity checks in the `default.cy.snap.js` spec.
+ *
+ * SAMPLE_DB_TABLES contains only the references to the four main tables ids in sample database.
+ * We need these references to avoid circular dependecy issue in custom commands and e2e helpers.
+ * That is the only place they should be used. NEVER use them in tests!
+ *
+ * USER_GROUPS
+ * Although they are also hard coded, the assertions are put in place in the default snapshot generator
+ * that would break if the actual ids change. Unlike SAMPLE_DB_TABLES which depend on the order of SQL
+ * commands used to create the sample database, USER_GROUPS depend on the order in which we create new user groups.
+ *
+ * As a general note, whenever you add a new reference to this file, please make sure there is a trigger somewhere
+ * that would break and alert us if expected and actual values don't match.
+ */
+
 export const SAMPLE_DB_ID = 1;
 
+// Use only for e2e helpers and custom commands. Never in e2e tests directly!
+export const SAMPLE_DB_TABLES = {
+  STATIC_PRODUCTS_ID: 1,
+  STATIC_ORDERS_ID: 2,
+  STATIC_PEOPLE_ID: 3,
+  STATIC_REVIEWS_ID: 4,
+};
+
+// All users and admin groups are the defaults that come with Metabase.
+// The rest are the ones we choose the name and the order for.
 export const USER_GROUPS = {
   ALL_USERS_GROUP: 1,
   ADMIN_GROUP: 2,

frontend/test/__support__/e2e/helpers/e2e-ad-hoc-question-helpers.js

-import { SAMPLE_DB_ID } from "__support__/e2e/cypress_data";
+import { SAMPLE_DB_ID, SAMPLE_DB_TABLES } from "__support__/e2e/cypress_data";
+
+const {
+  STATIC_ORDERS_ID,
+  STATIC_PRODUCTS_ID,
+  STATIC_PEOPLE_ID,
+  STATIC_REVIEWS_ID,
+} = SAMPLE_DB_TABLES;
 
 export function adhocQuestionHash(question) {
   if (question.display) {
@@ -56,19 +63,19 @@ export function openTable({
 }
 
 export function openProductsTable({ mode, limit, callback } = {}) {
-  return openTable({ table: 1, mode, limit, callback });
+  return openTable({ table: STATIC_PRODUCTS_ID, mode, limit, callback });
 }
 
 export function openOrdersTable({ mode, limit, callback } = {}) {
-  return openTable({ table: 2, mode, limit, callback });
+  return openTable({ table: STATIC_ORDERS_ID, mode, limit, callback });
 }
 
 export function openPeopleTable({ mode, limit, callback } = {}) {
-  return openTable({ table: 3, mode, limit, callback });
+  return openTable({ table: STATIC_PEOPLE_ID, mode, limit, callback });
 }
 
 export function openReviewsTable({ mode, limit, callback } = {}) {
-  return openTable({ table: 4, mode, limit, callback });
+  return openTable({ table: STATIC_REVIEWS_ID, mode, limit, callback });
 }
 
 function getInterceptDetails(question, mode) {

frontend/test/__support__/e2e/helpers/e2e-database-metadata-helpers.js

+import { SAMPLE_DB_ID } from "__support__/e2e/cypress_data";
+
 export function withDatabase(databaseId, f) {
   cy.request("GET", `/api/database/${databaseId}/metadata`).then(({ body }) => {
     const database = {};
@@ -14,5 +16,5 @@ export function withDatabase(databaseId, f) {
 }
 
 export function withSampleDatabase(f) {
-  return withDatabase(1, f);
+  return withDatabase(SAMPLE_DB_ID, f);
 }

frontend/test/snapshot-creators/default.cy.snap.js

 import _ from "underscore";
 import { snapshot, restore, withSampleDatabase } from "__support__/e2e/cypress";
-import { USERS, USER_GROUPS, SAMPLE_DB_ID } from "__support__/e2e/cypress_data";
+import {
+  USERS,
+  USER_GROUPS,
+  SAMPLE_DB_ID,
+  SAMPLE_DB_TABLES,
+} from "__support__/e2e/cypress_data";
+
+const {
+  STATIC_ORDERS_ID,
+  STATIC_PRODUCTS_ID,
+  STATIC_REVIEWS_ID,
+  STATIC_PEOPLE_ID,
+} = SAMPLE_DB_TABLES;
 
 const {
   ALL_USERS_GROUP,
@@ -20,6 +32,7 @@ describe("snapshots", () => {
       addUsersAndGroups();
       createCollections();
       withSampleDatabase(SAMPLE_DATABASE => {
+        ensureTableIdsAreCorrect(SAMPLE_DATABASE);
         createQuestionsAndDashboards(SAMPLE_DATABASE);
         cy.writeFile(
           "frontend/test/__support__/e2e/cypress_sample_database.json",
@@ -188,6 +201,18 @@ describe("snapshots", () => {
     });
   }
 
+  function ensureTableIdsAreCorrect({
+    ORDERS_ID,
+    PRODUCTS_ID,
+    REVIEWS_ID,
+    PEOPLE_ID,
+  }) {
+    expect(ORDERS_ID).to.eq(STATIC_ORDERS_ID);
+    expect(PEOPLE_ID).to.eq(STATIC_PEOPLE_ID);
+    expect(REVIEWS_ID).to.eq(STATIC_REVIEWS_ID);
+    expect(PRODUCTS_ID).to.eq(STATIC_PRODUCTS_ID);
+  }
+
   // TODO: It'd be nice to have one file per snapshot.
   // To do that we need to enforce execution order among them.
   describe("withSqlite", () => {