fixing updating editor state (#22869) (#24484)

* editor state update

* remove lockDirty

* Add functional tests

* Add data to functional tests

src/core_plugins/kibana/public/visualize/wizard/step_1.html

@@ -50,6 +50,7 @@
           tooltip="{{ getVisTypeTooltip(type) }}"
           tooltip-placement="{{ getVisTypeTooltipPosition($parent.$index) }}"
           aria-describedby="visDescription_{{ ::getVisTypeId(type) }}"
+          data-test-subj="visType-{{::type.name}}"
         >
           <div class="kuiGalleryItem__image">
             <img

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

@@ -32,7 +32,6 @@ 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) {
@@ -116,18 +115,10 @@ const defaultEditor = function ($rootScope, $compile) {
             }
           };
 
-          let lockDirty = false;
           $scope.$watch(() => {
             return $scope.vis.getSerializableState($scope.state);
           }, function (newState) {
-            // when visualization updates its `vis.params` we need to update the editor, but we should
-            // not set the dirty flag (as this change came from vis itself and is already applied)
-            if (lockDirty) {
-              lockDirty = false;
-            } else {
-              $scope.vis.dirty = !angular.equals(newState, $scope.oldState);
-            }
-
+            $scope.vis.dirty = !angular.equals(newState, $scope.oldState);
             $scope.responseValueAggs = null;
             try {
               $scope.responseValueAggs = $scope.state.aggs.getResponseAggs().filter(function (agg) {
@@ -144,8 +135,9 @@ const defaultEditor = function ($rootScope, $compile) {
           $scope.$watch(() => {
             return $scope.vis.getCurrentState(false);
           }, (newState) => {
-            if (updateEditorStateWithChanges(newState, $scope.oldState, $scope.state)) {
-              lockDirty = true;
+            if (!_.isEqual(newState, $scope.oldState)) {
+              $scope.state = $scope.vis.copyCurrentState(true);
+              $scope.oldState = newState;
             }
           }, true);
 

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

@@ -1,51 +0,0 @@
-/*
- * 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

@@ -1,78 +0,0 @@
-/*
- * 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/page_objects/visualize_page.js

@@ -50,6 +50,11 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       });
     }
 
+    async clickVisType(type) {
+      await testSubjects.click(`visType-${type}`);
+      await PageObjects.header.waitUntilLoadingHasFinished();
+    }
+
     async clickAreaChart() {
       await find.clickByPartialLinkText('Area');
       await PageObjects.header.waitUntilLoadingHasFinished();