Handle mutation of vizSetting column when syncing columns in a native question (#17279)

* Handle mutation of vizSetting column when syncing columns of a native question

When calling Question's _syncNativeQuerySettings method we need to treat
existing, mutated columns as invalid so that they are filtered out and a
new column takes the existing column's place.

The function we use to find valid columns, findColumnIndexForColumnSetting,
will return true if it finds a matching field_ref OR a matching name, so we
also need to check the addedColumns array we just created in order to remove
any columns from vizSettings that also exist there, since we combine the two
at the end of this method.

* add unit tests for _syncNativeQuerySettings

frontend/src/metabase-lib/lib/Question.js

@@ -624,8 +624,11 @@ export default class Question {
 
     const validVizSettings = vizSettings.filter(colSetting => {
       const hasColumn = findColumnIndexForColumnSetting(cols, colSetting) >= 0;
-      return hasColumn;
+      const isMutatingColumn =
+        findColumnIndexForColumnSetting(addedColumns, colSetting) >= 0;
+      return hasColumn && !isMutatingColumn;
     });
+
     const noColumnsRemoved = validVizSettings.length === vizSettings.length;
 
     if (noColumnsRemoved && addedColumns.length === 0) {

frontend/test/metabase-lib/lib/Question.unit.spec.js

@@ -636,4 +636,180 @@ describe("Question", () => {
       });
     });
   });
+
+  describe("Question.prototype._syncNativeQuerySettings", () => {
+    let question;
+    const cols = [
+      {
+        display_name: "num",
+        source: "native",
+        field_ref: [
+          "field",
+          "num",
+          {
+            "base-type": "type/Float",
+          },
+        ],
+        name: "num",
+        base_type: "type/Float",
+      },
+      {
+        display_name: "text",
+        source: "native",
+        field_ref: [
+          "field",
+          "text",
+          {
+            "base-type": "type/Text",
+          },
+        ],
+        name: "text",
+        base_type: "type/Text",
+      },
+    ];
+
+    const vizSettingCols = [
+      {
+        name: "num",
+        fieldRef: ["field", "num", { "base-type": "type/Float" }],
+        enabled: true,
+      },
+      {
+        name: "text",
+        fieldRef: ["field", "text", { "base-type": "type/Text" }],
+        enabled: true,
+      },
+    ];
+
+    beforeEach(() => {
+      question = new Question(native_orders_count_card, metadata);
+      question.setting = jest.fn();
+      question.updateSettings = jest.fn();
+    });
+
+    describe("when columns have not been defined", () => {
+      it("should do nothing when given no cols", () => {
+        question._syncNativeQuerySettings({});
+        question._syncNativeQuerySettings({ data: { cols: [] } });
+        question._syncNativeQuerySettings({ data: { cols } });
+
+        expect(question.updateSettings).not.toHaveBeenCalled();
+      });
+
+      it("should do nothing when given cols", () => {
+        question._syncNativeQuerySettings({ data: { cols } });
+
+        expect(question.updateSettings).not.toHaveBeenCalled();
+      });
+    });
+
+    describe("after vizSetting columns have been defined", () => {
+      beforeEach(() => {
+        question.setting.mockImplementation(property => {
+          if (property === "table.columns") {
+            return vizSettingCols;
+          }
+        });
+      });
+
+      it("should handle the addition and removal of columns", () => {
+        question._syncNativeQuerySettings({
+          data: {
+            cols: [
+              ...cols.slice(1),
+              {
+                display_name: "foo",
+                source: "native",
+                field_ref: [
+                  "field",
+                  "foo",
+                  {
+                    "base-type": "type/Float",
+                  },
+                ],
+                name: "foo",
+                base_type: "type/Float",
+              },
+            ],
+          },
+        });
+
+        expect(question.updateSettings).toHaveBeenCalledWith({
+          "table.columns": [
+            ...vizSettingCols.slice(1),
+            {
+              name: "foo",
+              fieldRef: [
+                "field",
+                "foo",
+                {
+                  "base-type": "type/Float",
+                },
+              ],
+              enabled: true,
+            },
+          ],
+        });
+      });
+
+      it("should handle the mutation of extraneous column props", () => {
+        question._syncNativeQuerySettings({
+          data: {
+            cols: [
+              {
+                display_name: "num with mutated display_name",
+                source: "native",
+                field_ref: [
+                  "field",
+                  "num",
+                  {
+                    "base-type": "type/Float",
+                  },
+                ],
+                name: "foo",
+                base_type: "type/Float",
+              },
+              ...cols.slice(1),
+            ],
+          },
+        });
+
+        expect(question.updateSettings).not.toHaveBeenCalled();
+      });
+
+      it("should handle the mutation of a field_ref on an existing column", () => {
+        question._syncNativeQuerySettings({
+          data: {
+            cols: [
+              {
+                display_name: "foo",
+                source: "native",
+                field_ref: [
+                  "field",
+                  "foo",
+                  {
+                    "base-type": "type/Integer",
+                  },
+                ],
+                name: "foo",
+                base_type: "type/Integer",
+              },
+              ...cols.slice(1),
+            ],
+          },
+        });
+
+        expect(question.updateSettings).toHaveBeenCalledWith({
+          "table.columns": [
+            ...vizSettingCols.slice(1),
+            {
+              name: "foo",
+              fieldRef: ["field", "foo", { "base-type": "type/Integer" }],
+              enabled: true,
+            },
+          ],
+        });
+      });
+    });
+  });
 });