fixing editor bug with apply being disabled in some cases (#21333) (#21385)

src/ui/public/vis/editors/default/default.js

@@ -32,6 +32,8 @@ import { DefaultEditorSize } from '../../editor_size';
 
 import { VisEditorTypesRegistryProvider } from '../../../registry/vis_editor_types';
 import { getVisualizeLoader } from '../../../visualize/loader/visualize_loader';
+import { updateEditorStateWithChanges } from './update_editor_state';
+
 
 const defaultEditor = function ($rootScope, $compile) {
   return class DefaultEditor {
@@ -142,36 +144,9 @@ const defaultEditor = function ($rootScope, $compile) {
           $scope.$watch(() => {
             return $scope.vis.getCurrentState(false);
           }, (newState) => {
-            const updateEditorStateWithChanges = (newState, oldState, editorState) => {
-              for (const prop in newState) {
-                if (newState.hasOwnProperty(prop)) {
-                  const newStateValue = newState[prop];
-                  const oldStateValue = oldState[prop];
-                  const editorStateValue = editorState[prop];
-
-                  if (typeof newStateValue === 'object') {
-                    if (editorStateValue) {
-                      // Keep traversing.
-                      return updateEditorStateWithChanges(newStateValue, oldStateValue, editorStateValue);
-                    }
-
-                    const newStateValueCopy = _.cloneDeep(newStateValue);
-                    editorState[prop] = newStateValueCopy;
-                    oldState[prop] = newStateValueCopy;
-                    lockDirty = true;
-                    return;
-                  }
-
-                  if (newStateValue !== oldStateValue) {
-                    oldState[prop] = newStateValue;
-                    editorState[prop] = newStateValue;
-                    lockDirty = true;
-                  }
-                }
-              }
-            };
-
-            updateEditorStateWithChanges(newState, $scope.oldState, $scope.state);
+            if (updateEditorStateWithChanges(newState, $scope.oldState, $scope.state)) {
+              lockDirty = true;
+            }
           }, true);
 
           // Load the default editor template, attach it to the DOM and compile it.

src/ui/public/vis/editors/default/sidebar.html

@@ -124,6 +124,7 @@
             ng-hide="autoApplyEnabled"
           >
             <button
+              data-test-subj="visualizeEditorResetButton"
               class="kuiButton kuiButton--basic navbar-btn-link"
               ng-disabled="!vis.dirty"
               ng-click="resetEditableVis()"

src/ui/public/vis/editors/default/update_editor_state.js

@@ -0,0 +1,51 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { cloneDeep } from 'lodash';
+
+export const updateEditorStateWithChanges = (newState, oldState, editorState) => {
+  let dirty = false;
+  for (const prop in newState) {
+    if (newState.hasOwnProperty(prop)) {
+      const newStateValue = newState[prop];
+      const oldStateValue = oldState[prop];
+      const editorStateValue = editorState[prop];
+
+      if (newStateValue && typeof newStateValue === 'object') {
+        if (editorStateValue) {
+          // Keep traversing.
+          if (updateEditorStateWithChanges(newStateValue, oldStateValue, editorStateValue)) {
+            dirty = true;
+          }
+          break;
+        }
+
+        const newStateValueCopy = cloneDeep(newStateValue);
+        editorState[prop] = newStateValueCopy;
+        oldState[prop] = newStateValueCopy;
+        dirty = true;
+      } else if (newStateValue !== oldStateValue) {
+        oldState[prop] = newStateValue;
+        editorState[prop] = newStateValue;
+        dirty = true;
+      }
+    }
+  }
+  return dirty;
+};

src/ui/public/vis/editors/default/update_editor_state.test.js

@@ -0,0 +1,78 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { cloneDeep } from 'lodash';
+import { updateEditorStateWithChanges } from './update_editor_state';
+
+// Parts of the tests in this file are generated more dynamically, based on the
+// values inside the Status object.Make sure this object has one function per entry
+// in Status, that actually change on the passed $scope, what needs to be changed
+// so that we expect the getUpdateStatus function to actually detect a change.
+const oldState = {
+  booleanValue: false,
+  intValue: 0,
+  stringValue: '',
+  nullValue: null,
+  undefinedValue: undefined,
+  objectValue: {},
+  arrayValue: [],
+  testObject: {
+    booleanValue: false,
+    intValue: 0,
+    nullValue: null,
+    arrayValue: [1, 2, 3, { nullValue: null, stringValue: 'hello' }],
+  }
+};
+
+describe('updateEditorStateWithChanges', () => {
+  let editorState;
+  let newState;
+
+  beforeEach(() => {
+    editorState = cloneDeep(oldState);
+    newState = cloneDeep(oldState);
+  });
+
+  it('should be a function', () => {
+    expect(typeof updateEditorStateWithChanges).toBe('function');
+  });
+
+  it('should return false if no changes are detected', () => {
+    const dirty = updateEditorStateWithChanges(newState, oldState, editorState);
+    expect(dirty).toBe(false);
+  });
+
+  it('should return true if changes are detected', () => {
+    let dirty;
+
+    newState.booleanValue = true;
+    dirty = updateEditorStateWithChanges(newState, oldState, editorState);
+    expect(dirty).toBe(true);
+
+    dirty = updateEditorStateWithChanges(newState, oldState, editorState);
+    expect(dirty).toBe(false);
+
+    newState.nullValue = 'test';
+    dirty = updateEditorStateWithChanges(newState, oldState, editorState);
+    expect(dirty).toBe(true);
+
+    dirty = updateEditorStateWithChanges(newState, oldState, editorState);
+    expect(dirty).toBe(false);
+  });
+});

test/functional/apps/visualize/_data_table.js

@@ -51,6 +51,20 @@ export default function ({ getService, getPageObjects }) {
       await PageObjects.visualize.clickGo();
     });
 
+    it('should allow applying changed params', async () => {
+      await PageObjects.visualize.setNumericInterval('1', { append: true });
+      const interval = await PageObjects.visualize.getInputTypeParam('interval');
+      expect(interval).to.be('20001');
+      const isApplyButtonEnabled = await PageObjects.visualize.isApplyEnabled();
+      expect(isApplyButtonEnabled).to.be(true);
+    });
+
+    it('should allow resseting changed params', async () => {
+      await PageObjects.visualize.clickReset();
+      const interval = await PageObjects.visualize.getInputTypeParam('interval');
+      expect(interval).to.be('2000');
+    });
+
     it('should be able to save and load', async function () {
       await PageObjects.visualize.saveVisualization(vizName1);
       const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();

test/functional/page_objects/visualize_page.js

@@ -594,6 +594,11 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       await find.clickByCssSelector(`select[name="orderBy"] > option[value="${fieldValue}"]`);
     }
 
+    async getInputTypeParam(paramName) {
+      const input = await find.byCssSelector(`input[ng-model="agg.params.${paramName}"]`);
+      return await input.getProperty('value');
+    }
+
     async getInterval() {
       const select = await find.byCssSelector('select[ng-model="agg.params.interval"]');
       const selectedIndex = await select.getProperty('selectedIndex');
@@ -607,9 +612,11 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       await input.type(newValue);
     }
 
-    async setNumericInterval(newValue) {
+    async setNumericInterval(newValue, { append } = {}) {
       const input = await find.byCssSelector('input[name="interval"]');
-      await input.clearValue();
+      if (!append) {
+        await input.clearValue();
+      }
       await input.type(newValue + '');
       await PageObjects.common.sleep(1000);
     }
@@ -633,11 +640,20 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       return await find.clickByCssSelector('input[name="showMissing"]');
     }
 
+    async isApplyEnabled() {
+      const applyButton = await testSubjects.find('visualizeEditorRenderButton');
+      return await applyButton.isEnabled();
+    }
+
     async clickGo() {
       await testSubjects.click('visualizeEditorRenderButton');
       await PageObjects.header.waitUntilLoadingHasFinished();
     }
 
+    async clickReset() {
+      await testSubjects.click('visualizeEditorResetButton');
+    }
+
     async toggleAutoMode() {
       await testSubjects.click('visualizeEditorAutoButton');
     }