Preserve template-tags key order (#11582)

frontend/src/metabase/query_builder/actions.js

@@ -629,15 +629,19 @@ export const updateTemplateTag = createThunkAction(
         delete updatedCard.description;
       }
 
-      // using updateIn instead of assocIn due to not preserving order of keys
+      // we need to preserve the order of the keys to avoid UI jumps
       return updateIn(
         updatedCard,
-        ["dataset_query", "native", "template-tags", templateTag.name],
-        tag =>
-          // when we switch type, null out any default
-          tag.type !== templateTag.type
-            ? { ...templateTag, default: null }
-            : templateTag,
+        ["dataset_query", "native", "template-tags"],
+        tags => {
+          const { name } = templateTag;
+          const newTag =
+            tags[name] && tags[name].type !== templateTag.type
+              ? // when we switch type, null out any default
+                { ...templateTag, default: null }
+              : templateTag;
+          return { ...tags, [name]: newTag };
+        },
       );
     };
   },

frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx

@@ -161,7 +161,10 @@ export default class NativeQueryEditor extends Component {
       return;
     }
 
-    if (this._editor.getValue() !== query.queryText()) {
+    // Check that the query prop changed before updating the editor. Otherwise,
+    // we might overwrite just typed characters before onChange is called.
+    const queryPropUpdated = this.props.query !== prevProps.query;
+    if (queryPropUpdated && this._editor.getValue() !== query.queryText()) {
       // This is a weird hack, but the purpose is to avoid an infinite loop caused by the fact that calling editor.setValue()
       // will trigger the editor 'change' event, update the query, and cause another rendering loop which we don't want, so
       // we need a way to update the editor without causing the onChange event to go through as well

frontend/test/metabase/query_builder/components/NativeQueryEditor.cy.spec.js

@@ -64,4 +64,45 @@ describe("NativeQueryEditor", () => {
       "You'll need to pick a value for 'X' before this query can run.",
     );
   });
+
+  it("doesn't reorder template tags when updated", () => {
+    cy.visit("/question/new");
+    cy.contains("Native query").click();
+
+    // Write a query with parameter x. It defaults to a text parameter.
+    cy.get(".ace_content").type("{{foo}} {{bar}}", {
+      parseSpecialCharSequences: false,
+    });
+
+    cy.contains("Variables")
+      .parent()
+      .parent()
+      .find(".text-brand")
+      .as("variableLabels");
+
+    // ensure they're in the right order to start
+    cy.get("@variableLabels")
+      .first()
+      .should("have.text", "foo");
+    cy.get("@variableLabels")
+      .last()
+      .should("have.text", "bar");
+
+    // change the parameter to a number.
+    cy.contains("Variable type")
+      .first()
+      .next()
+      .as("variableType");
+    cy.get("@variableType").click();
+    cy.contains("Number").click();
+    cy.get("@variableType").should("have.text", "Number");
+
+    // ensure they're still in the right order
+    cy.get("@variableLabels")
+      .first()
+      .should("have.text", "foo");
+    cy.get("@variableLabels")
+      .last()
+      .should("have.text", "bar");
+  });
 });