backpot "[MLv2] Handle Object.isFrozen() legacy columns as input (#…

…39032)"

Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>

frontend/src/metabase-lib/comparison.ts

@@ -2,7 +2,7 @@ import * as ML from "cljs/metabase.lib.js";
 import type {
   DatasetColumn,
   DatasetQuery,
-  FieldReference,
+  DimensionReference,
 } from "metabase-types/api";
 
 import type { ColumnMetadata, Query } from "./types";
@@ -28,7 +28,7 @@ export function findColumnIndexesFromLegacyRefs(
   query: Query,
   stageIndex: number,
   columns: ColumnMetadata[] | DatasetColumn[],
-  fieldRefs: FieldReference[],
+  fieldRefs: DimensionReference[],
 ): number[] {
   return ML.find_column_indexes_from_legacy_refs(
     query,

frontend/src/metabase-lib/comparison.unit.spec.ts

@@ -0,0 +1,30 @@
+import { freeze } from "immer";
+
+import * as Lib from "metabase-lib";
+import {
+  createOrdersTaxDatasetColumn,
+  createOrdersTotalDatasetColumn,
+} from "metabase-types/api/mocks/presets";
+import { createQuery } from "metabase-lib/test-helpers";
+
+describe("findColumnIndexesFromLegacyRefs", () => {
+  it("works even on frozen columns and refs", () => {
+    const query = createQuery();
+    const stageIndex = -1;
+    const columns = freeze(
+      [createOrdersTotalDatasetColumn(), createOrdersTaxDatasetColumn()],
+      true,
+    );
+
+    const columnIndexes = Lib.findColumnIndexesFromLegacyRefs(
+      query,
+      stageIndex,
+      columns,
+      columns.map(({ field_ref }) => field_ref!),
+    );
+
+    expect(Object.isFrozen(columns[0])).toBe(true);
+    expect(Object.isFrozen(columns[1])).toBe(true);
+    expect(columnIndexes).toEqual([0, 1]);
+  });
+});

src/metabase/lib/js/metadata.cljs

@@ -292,6 +292,17 @@
                                           (walk/keywordize-keys v)
                                           (js->clj v :keywordize-keys true))
       :has-field-values                 (keyword v)
+
+      ;; Field refs are JS arrays, which we do not alter but do need to clone.
+      ;; Why? Come sit by the fire, it's story time:
+      ;; Sometimes in the FE the input `DatasetColumn` object is coming from the Redux store, where it has been deeply
+      ;; frozen (Object.freeze()) by the immer library.
+      ;; `:metadata/column` values (which contain such a :field-ref) are sometimes used as a map key, which calls
+      ;; [[cljs.core/hash]], which for a vanilla JS array uses goog.getUid() to mutate a uid number onto the array with
+      ;; a key like `closure_uid_123456789` (the number is randomized at load time).
+      ;; If the array has been frozen, that mutation will throw. So we clone the `:field-ref` array on its way into CLJS
+      ;; land, and avoid the issue.
+      :field-ref                        (to-array v)
       :lib/source                       (case v
                                           "aggregation" :source/aggregations
                                           "breakout"    :source/breakouts