Revert "feat(explore): Show confirmation modal if user exits Explore without saving changes (apache#19993)" (apache#20092)

This reverts commit ca9766c.

Author: kgabryje

superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx

@@ -308,4 +308,34 @@ describe('AlteredSliceTag', () => {
       ).toBe(expected);
     });
   });
+  describe('isEqualish', () => {
+    it('considers null, undefined, {} and [] as equal', () => {
+      const inst = wrapper.instance();
+      expect(inst.isEqualish(null, undefined)).toBe(true);
+      expect(inst.isEqualish(null, [])).toBe(true);
+      expect(inst.isEqualish(null, {})).toBe(true);
+      expect(inst.isEqualish(undefined, {})).toBe(true);
+    });
+    it('considers empty strings are the same as null', () => {
+      const inst = wrapper.instance();
+      expect(inst.isEqualish(undefined, '')).toBe(true);
+      expect(inst.isEqualish(null, '')).toBe(true);
+    });
+    it('considers deeply equal objects as equal', () => {
+      const inst = wrapper.instance();
+      expect(inst.isEqualish('', '')).toBe(true);
+      expect(inst.isEqualish({ a: 1, b: 2, c: 3 }, { a: 1, b: 2, c: 3 })).toBe(
+        true,
+      );
+      // Out of order
+      expect(inst.isEqualish({ a: 1, b: 2, c: 3 }, { b: 2, a: 1, c: 3 })).toBe(
+        true,
+      );
+
+      // Actually  not equal
+      expect(inst.isEqualish({ a: 1, b: 2, z: 9 }, { a: 1, b: 2, c: 3 })).toBe(
+        false,
+      );
+    });
+  });
 });

superset-frontend/src/components/AlteredSliceTag/index.jsx

@@ -20,7 +20,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { isEqual, isEmpty } from 'lodash';
 import { styled, t } from '@superset-ui/core';
-import { getFormDataDiffs } from 'src/explore/exploreUtils/formData';
+import { sanitizeFormData } from 'src/explore/exploreUtils/formData';
 import getControlsForVizType from 'src/utils/getControlsForVizType';
 import { safeStringify } from 'src/utils/safeStringify';
 import { Tooltip } from 'src/components/Tooltip';
