[Maps] do not allow save when map has unsaved layer changes (#39529) (#39550)

* [Maps] do not allow save when map has unsaved layer changes

* fix jest test

* refactor add layer panel functional tests

test/functional/page_objects/visualize_page.js

@@ -743,11 +743,6 @@ export function VisualizePageProvider({ getService, getPageObjects, updateBaseli
       ));
     }
 
-    async expectNoSaveOption() {
-      const saveButtonExists = await testSubjects.exists('visualizeSaveButton');
-      expect(saveButtonExists).to.be(false);
-    }
-
     async clickLoadSavedVisButton() {
       // TODO: Use a test subject selector once we rewrite breadcrumbs to accept each breadcrumb
       // element as a child instead of building the breadcrumbs dynamically.

x-pack/legacy/plugins/maps/public/angular/map_controller.js

@@ -36,7 +36,7 @@ import {
   setIsLayerTOCOpen,
   setOpenTOCDetails,
 } from '../store/ui';
-import { getQueryableUniqueIndexPatternIds } from '../selectors/map_selectors';
+import { getQueryableUniqueIndexPatternIds, hasDirtyState } from '../selectors/map_selectors';
 import { getInspectorAdapters } from '../store/non_serializable_instances';
 import { Inspector } from 'ui/inspector';
 import { docTitle } from 'ui/doc_title';
@@ -191,6 +191,7 @@ app.controller('GisMapController', ($scope, $route, config, kbnUrl, localStorage
   }
 
   $scope.isFullScreen = false;
+  $scope.isSaveDisabled = false;
   function handleStoreChanges(store) {
     const nextIsFullScreen = getIsFullScreen(store.getState());
     if (nextIsFullScreen !== $scope.isFullScreen) {
@@ -205,6 +206,13 @@ app.controller('GisMapController', ($scope, $route, config, kbnUrl, localStorage
       prevIndexPatternIds = nextIndexPatternIds;
       updateIndexPatterns(nextIndexPatternIds);
     }
+
+    const nextIsSaveDisabled = hasDirtyState(store.getState());
+    if (nextIsSaveDisabled !== $scope.isSaveDisabled) {
+      $scope.$evalAsync(() => {
+        $scope.isSaveDisabled = nextIsSaveDisabled;
+      });
+    }
   }
 
   $scope.$on('$destroy', () => {
@@ -307,6 +315,16 @@ app.controller('GisMapController', ($scope, $route, config, kbnUrl, localStorage
       defaultMessage: `Save map`
     }),
     testId: 'mapSaveButton',
+    disableButton() {
+      return $scope.isSaveDisabled;
+    },
+    tooltip() {
+      if ($scope.isSaveDisabled) {
+        return i18n.translate('xpack.maps.mapController.saveMapDisabledButtonTooltip', {
+          defaultMessage: 'Save or Cancel your layer changes before saving'
+        });
+      }
+    },
     run: async () => {
       const onSave = ({ newTitle, newCopyOnSave, isTitleDuplicateConfirmed, onTitleDuplicate }) => {
         const currentTitle = savedMap.title;

x-pack/legacy/plugins/maps/public/components/layer_panel/__snapshots__/view.test.js.snap

@@ -102,9 +102,7 @@ exports[`LayerPanel is rendered 1`] = `
   <EuiFlyoutFooter
     className="mapLayerPanel__footer"
   >
-    <FlyoutFooter
-      hasStateChanged={[Function]}
-    />
+    <FlyoutFooter />
   </EuiFlyoutFooter>
 </EuiFlexGroup>
 `;

x-pack/legacy/plugins/maps/public/components/layer_panel/flyout_footer/index.js

@@ -7,12 +7,19 @@
 import { connect } from 'react-redux';
 import { FlyoutFooter } from './view';
 import { updateFlyout, FLYOUT_STATE } from '../../../store/ui';
+import { hasDirtyState } from '../../../selectors/map_selectors';
 import {
   setSelectedLayer,
   removeSelectedLayer,
   removeTrackedLayerStateForSelectedLayer
 } from '../../../actions/store_actions';
 
+function mapStateToProps(state = {}) {
+  return {
+    hasStateChanged: hasDirtyState(state)
+  };
+}
+
 const mapDispatchToProps = (dispatch) => {
   return {
     cancelLayerPanel: () => {
@@ -31,5 +38,5 @@ const mapDispatchToProps = (dispatch) => {
   };
 };
 
-const connectedFlyoutFooter = connect(null, mapDispatchToProps)(FlyoutFooter);
+const connectedFlyoutFooter = connect(mapStateToProps, mapDispatchToProps)(FlyoutFooter);
 export { connectedFlyoutFooter as FlyoutFooter };

x-pack/legacy/plugins/maps/public/components/layer_panel/index.js

@@ -6,16 +6,14 @@
 
 import { connect } from 'react-redux';
 import { LayerPanel } from './view';
-import { getSelectedLayer, hasDirtyState } from '../../selectors/map_selectors';
+import { getSelectedLayer } from '../../selectors/map_selectors';
 import {
   fitToLayerExtent
 } from '../../actions/store_actions';
 
 function mapStateToProps(state = {}) {
-  const selectedLayer = getSelectedLayer(state);
   return {
-    selectedLayer,
-    hasStateChanged: hasDirtyState(state)
+    selectedLayer: getSelectedLayer(state),
   };
 }
 

x-pack/legacy/plugins/maps/public/components/layer_panel/view.js

@@ -210,7 +210,7 @@ export class LayerPanel extends React.Component {
         </div>
 
         <EuiFlyoutFooter className="mapLayerPanel__footer">
-          <FlyoutFooter hasStateChanged={this.props.hasStateChanged}/>
+          <FlyoutFooter />
         </EuiFlyoutFooter>
       </EuiFlexGroup>
     );

x-pack/legacy/plugins/maps/public/components/layer_panel/view.test.js

@@ -66,7 +66,6 @@ const mockLayer = {
 
 const defaultProps = {
   selectedLayer: mockLayer,
-  hasStateChanged: () => {},
   fitToBounds: () => {},
 };
 

x-pack/legacy/plugins/maps/public/index.js

@@ -21,6 +21,7 @@ import { capabilities } from 'ui/capabilities';
 import chrome from 'ui/chrome';
 import routes from 'ui/routes';
 import 'ui/kbn_top_nav';
+import 'ui/angular-bootstrap'; // required for kbn-top-nav button tooltips
 import { uiModules } from 'ui/modules';
 import { docTitle } from 'ui/doc_title';
 import 'ui/autoload/styles';

x-pack/legacy/plugins/maps/public/selectors/map_selectors.js

@@ -203,10 +203,21 @@ export const getQueryableUniqueIndexPatternIds = createSelector(
   }
 );
 
-export const hasDirtyState = createSelector(getLayerListRaw, (layerListRaw) => {
-  return layerListRaw.some(layerDescriptor => {
-    const currentState = copyPersistentState(layerDescriptor);
-    const trackedState = layerDescriptor[TRACKED_LAYER_DESCRIPTOR];
-    return (trackedState) ? !_.isEqual(currentState, trackedState) : false;
-  });
-});
+export const hasDirtyState = createSelector(
+  getLayerListRaw,
+  getTransientLayerId,
+  (layerListRaw, transientLayerId) => {
+    if (transientLayerId) {
+      return true;
+    }
+
+    return layerListRaw.some(layerDescriptor => {
+      const trackedState = layerDescriptor[TRACKED_LAYER_DESCRIPTOR];
+      if (!trackedState) {
+        return false;
+      }
+      const currentState = copyPersistentState(layerDescriptor);
+      return !_.isEqual(currentState, trackedState);
+    });
+  }
+);

x-pack/test/functional/apps/maps/add_layer_panel.js

@@ -6,84 +6,61 @@
 
 import expect from '@kbn/expect';
 
-export default function ({ getPageObjects }) {
-  const PageObjects = getPageObjects(['maps', 'common']);
+export default function ({ getService, getPageObjects }) {
+  const testSubjects = getService('testSubjects');
+  const PageObjects = getPageObjects(['maps']);
 
   describe('Add layer panel', () => {
     before(async () => {
       await PageObjects.maps.openNewMap();
     });
 
-    beforeEach(async () => {
-      await PageObjects.maps.clickAddLayer();
-    });
-
-    afterEach(async () => {
-      await PageObjects.maps.cancelLayerAdd();
-    });
-
-    it('should open on clicking "Add layer"', async () => {
-      // Verify panel page element is open
-      const panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
-      expect(panelOpen).to.be(true);
-    });
-
-    it('should close on clicking "Cancel"', async () => {
-      // Verify panel page element is open
-      let panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
-      expect(panelOpen).to.be(true);
-      // Click cancel
-      await PageObjects.maps.cancelLayerAdd();
-      // Verify panel isn't open
-      panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
-      expect(panelOpen).to.be(false);
-    });
-
-    it('should close & remove layer on clicking "Cancel" after selecting layer',
-      async () => {
-        // Verify panel page element is open
-        let panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
+    describe('visibility', () => {
+      it('should open on clicking "Add layer"', async () => {
+        await PageObjects.maps.clickAddLayer();
+        const panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
         expect(panelOpen).to.be(true);
-        // Select source
-        await PageObjects.maps.selectVectorSource();
-        // Select layer
-        const vectorLayer = await PageObjects.maps.selectVectorLayer();
-        // Confirm layer added
-        await PageObjects.maps.waitForLayersToLoad();
-        let vectorLayerExists = await PageObjects.maps.doesLayerExist(vectorLayer);
-        expect(vectorLayerExists).to.be(true);
-        // Click cancel
-        await PageObjects.maps.cancelLayerAdd();
-        // Verify panel isn't open
-        panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
-        expect(panelOpen).to.be(false);
-        // Verify layer has been removed
-        await PageObjects.maps.waitForLayerDeleted(vectorLayer);
-        vectorLayerExists = await PageObjects.maps.doesLayerExist(vectorLayer);
-        expect(vectorLayerExists).to.be(false);
       });
 
-    it('should close and remove layer on map save', async () => {
-      // Verify panel page element is open
-      let panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
-      expect(panelOpen).to.be(true);
-      // Select source
-      await PageObjects.maps.selectVectorSource();
-      // Select layer
-      const vectorLayer = await PageObjects.maps.selectVectorLayer();
-      // Confirm layer added
-      await PageObjects.maps.waitForLayersToLoad();
-      let vectorLayerExists = await PageObjects.maps.doesLayerExist(vectorLayer);
-      expect(vectorLayerExists).to.be(true);
-      // Click save
-      await PageObjects.maps.saveMap('Mappishness');
-      // Verify panel isn't open
-      panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
-      expect(panelOpen).to.be(false);
-      // Verify layer has been removed
-      await PageObjects.maps.waitForLayerDeleted(vectorLayer);
-      vectorLayerExists = await PageObjects.maps.doesLayerExist(vectorLayer);
-      expect(vectorLayerExists).to.be(false);
+      it('should close on clicking "Cancel"', async () => {
+        await PageObjects.maps.cancelLayerAdd();
+        const panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
+        expect(panelOpen).to.be(false);
+      });
+    });
+
+    describe('with unsaved layer', () => {
+      const LAYER_NAME = 'World Countries';
+
+      before(async () => {
+        await PageObjects.maps.clickAddLayer();
+        await PageObjects.maps.selectVectorSource();
+        await PageObjects.maps.selectVectorLayer(LAYER_NAME);
+      });
+
+      it('should show unsaved layer in layer TOC', async () => {
+        const vectorLayerExists = await PageObjects.maps.doesLayerExist(LAYER_NAME);
+        expect(vectorLayerExists).to.be(true);
+      });
+
+      it('should disable Map application save button', async () => {
+        // saving map should be a no-op because its diabled
+        await testSubjects.click('mapSaveButton');
+
+        const panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
+        expect(panelOpen).to.be(true);
+        const vectorLayerExists = await PageObjects.maps.doesLayerExist(LAYER_NAME);
+        expect(vectorLayerExists).to.be(true);
+      });
+
+      it('should close & remove layer on clicking "Cancel"', async () => {
+        await PageObjects.maps.cancelLayerAdd(LAYER_NAME);
+
+        const panelOpen = await PageObjects.maps.isLayerAddPanelOpen();
+        expect(panelOpen).to.be(false);
+        const vectorLayerExists = await PageObjects.maps.doesLayerExist(LAYER_NAME);
+        expect(vectorLayerExists).to.be(false);
+      });
     });
   });
 }

x-pack/test/functional/page_objects/gis_page.js

@@ -329,11 +329,14 @@ export function GisPageProvider({ getService, getPageObjects }) {
       return await testSubjects.exists('layerAddForm');
     }
 
-    async cancelLayerAdd() {
+    async cancelLayerAdd(layerName) {
       log.debug(`Cancel layer add`);
       const cancelExists = await testSubjects.exists('layerAddCancelButton');
       if (cancelExists) {
         await testSubjects.click('layerAddCancelButton');
+        if (layerName) {
+          await this.waitForLayerDeleted(layerName);
+        }
       }
     }
 
@@ -356,14 +359,13 @@ export function GisPageProvider({ getService, getPageObjects }) {
     }
 
     // Returns first layer by default
-    async selectVectorLayer(vectorLayerName = '') {
-      log.debug(`Select vector layer ${vectorLayerName}`);
-      const optionsStringList = await comboBox.getOptionsList('emsVectorComboBox');
-      const selectedVectorLayer = vectorLayerName
-        ? vectorLayerName
-        : optionsStringList.trim().split('\n')[0];
-      await comboBox.set('emsVectorComboBox', selectedVectorLayer);
-      return selectedVectorLayer;
+    async selectVectorLayer(vectorLayerName) {
+      log.debug(`Select EMS vector layer ${vectorLayerName}`);
+      if (!vectorLayerName) {
+        throw new Error(`You did not provide the EMS layer to select`);
+      }
+      await comboBox.set('emsVectorComboBox', vectorLayerName);
+      await this.waitForLayersToLoad();
     }
 
     async removeLayer(layerName) {