@@ -44,6 +44,24 @@ const StyledLabel = styled.span`
   `}
 `;
 
+function alterForComparison(value) {
+  // Considering `[]`, `{}`, `null` and `undefined` as identical
+  // for this purpose
+  if (value === undefined || value === null || value === '') {
+    return null;
+  }
+  if (typeof value === 'object') {
+    if (Array.isArray(value) && value.length === 0) {
+      return null;
+    }
+    const keys = Object.keys(value);
+    if (keys && keys.length === 0) {
+      return null;
+    }
+  }
+  return value;
+}
+
 export default class AlteredSliceTag extends React.Component {
   constructor(props) {
     super(props);
@@ -77,7 +95,27 @@ export default class AlteredSliceTag extends React.Component {
   getDiffs(props) {
     // Returns all properties that differ in the
     // current form data and the saved form data
-    return getFormDataDiffs(props.origFormData, props.currentFormData);
+    const ofd = sanitizeFormData(props.origFormData);
+    const cfd = sanitizeFormData(props.currentFormData);
+
+    const fdKeys = Object.keys(cfd);
+    const diffs = {};
+    fdKeys.forEach(fdKey => {
+      if (!ofd[fdKey] && !cfd[fdKey]) {
+        return;
+      }
+      if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
+        return;
+      }
+      if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) {
+        diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
+      }
+    });
+    return diffs;
+  }
+
+  isEqualish(val1, val2) {
+    return isEqual(alterForComparison(val1), alterForComparison(val2));
   }
 
   formatValue(value, key, controlsMap) {

superset-frontend/src/explore/components/ExploreViewContainer/index.jsx

@@ -17,18 +17,12 @@
  * under the License.
  */
 /* eslint camelcase: 0 */
-import React, {
-  useCallback,
-  useEffect,
-  useMemo,
-  useRef,
-  useState,
-} from 'react';
+import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import PropTypes from 'prop-types';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
 import { styled, t, css, useTheme, logging } from '@superset-ui/core';
-import { debounce, pick, isEmpty } from 'lodash';
+import { debounce, pick } from 'lodash';
 import { Resizable } from 're-resizable';
 import { useChangeEffect } from 'src/hooks/useChangeEffect';
 import { usePluginContext } from 'src/components/DynamicPlugins';
@@ -49,11 +43,7 @@ import * as chartActions from 'src/components/Chart/chartAction';
 import { fetchDatasourceMetadata } from 'src/dashboard/actions/datasources';
 import { chartPropShape } from 'src/dashboard/util/propShapes';
 import { mergeExtraFormData } from 'src/dashboard/components/nativeFilters/utils';
-import {
-  getFormDataDiffs,
-  postFormData,
-  putFormData,
-} from 'src/explore/exploreUtils/formData';
+import { postFormData, putFormData } from 'src/explore/exploreUtils/formData';
 import { useTabId } from 'src/hooks/useTabId';
 import ExploreChartPanel from '../ExploreChartPanel';
 import ConnectedControlPanelsContainer from '../ControlPanelsContainer';
@@ -226,11 +216,6 @@ const updateHistory = debounce(
   1000,
 );
 
-const handleUnloadEvent = e => {
-  e.preventDefault();
-  e.returnValue = 'Controls changed';
-};
-
 function ExploreViewContainer(props) {
   const dynamicPluginContext = usePluginContext();
   const dynamicPlugin = dynamicPluginContext.dynamicPlugins[props.vizType];
@@ -251,9 +236,6 @@ function ExploreViewContainer(props) {
 
   const theme = useTheme();
 
-  const isBeforeUnloadActive = useRef(false);
-  const initialFormData = useRef(props.form_data);
-
   const defaultSidebarsWidth = {
     controls_width: 320,
     datasource_width: 300,
@@ -383,27 +365,6 @@ function ExploreViewContainer(props) {
     };
   }, [handleKeydown, previousHandleKeyDown]);
 
-  useEffect(() => {
-    const formDataChanged = !isEmpty(
-      getFormDataDiffs(initialFormData.current, props.form_data),
-    );
-    if (formDataChanged && !isBeforeUnloadActive.current) {
-      window.addEventListener('beforeunload', handleUnloadEvent);
-      isBeforeUnloadActive.current = true;
-    }
-    if (!formDataChanged && isBeforeUnloadActive.current) {
-      window.removeEventListener('beforeunload', handleUnloadEvent);
-      isBeforeUnloadActive.current = false;
-    }
-  }, [props.form_data]);
-
-  // cleanup beforeunload event listener
-  // we use separate useEffect to call it only on component unmount instead of on every form data change
-  useEffect(
-    () => () => window.removeEventListener('beforeunload', handleUnloadEvent),
-    [],
-  );
-
   useEffect(() => {
     if (wasDynamicPluginLoading && !isDynamicPluginLoading) {
       // reload the controls now that we actually have the control config

superset-frontend/src/explore/exploreUtils/formData.test.ts

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { sanitizeFormData, isEqualish } from './formData';
+import { sanitizeFormData } from './formData';
 
 test('sanitizeFormData removes temporary control values', () => {
   expect(
@@ -26,25 +26,3 @@ test('sanitizeFormData removes temporary control values', () => {
     }),
   ).toEqual({ metrics: ['foo', 'bar'] });
 });
-
-test('isEqualish', () => {
-  // considers null, undefined, {} and [] as equal
-  expect(isEqualish(null, undefined)).toBe(true);
-  expect(isEqualish(null, [])).toBe(true);
-  expect(isEqualish(null, {})).toBe(true);
-  expect(isEqualish(undefined, {})).toBe(true);
-
-  // considers empty strings are the same as null
-  expect(isEqualish(undefined, '')).toBe(true);
-  expect(isEqualish(null, '')).toBe(true);
-
-  // considers deeply equal objects as equal
-  expect(isEqualish('', '')).toBe(true);
-  expect(isEqualish({ a: 1, b: 2, c: 3 }, { a: 1, b: 2, c: 3 })).toBe(true);
-
-  // Out of order
-  expect(isEqualish({ a: 1, b: 2, c: 3 }, { b: 2, a: 1, c: 3 })).toBe(true);
-
-  // Actually  not equal
-  expect(isEqualish({ a: 1, b: 2, z: 9 }, { a: 1, b: 2, c: 3 })).toBe(false);
-});

superset-frontend/src/explore/exploreUtils/formData.ts

@@ -16,8 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { isEqual, omit } from 'lodash';
-import { SupersetClient, JsonObject, JsonValue } from '@superset-ui/core';
+import { omit } from 'lodash';
+import { SupersetClient, JsonObject } from '@superset-ui/core';
 
 type Payload = {
   dataset_id: number;
@@ -30,56 +30,6 @@ const TEMPORARY_CONTROLS = ['url_params'];
 export const sanitizeFormData = (formData: JsonObject): JsonObject =>
   omit(formData, TEMPORARY_CONTROLS);
 
-export const alterForComparison = (value: JsonValue | undefined) => {
-  // Considering `[]`, `{}`, `null` and `undefined` as identical
-  // for this purpose
-  if (
-    value === undefined ||
-    value === null ||
-    value === '' ||
-    (Array.isArray(value) && value.length === 0) ||
-    (typeof value === 'object' && Object.keys(value).length === 0)
-  ) {
-    return null;
-  }
-  if (Array.isArray(value)) {
-    // omit prototype for comparison of class instances with json objects
-    return value.map(v => (typeof v === 'object' ? omit(v, ['__proto__']) : v));
-  }
-  if (typeof value === 'object') {
-    return omit(value, ['__proto__']);
-  }
-  return value;
-};
-
-export const isEqualish = (
-  val1: JsonValue | undefined,
-  val2: JsonValue | undefined,
-) => isEqual(alterForComparison(val1), alterForComparison(val2));
-
-export const getFormDataDiffs = (
-  formData1: JsonObject,
-  formData2: JsonObject,
-) => {
-  const ofd = sanitizeFormData(formData1);
-  const cfd = sanitizeFormData(formData2);
-
-  const fdKeys = Object.keys(cfd);
-  const diffs = {};
-  fdKeys.forEach(fdKey => {
-    if (!ofd[fdKey] && !cfd[fdKey]) {
-      return;
-    }
-    if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
-      return;
-    }
-    if (!isEqualish(ofd[fdKey], cfd[fdKey])) {
-      diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
-    }
-  });
-  return diffs;
-};
-
 const assembleEndpoint = (key?: string, tabId?: string) => {
   let endpoint = 'api/v1/explore/form_data';
   if (key) {