diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index 40db66f80b2cd..aa4d6c635565d 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -51,6 +51,7 @@ These features are **finished** but currently being tested. They are usable, but - ALERT_REPORTS: [(docs)](https://superset.apache.org/docs/installation/alerts-reports) - ALLOW_FULL_CSV_EXPORT - CACHE_IMPERSONATION +- CONFIRM_DASHBOARD_DIFF - DASHBOARD_EDIT_CHART_IN_NEW_TAB - DASHBOARD_FILTERS_EXPERIMENTAL - DASHBOARD_NATIVE_FILTERS diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index c196947a144a6..3ae4d821b7b9e 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -100,12 +100,14 @@ "react-checkbox-tree": "^1.5.1", "react-color": "^2.13.8", "react-datetime": "^3.0.4", + "react-diff-viewer": "^3.1.1", "react-dnd": "^11.1.3", "react-dnd-html5-backend": "^11.1.3", "react-dom": "^16.13.0", "react-draggable": "^4.4.3", "react-gravatar": "^2.6.1", "react-hot-loader": "^4.12.20", + "react-intersection-observer": "^8.26.2", "react-js-cron": "^1.2.0", "react-json-tree": "^0.11.2", "react-jsonschema-form": "^1.2.0", @@ -24764,6 +24766,28 @@ "resolved": "https://registry.npmjs.org/bn.js/-/bn.js-4.12.0.tgz", "integrity": "sha512-c98Bf3tPniI+scsdk237ku1Dc3ujXQTSgyiPUDEOe7tRkhrqridvh8klBv0HCEso1OLOYcHuCv/cS6DNxKH+ZA==" }, + "node_modules/create-emotion": { + "version": "10.0.27", + "resolved": "https://registry.npmjs.org/create-emotion/-/create-emotion-10.0.27.tgz", + "integrity": "sha512-fIK73w82HPPn/RsAij7+Zt8eCE8SptcJ3WoRMfxMtjteYxud8GDTKKld7MYwAX2TVhrw29uR1N/bVGxeStHILg==", + "dependencies": { + "@emotion/cache": "^10.0.27", + "@emotion/serialize": "^0.11.15", + "@emotion/sheet": "0.9.4", + "@emotion/utils": "0.11.3" + } + }, + "node_modules/create-emotion/node_modules/@emotion/cache": { + "version": "10.0.29", + "resolved": "https://registry.npmjs.org/@emotion/cache/-/cache-10.0.29.tgz", + "integrity": "sha512-fU2VtSVlHiF27empSbxi1O2JFdNWZO+2NFHfwO0pxgTep6Xa3uGb+3pVKfLww2l/IBGLNEZl5Xf/++A4wAYDYQ==", + "dependencies": { + "@emotion/sheet": "0.9.4", + "@emotion/stylis": "0.8.5", + "@emotion/utils": "0.11.3", + "@emotion/weak-memoize": "0.2.5" + } + }, "node_modules/create-hash": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/create-hash/-/create-hash-1.2.0.tgz", @@ -27069,7 +27093,6 @@ "version": "4.0.2", "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", - "dev": true, "engines": { "node": ">=0.3.1" } @@ -27530,6 +27553,15 @@ "node": ">= 4" } }, + "node_modules/emotion": { + "version": "10.0.27", + "resolved": "https://registry.npmjs.org/emotion/-/emotion-10.0.27.tgz", + "integrity": "sha512-2xdDzdWWzue8R8lu4G76uWX5WhyQuzATon9LmNeCy/2BHVC6dsEpfhN1a0qhELgtDVdjyEA6J8Y/VlI5ZnaH0g==", + "dependencies": { + "babel-plugin-emotion": "^10.0.27", + "create-emotion": "^10.0.27" + } + }, "node_modules/emotion-rgba": { "version": "0.0.9", "resolved": "https://registry.npmjs.org/emotion-rgba/-/emotion-rgba-0.0.9.tgz", @@ -44792,6 +44824,26 @@ "react": "^16.5.0" } }, + "node_modules/react-diff-viewer": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/react-diff-viewer/-/react-diff-viewer-3.1.1.tgz", + "integrity": "sha512-rmvwNdcClp6ZWdS11m1m01UnBA4OwYaLG/li0dB781e/bQEzsGyj+qewVd6W5ztBwseQ72pO7nwaCcq5jnlzcw==", + "dependencies": { + "classnames": "^2.2.6", + "create-emotion": "^10.0.14", + "diff": "^4.0.1", + "emotion": "^10.0.14", + "memoize-one": "^5.0.4", + "prop-types": "^15.6.2" + }, + "engines": { + "node": ">= 8" + }, + "peerDependencies": { + "react": "^15.3.0 || ^16.0.0", + "react-dom": "^15.3.0 || ^16.0.0" + } + }, "node_modules/react-dnd": { "version": "11.1.3", "resolved": "https://registry.npmjs.org/react-dnd/-/react-dnd-11.1.3.tgz", @@ -45027,6 +45079,17 @@ "react": "^16.8.4 || ^17.0.0" } }, + "node_modules/react-intersection-observer": { + "version": "8.26.2", + "resolved": "https://registry.npmjs.org/react-intersection-observer/-/react-intersection-observer-8.26.2.tgz", + "integrity": "sha512-GmSjLNK+oV7kS+BHfrJSaA4wF61ELA33gizKHmN+tk59UT6/aW8kkqvlrFGPwxGoaIzLKS2evfG5fgkw5MIIsg==", + "dependencies": { + "tiny-invariant": "^1.1.0" + }, + "peerDependencies": { + "react": "^15.0.0 || ^16.0.0 || ^17.0.0" + } + }, "node_modules/react-is": { "version": "16.6.3", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.6.3.tgz", @@ -76266,6 +76329,30 @@ } } }, + "create-emotion": { + "version": "10.0.27", + "resolved": "https://registry.npmjs.org/create-emotion/-/create-emotion-10.0.27.tgz", + "integrity": "sha512-fIK73w82HPPn/RsAij7+Zt8eCE8SptcJ3WoRMfxMtjteYxud8GDTKKld7MYwAX2TVhrw29uR1N/bVGxeStHILg==", + "requires": { + "@emotion/cache": "^10.0.27", + "@emotion/serialize": "^0.11.15", + "@emotion/sheet": "0.9.4", + "@emotion/utils": "0.11.3" + }, + "dependencies": { + "@emotion/cache": { + "version": "10.0.29", + "resolved": "https://registry.npmjs.org/@emotion/cache/-/cache-10.0.29.tgz", + "integrity": "sha512-fU2VtSVlHiF27empSbxi1O2JFdNWZO+2NFHfwO0pxgTep6Xa3uGb+3pVKfLww2l/IBGLNEZl5Xf/++A4wAYDYQ==", + "requires": { + "@emotion/sheet": "0.9.4", + "@emotion/stylis": "0.8.5", + "@emotion/utils": "0.11.3", + "@emotion/weak-memoize": "0.2.5" + } + } + } + }, "create-hash": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/create-hash/-/create-hash-1.2.0.tgz", @@ -78020,8 +78107,7 @@ "diff": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", - "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", - "dev": true + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==" }, "diff-match-patch": { "version": "1.0.5", @@ -78439,6 +78525,15 @@ "resolved": "https://registry.npmjs.org/emojis-list/-/emojis-list-3.0.0.tgz", "integrity": "sha512-/kyM18EfinwXZbno9FyUGeFh87KC8HRQBQGildHZbEuRyWFOmv1U10o9BBp8XVZDVNNuQKyIGIu5ZYAAXJ0V2Q==" }, + "emotion": { + "version": "10.0.27", + "resolved": "https://registry.npmjs.org/emotion/-/emotion-10.0.27.tgz", + "integrity": "sha512-2xdDzdWWzue8R8lu4G76uWX5WhyQuzATon9LmNeCy/2BHVC6dsEpfhN1a0qhELgtDVdjyEA6J8Y/VlI5ZnaH0g==", + "requires": { + "babel-plugin-emotion": "^10.0.27", + "create-emotion": "^10.0.27" + } + }, "emotion-rgba": { "version": "0.0.9", "resolved": "https://registry.npmjs.org/emotion-rgba/-/emotion-rgba-0.0.9.tgz", @@ -91784,6 +91879,19 @@ "prop-types": "^15.5.7" } }, + "react-diff-viewer": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/react-diff-viewer/-/react-diff-viewer-3.1.1.tgz", + "integrity": "sha512-rmvwNdcClp6ZWdS11m1m01UnBA4OwYaLG/li0dB781e/bQEzsGyj+qewVd6W5ztBwseQ72pO7nwaCcq5jnlzcw==", + "requires": { + "classnames": "^2.2.6", + "create-emotion": "^10.0.14", + "diff": "^4.0.1", + "emotion": "^10.0.14", + "memoize-one": "^5.0.4", + "prop-types": "^15.6.2" + } + }, "react-dnd": { "version": "11.1.3", "resolved": "https://registry.npmjs.org/react-dnd/-/react-dnd-11.1.3.tgz", @@ -91968,6 +92076,14 @@ "prop-types": "^15.0.0" } }, + "react-intersection-observer": { + "version": "8.26.2", + "resolved": "https://registry.npmjs.org/react-intersection-observer/-/react-intersection-observer-8.26.2.tgz", + "integrity": "sha512-GmSjLNK+oV7kS+BHfrJSaA4wF61ELA33gizKHmN+tk59UT6/aW8kkqvlrFGPwxGoaIzLKS2evfG5fgkw5MIIsg==", + "requires": { + "tiny-invariant": "^1.1.0" + } + }, "react-is": { "version": "16.6.3", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.6.3.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index bfe41ac3eae16..a746b8c096529 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -164,12 +164,14 @@ "react-checkbox-tree": "^1.5.1", "react-color": "^2.13.8", "react-datetime": "^3.0.4", + "react-diff-viewer": "^3.1.1", "react-dnd": "^11.1.3", "react-dnd-html5-backend": "^11.1.3", "react-dom": "^16.13.0", "react-draggable": "^4.4.3", "react-gravatar": "^2.6.1", "react-hot-loader": "^4.12.20", + "react-intersection-observer": "^8.26.2", "react-js-cron": "^1.2.0", "react-json-tree": "^0.11.2", "react-jsonschema-form": "^1.2.0", diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 380704f0180b6..fa44063de80a4 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -28,6 +28,7 @@ export enum FeatureFlag { DASHBOARD_CROSS_FILTERS = 'DASHBOARD_CROSS_FILTERS', DASHBOARD_EDIT_CHART_IN_NEW_TAB = 'DASHBOARD_EDIT_CHART_IN_NEW_TAB', DASHBOARD_FILTERS_EXPERIMENTAL = 'DASHBOARD_FILTERS_EXPERIMENTAL', + CONFIRM_DASHBOARD_DIFF = 'CONFIRM_DASHBOARD_DIFF', DASHBOARD_NATIVE_FILTERS = 'DASHBOARD_NATIVE_FILTERS', DASHBOARD_NATIVE_FILTERS_SET = 'DASHBOARD_NATIVE_FILTERS_SET', DASHBOARD_VIRTUALIZATION = 'DASHBOARD_VIRTUALIZATION', diff --git a/superset-frontend/spec/fixtures/mockDashboardState.js b/superset-frontend/spec/fixtures/mockDashboardState.js index 49b71053710a8..b605443af6b02 100644 --- a/superset-frontend/spec/fixtures/mockDashboardState.js +++ b/superset-frontend/spec/fixtures/mockDashboardState.js @@ -1,3 +1,4 @@ +/* eslint-disable theme-colors/no-literal-colors */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -30,3 +31,88 @@ export default { focusedFilterField: null, refreshFrequency: 0, }; + +export const overwriteConfirmMetadata = { + updatedAt: '2022-10-07T16:35:30.924212', + updatedBy: 'Superset Admin', + overwriteConfirmItems: [ + { + keyPath: 'css', + oldValue: '', + newValue: ` + .navbar { + transition: opacity 0.5s ease; + } +`, + }, + { + keyPath: 'json_metadata.filter_scopes', + oldValue: `{ + "122": { + "ethnic_minority": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "gender": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "developer_type": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "lang_at_home": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "country_live": { + "scope": ["ROOT_ID"], + "immune": [] + } + } +}`, + newValue: `{ + "122": { + "ethnic_minority": { + "scope": ["ROOT_ID"], + "immune": [ + 131, + 115, + 123, + 89, + 94, + 71 + ] + }, + "gender": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "developer_type": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "lang_at_home": { + "scope": ["ROOT_ID"], + "immune": [] + }, + "country_live": { + "scope": ["ROOT_ID"], + "immune": [] + } + } +}`, + }, + ], + dashboardId: 9, + data: { + certified_by: '', + certification_details: '', + css: ".navbar {\n transition: opacity 0.5s ease;\n opacity: 0.05;\n}\n.navbar:hover {\n opacity: 1;\n}\n.chart-header .header{\n font-weight: @font-weight-normal;\n font-size: 12px;\n}\n/*\nvar bnbColors = [\n //rausch hackb kazan babu lima beach tirol\n '#ff5a5f', '#7b0051', '#007A87', '#00d1c1', '#8ce071', '#ffb400', '#b4a76c',\n '#ff8083', '#cc0086', '#00a1b3', '#00ffeb', '#bbedab', '#ffd266', '#cbc29a',\n '#ff3339', '#ff1ab1', '#005c66', '#00b3a5', '#55d12e', '#b37e00', '#988b4e',\n ];\n*/\n", + dashboard_title: 'FCC New Coder Survey 2018', + slug: null, + owners: [], + json_metadata: + '{"timed_refresh_immune_slices":[],"expanded_slices":{},"refresh_frequency":0,"default_filters":"{}","color_scheme":"supersetColors","label_colors":{"0":"#FCC700","1":"#A868B7","15":"#3CCCCB","30":"#A38F79","45":"#8FD3E4","age":"#1FA8C9","Yes,":"#1FA8C9","Female":"#454E7C","Prefer":"#5AC189","No,":"#FF7F44","Male":"#666666","Prefer not to say":"#E04355","Ph.D.":"#FCC700","associate\'s degree":"#A868B7","bachelor\'s degree":"#3CCCCB","high school diploma or equivalent (GED)":"#A38F79","master\'s degree (non-professional)":"#8FD3E4","no high school (secondary school)":"#A1A6BD","professional degree (MBA, MD, JD, etc.)":"#ACE1C4","some college credit, no degree":"#FEC0A1","some high school":"#B2B2B2","trade, technical, or vocational training":"#EFA1AA","No, not an ethnic minority":"#1FA8C9","Yes, an ethnic minority":"#454E7C","":"#5AC189","Yes":"#FF7F44","No":"#666666","last_yr_income":"#E04355","More":"#A1A6BD","Less":"#ACE1C4","I":"#FEC0A1","expected_earn":"#B2B2B2","Yes: Willing To":"#EFA1AA","No: Not Willing to":"#FDE380","No Answer":"#D3B3DA","In an Office (with Other Developers)":"#9EE5E5","No Preference":"#D1C6BC","From Home":"#1FA8C9"},"show_native_filters":true,"color_scheme_domain":["#1FA8C9","#454E7C","#5AC189","#FF7F44","#666666","#E04355","#FCC700","#A868B7","#3CCCCB","#A38F79","#8FD3E4","#A1A6BD","#ACE1C4","#FEC0A1","#B2B2B2","#EFA1AA","#FDE380","#D3B3DA","#9EE5E5","#D1C6BC"],"shared_label_colors":{"Male":"#5ac19e","Female":"#1f86c9","":"#5AC189","Prefer not to say":"#47457c","No Answer":"#e05043","Yes, an ethnic minority":"#666666","No, not an ethnic minority":"#ffa444","age":"#1FA8C9"},"filter_scopes":{},"chart_configuration":{},"positions":{}}', + }, +}; diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 701eab78454d4..1afde9655b410 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -32,7 +32,10 @@ import { import { chart as initChart } from 'src/components/Chart/chartReducer'; import { applyDefaultFormData } from 'src/explore/store'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; -import { SAVE_TYPE_OVERWRITE } from 'src/dashboard/util/constants'; +import { + SAVE_TYPE_OVERWRITE, + SAVE_TYPE_OVERWRITE_CONFIRMED, +} from 'src/dashboard/util/constants'; import { addSuccessToast, addWarningToast, @@ -43,6 +46,8 @@ import serializeFilterScopes from 'src/dashboard/util/serializeFilterScopes'; import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; import { safeStringify } from 'src/utils/safeStringify'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; +import { logEvent } from 'src/logger/actions'; +import { LOG_ACTIONS_CONFIRM_OVERWRITE_DASHBOARD_METADATA } from 'src/logger/LogUtils'; import { UPDATE_COMPONENTS_PARENTS_LIST } from './dashboardLayout'; import { setChartConfiguration, @@ -56,6 +61,7 @@ import { updateDirectPathToFilter, } from './dashboardFilters'; import { SET_FILTER_CONFIG_COMPLETE } from './nativeFilters'; +import getOverwriteItems from '../util/getOverwriteItems'; export const SET_UNSAVED_CHANGES = 'SET_UNSAVED_CHANGES'; export function setUnsavedChanges(hasUnsavedChanges) { @@ -189,6 +195,14 @@ export function saveDashboardRequestSuccess(lastModifiedTime) { }; } +export const SET_OVERRIDE_CONFIRM = 'SET_OVERRIDE_CONFIRM'; +export function setOverrideConfirm(overwriteConfirmMetadata) { + return { + type: SET_OVERRIDE_CONFIRM, + overwriteConfirmMetadata, + }; +} + export function saveDashboardRequest(data, id, saveType) { return (dispatch, getState) => { dispatch({ type: UPDATE_COMPONENTS_PARENTS_LIST }); @@ -316,6 +330,7 @@ export function saveDashboardRequest(data, id, saveType) { ); dispatch(addSuccessToast(t('This dashboard was saved successfully.'))); + dispatch(setOverrideConfirm(undefined)); return response; }; @@ -335,34 +350,85 @@ export function saveDashboardRequest(data, id, saveType) { dispatch(addDangerToast(errorText)); }; - if (saveType === SAVE_TYPE_OVERWRITE) { + if ( + [SAVE_TYPE_OVERWRITE, SAVE_TYPE_OVERWRITE_CONFIRMED].includes(saveType) + ) { let chartConfiguration = {}; if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { chartConfiguration = handleChartConfiguration(); } - const updatedDashboard = { - certified_by: cleanedData.certified_by, - certification_details: cleanedData.certification_details, - css: cleanedData.css, - dashboard_title: cleanedData.dashboard_title, - slug: cleanedData.slug, - owners: cleanedData.owners, - roles: cleanedData.roles, - json_metadata: safeStringify({ - ...(cleanedData?.metadata || {}), - default_filters: safeStringify(serializedFilters), - filter_scopes: serializedFilterScopes, - chart_configuration: chartConfiguration, - }), - }; - - return SupersetClient.put({ - endpoint: `/api/v1/dashboard/${id}`, - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(updatedDashboard), + const updatedDashboard = + saveType === SAVE_TYPE_OVERWRITE_CONFIRMED + ? data + : { + certified_by: cleanedData.certified_by, + certification_details: cleanedData.certification_details, + css: cleanedData.css, + dashboard_title: cleanedData.dashboard_title, + slug: cleanedData.slug, + owners: cleanedData.owners, + roles: cleanedData.roles, + json_metadata: safeStringify({ + ...(cleanedData?.metadata || {}), + default_filters: safeStringify(serializedFilters), + filter_scopes: serializedFilterScopes, + chart_configuration: chartConfiguration, + }), + }; + + const updateDashboard = () => + SupersetClient.put({ + endpoint: `/api/v1/dashboard/${id}`, + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(updatedDashboard), + }) + .then(response => onUpdateSuccess(response)) + .catch(response => onError(response)); + return new Promise((resolve, reject) => { + if ( + !isFeatureEnabled(FeatureFlag.CONFIRM_DASHBOARD_DIFF) || + saveType === SAVE_TYPE_OVERWRITE_CONFIRMED + ) { + // skip overwrite precheck + resolve(); + return; + } + + // precheck for overwrite items + SupersetClient.get({ + endpoint: `/api/v1/dashboard/${id}`, + }).then(response => { + const dashboard = response.json.result; + const overwriteConfirmItems = getOverwriteItems( + dashboard, + updatedDashboard, + ); + if (overwriteConfirmItems.length > 0) { + dispatch( + setOverrideConfirm({ + updatedAt: dashboard.changed_on, + updatedBy: dashboard.changed_by_name, + overwriteConfirmItems, + dashboardId: id, + data: updatedDashboard, + }), + ); + return reject(overwriteConfirmItems); + } + return resolve(); + }); }) - .then(response => onUpdateSuccess(response)) - .catch(response => onError(response)); + .then(updateDashboard) + .catch(overwriteConfirmItems => { + const errorText = t('Please confirm the overwrite values.'); + dispatch( + logEvent(LOG_ACTIONS_CONFIRM_OVERWRITE_DASHBOARD_METADATA, { + dashboard_id: id, + items: overwriteConfirmItems, + }), + ); + dispatch(addDangerToast(errorText)); + }); } // changing the data as the endpoint requires const copyData = { ...cleanedData }; diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js index c985496f892d8..25aa54ed6638a 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.test.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.test.js @@ -18,14 +18,21 @@ */ import sinon from 'sinon'; import { SupersetClient } from '@superset-ui/core'; +import { waitFor } from '@testing-library/react'; import { removeSliceFromDashboard, saveDashboardRequest, + SET_OVERRIDE_CONFIRM, } from 'src/dashboard/actions/dashboardState'; import { REMOVE_FILTER } from 'src/dashboard/actions/dashboardFilters'; +import * as featureFlags from 'src/featureFlags'; import { UPDATE_COMPONENTS_PARENTS_LIST } from 'src/dashboard/actions/dashboardLayout'; -import { DASHBOARD_GRID_ID } from 'src/dashboard/util/constants'; +import { + DASHBOARD_GRID_ID, + SAVE_TYPE_OVERWRITE, + SAVE_TYPE_OVERWRITE_CONFIRMED, +} from 'src/dashboard/util/constants'; import { filterId, sliceEntitiesForDashboard as sliceEntities, @@ -55,13 +62,32 @@ describe('dashboardState actions', () => { const newDashboardData = mockDashboardData; let postStub; + let getStub; + let putStub; + const updatedCss = '.updated_css_value {\n color: black;\n}'; + beforeEach(() => { postStub = sinon .stub(SupersetClient, 'post') .resolves('the value you want to return'); + getStub = sinon.stub(SupersetClient, 'get').resolves({ + json: { + result: { + ...mockDashboardData, + css: updatedCss, + }, + }, + }); + putStub = sinon.stub(SupersetClient, 'put').resolves({ + json: { + result: mockDashboardData, + }, + }); }); afterEach(() => { postStub.restore(); + getStub.restore(); + putStub.restore(); }); function setup(stateOverrides) { @@ -111,6 +137,58 @@ describe('dashboardState actions', () => { mockParentsList, ); }); + + describe('FeatureFlag.CONFIRM_DASHBOARD_DIFF', () => { + let isFeatureEnabledMock; + beforeEach(() => { + isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation(feature => feature === 'CONFIRM_DASHBOARD_DIFF'); + }); + + afterEach(() => { + isFeatureEnabledMock.mockRestore(); + }); + + it('dispatches SET_OVERRIDE_CONFIRM when an inspect value has diff', async () => { + const id = 192; + const { getState, dispatch } = setup(); + const thunk = saveDashboardRequest( + newDashboardData, + id, + SAVE_TYPE_OVERWRITE, + ); + thunk(dispatch, getState); + expect(getStub.callCount).toBe(1); + expect(postStub.callCount).toBe(0); + await waitFor(() => + expect(dispatch.getCall(1).args[0].type).toBe(SET_OVERRIDE_CONFIRM), + ); + expect( + dispatch.getCall(1).args[0].overwriteConfirmMetadata.dashboardId, + ).toBe(id); + }); + + it('should post dashboard data with after confirm the overwrite values', async () => { + const id = 192; + const { getState, dispatch } = setup(); + const confirmedDashboardData = { + ...newDashboardData, + css: updatedCss, + }; + const thunk = saveDashboardRequest( + confirmedDashboardData, + id, + SAVE_TYPE_OVERWRITE_CONFIRMED, + ); + thunk(dispatch, getState); + expect(getStub.callCount).toBe(0); + expect(postStub.callCount).toBe(0); + await waitFor(() => expect(putStub.callCount).toBe(1)); + const { body } = putStub.getCall(0).args[0]; + expect(body).toBe(JSON.stringify(confirmedDashboardData)); + }); + }); }); it('should dispatch removeFilter if a removed slice is a filter_box', () => { diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index c8b4c0cdde50b..ca3566345b6dd 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -56,6 +56,7 @@ import setPeriodicRunner, { import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants'; import { PageHeaderWithActions } from 'src/components/PageHeaderWithActions'; import { DashboardEmbedModal } from '../DashboardEmbedControls'; +import OverwriteConfirm from '../OverwriteConfirm'; const uiOverrideRegistry = getUiOverrideRegistry(); @@ -696,6 +697,8 @@ class Header extends React.PureComponent { /> )} + + {userCanCurate && ( { + const { queryByText } = render(, { + useRedux: true, + store: mockStore({ dashboardState: {} }), + }); + expect(queryByText('Confirm overwrite')).not.toBeInTheDocument(); +}); + +test('renders confirm modal on overwriteConfirmMetadata is provided', async () => { + const { queryByText } = render(, { + useRedux: true, + store: mockStore({ + dashboardState: { + overwriteConfirmMetadata, + }, + }), + }); + await waitFor(() => + expect(queryByText('Confirm overwrite')).toBeInTheDocument(), + ); +}); diff --git a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx new file mode 100644 index 0000000000000..751def6ebb65b --- /dev/null +++ b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 React from 'react'; +import thunk from 'redux-thunk'; +import configureStore from 'redux-mock-store'; +import fetchMock from 'fetch-mock'; +import { mockAllIsIntersecting } from 'react-intersection-observer/test-utils'; + +import { fireEvent, render, waitFor } from 'spec/helpers/testing-library'; +import { overwriteConfirmMetadata } from 'spec/fixtures/mockDashboardState'; +import OverwriteConfirmModal from './OverwriteConfirmModal'; + +const middlewares = [thunk]; +const mockStore = configureStore(middlewares); + +jest.mock('react-diff-viewer', () => () => ( +
+)); + +test('renders diff viewer when it contains overwriteConfirmMetadata', async () => { + const { queryByText, findAllByTestId } = render( + , + { + useRedux: true, + store: mockStore(), + }, + ); + expect(queryByText('Confirm overwrite')).toBeInTheDocument(); + const diffViewers = await findAllByTestId('mock-diff-viewer'); + expect(diffViewers).toHaveLength( + overwriteConfirmMetadata.overwriteConfirmItems.length, + ); +}); + +test('requests update dashboard api when save button is clicked', async () => { + const updateDashboardEndpoint = `glob:*/api/v1/dashboard/${overwriteConfirmMetadata.dashboardId}`; + fetchMock.put(updateDashboardEndpoint, { + id: overwriteConfirmMetadata.dashboardId, + last_modified_time: +new Date(), + result: overwriteConfirmMetadata.data, + }); + const store = mockStore({ + dashboardLayout: {}, + dashboardFilters: {}, + }); + const { findByTestId } = render( + , + { + useRedux: true, + store, + }, + ); + const saveButton = await findByTestId('overwrite-confirm-save-button'); + expect(fetchMock.calls(updateDashboardEndpoint)).toHaveLength(0); + fireEvent.click(saveButton); + expect(fetchMock.calls(updateDashboardEndpoint)).toHaveLength(0); + mockAllIsIntersecting(true); + fireEvent.click(saveButton); + await waitFor(() => + expect(fetchMock.calls(updateDashboardEndpoint)?.[0]?.[1]?.body).toEqual( + JSON.stringify(overwriteConfirmMetadata.data), + ), + ); + await waitFor(() => + expect(store.getActions()).toContainEqual({ + type: 'SET_OVERRIDE_CONFIRM', + overwriteConfirmMetadata: undefined, + }), + ); +}); diff --git a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.tsx b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.tsx new file mode 100644 index 0000000000000..32fea9b4fc140 --- /dev/null +++ b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.tsx @@ -0,0 +1,209 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 React, { useMemo, useCallback, RefObject, createRef } from 'react'; +import moment from 'moment'; +import { useDispatch } from 'react-redux'; +import ReactDiffViewer from 'react-diff-viewer'; +import { useInView } from 'react-intersection-observer'; +import Modal from 'src/components/Modal'; +import Button from 'src/components/Button'; +import { DashboardState } from 'src/dashboard/types'; +import { + saveDashboardRequest, + setOverrideConfirm, +} from 'src/dashboard/actions/dashboardState'; +import { t, styled } from '@superset-ui/core'; +import { SAVE_TYPE_OVERWRITE_CONFIRMED } from 'src/dashboard/util/constants'; + +const STICKY_HEADER_TOP = 16; +const STICKY_HEADER_HEIGHT = 32; + +const StyledTitle = styled.h2` + ${({ theme }) => ` + color: ${theme.colors.grayscale.dark1} + `} +`; + +const StyledEditor = styled.div` + ${({ theme }) => ` + table { + border: 1px ${theme.colors.grayscale.light2} solid; + } + pre { + font-size: 11px; + padding: 0px; + background-color: transparent; + border: 0px; + line-height: 110%; + } + `} +`; + +const StackableHeader = styled(Button)<{ top: number }>` + ${({ theme, top }) => ` + position: sticky; + top: ${top}px; + background-color: ${theme.colors.grayscale.light5}; + margin: 0px; + padding: 8px 4px; + z-index: 1; + border: 0px; + border-radius: 0px; + width: 100%; + justify-content: flex-start; + border-bottom: 1px ${theme.colors.grayscale.light1} solid; + &::before { + display: inline-block; + position: relative; + opacity: 1; + content: "\\00BB"; + } + `} +`; + +const StyledBottom = styled.div<{ inView: boolean }>` + ${({ inView }) => ` + margin: 8px auto; + text-align: center; + opacity: ${inView ? 0 : 1}; + `} +`; + +type Props = { + overwriteConfirmMetadata: DashboardState['overwriteConfirmMetadata']; +}; + +const OverrideConfirmModal = ({ overwriteConfirmMetadata }: Props) => { + const [bottomRef, hasReviewed] = useInView({ triggerOnce: true }); + const dispatch = useDispatch(); + const onHide = useCallback( + () => dispatch(setOverrideConfirm(undefined)), + [dispatch], + ); + const anchors = useMemo[]>( + () => + overwriteConfirmMetadata + ? overwriteConfirmMetadata.overwriteConfirmItems.map(() => + createRef(), + ) + : [], + [overwriteConfirmMetadata], + ); + const onAnchorClicked = useCallback( + (index: number) => { + anchors[index]?.current?.scrollIntoView({ behavior: 'smooth' }); + }, + [anchors], + ); + const onConfirmOverwrite = useCallback(() => { + if (overwriteConfirmMetadata) { + dispatch( + saveDashboardRequest( + overwriteConfirmMetadata.data, + overwriteConfirmMetadata.dashboardId, + SAVE_TYPE_OVERWRITE_CONFIRMED, + ), + ); + } + }, [dispatch, overwriteConfirmMetadata]); + + return ( + + {t('Scroll down to the bottom to enable overwriting changes. ')} + + + + } + onHide={onHide} + > + {overwriteConfirmMetadata && ( + <> + + {t('Are you sure you intend to overwrite the following values?')} + + + {overwriteConfirmMetadata.overwriteConfirmItems.map( + ({ keyPath, oldValue, newValue }, index) => ( + +
+ onAnchorClicked(index)} + > + {keyPath} + + + + ), + )} + + {/* Add submit button at the bottom in case of intersection-observer fallback */} + + + + + )} + + ); +}; + +export default OverrideConfirmModal; diff --git a/superset-frontend/src/dashboard/components/OverwriteConfirm/index.tsx b/superset-frontend/src/dashboard/components/OverwriteConfirm/index.tsx new file mode 100644 index 0000000000000..46e4f3ab50146 --- /dev/null +++ b/superset-frontend/src/dashboard/components/OverwriteConfirm/index.tsx @@ -0,0 +1,41 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 React from 'react'; +import { useSelector } from 'react-redux'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; +import { DashboardState, RootState } from 'src/dashboard/types'; + +const Modal = AsyncEsmComponent(() => import('./OverwriteConfirmModal')); + +const OverrideConfirm = () => { + const overwriteConfirmMetadata = useSelector< + RootState, + DashboardState['overwriteConfirmMetadata'] + >(({ dashboardState }) => dashboardState.overwriteConfirmMetadata); + + return ( + <> + {overwriteConfirmMetadata && ( + + )} + + ); +}; + +export default OverrideConfirm; diff --git a/superset-frontend/src/dashboard/constants.ts b/superset-frontend/src/dashboard/constants.ts index ab9d9b5967a95..97753abe45c54 100644 --- a/superset-frontend/src/dashboard/constants.ts +++ b/superset-frontend/src/dashboard/constants.ts @@ -41,3 +41,4 @@ export const OPEN_FILTER_BAR_MAX_WIDTH = 550; export const FILTER_BAR_HEADER_HEIGHT = 80; export const FILTER_BAR_TABS_HEIGHT = 46; export const BUILDER_SIDEPANEL_WIDTH = 374; +export const OVERWRITE_INSPECT_FIELDS = ['css', 'json_metadata.filter_scopes']; diff --git a/superset-frontend/src/dashboard/reducers/dashboardState.js b/superset-frontend/src/dashboard/reducers/dashboardState.js index 14b35caef04a7..1c339001d17d6 100644 --- a/superset-frontend/src/dashboard/reducers/dashboardState.js +++ b/superset-frontend/src/dashboard/reducers/dashboardState.js @@ -42,6 +42,7 @@ import { ON_FILTERS_REFRESH, ON_FILTERS_REFRESH_SUCCESS, SET_DATASETS_STATUS, + SET_OVERRIDE_CONFIRM, } from '../actions/dashboardState'; import { HYDRATE_DASHBOARD } from '../actions/hydrate'; @@ -173,6 +174,12 @@ export default function dashboardStateReducer(state = {}, action) { activeTabs: Array.from(newActiveTabs), }; }, + [SET_OVERRIDE_CONFIRM]() { + return { + ...state, + overwriteConfirmMetadata: action.overwriteConfirmMetadata, + }; + }, [SET_FOCUSED_FILTER_FIELD]() { return { ...state, diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 57a317bb83454..540acb980f17d 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -72,6 +72,17 @@ export type DashboardState = { chartId: number; column: string; }; + overwriteConfirmMetadata?: { + updatedAt: string; + updatedBy: string; + overwriteConfirmItems: { + keyPath: string; + oldValue: string; + newValue: string; + }[]; + dashboardId: number; + data: JsonObject; + }; }; export type DashboardInfo = { id: number; diff --git a/superset-frontend/src/dashboard/util/constants.ts b/superset-frontend/src/dashboard/util/constants.ts index 640028eb4e947..0743d7a5a42d3 100644 --- a/superset-frontend/src/dashboard/util/constants.ts +++ b/superset-frontend/src/dashboard/util/constants.ts @@ -58,6 +58,7 @@ export const UNDO_LIMIT = 50; // save dash options export const SAVE_TYPE_OVERWRITE = 'overwrite'; +export const SAVE_TYPE_OVERWRITE_CONFIRMED = 'overwriteConfirmed'; export const SAVE_TYPE_NEWDASHBOARD = 'newDashboard'; // default dashboard layout data size limit diff --git a/superset-frontend/src/dashboard/util/getOverwriteItems.test.ts b/superset-frontend/src/dashboard/util/getOverwriteItems.test.ts new file mode 100644 index 0000000000000..e4328fb08897d --- /dev/null +++ b/superset-frontend/src/dashboard/util/getOverwriteItems.test.ts @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 getOverwriteItems from './getOverwriteItems'; + +test('returns diff items', () => { + const prevFilterScopes = { + filter1: { + scope: ['abc'], + immune: [], + }, + }; + const nextFilterScopes = { + scope: ['ROOT_ID'], + immune: ['efg'], + }; + + const prevValue = { + css: '', + json_metadata: JSON.stringify({ + filter_scopes: prevFilterScopes, + default_filters: {}, + }), + }; + + const nextValue = { + css: '.updated_css {color: white;}', + json_metadata: JSON.stringify({ + filter_scopes: nextFilterScopes, + default_filters: {}, + }), + }; + expect(getOverwriteItems(prevValue, nextValue)).toEqual([ + { keyPath: 'css', newValue: nextValue.css, oldValue: prevValue.css }, + { + keyPath: 'json_metadata.filter_scopes', + newValue: JSON.stringify(nextFilterScopes, null, 2), + oldValue: JSON.stringify(prevFilterScopes, null, 2), + }, + ]); +}); diff --git a/superset-frontend/src/dashboard/util/getOverwriteItems.ts b/superset-frontend/src/dashboard/util/getOverwriteItems.ts new file mode 100644 index 0000000000000..5301cb03af4de --- /dev/null +++ b/superset-frontend/src/dashboard/util/getOverwriteItems.ts @@ -0,0 +1,44 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 { JsonObject } from '@superset-ui/core'; +import { OVERWRITE_INSPECT_FIELDS } from 'src/dashboard/constants'; + +const JSON_KEYS = new Set(['json_metadata', 'position_json']); + +function extractValue(object: JsonObject, keyPath: string) { + return keyPath.split('.').reduce((obj: JsonObject, key: string) => { + const value = obj?.[key]; + return JSON_KEYS.has(key) && value ? JSON.parse(value) : value; + }, object); +} + +export default function getOverwriteItems(prev: JsonObject, next: JsonObject) { + return OVERWRITE_INSPECT_FIELDS.map(keyPath => ({ + keyPath, + ...(keyPath.split('.').find(key => JSON_KEYS.has(key)) + ? { + oldValue: JSON.stringify(extractValue(prev, keyPath), null, 2) || '', + newValue: JSON.stringify(extractValue(next, keyPath), null, 2) || '', + } + : { + oldValue: extractValue(prev, keyPath) || '', + newValue: extractValue(next, keyPath) || '', + }), + })).filter(({ oldValue, newValue }) => oldValue !== newValue); +} diff --git a/superset-frontend/src/logger/LogUtils.ts b/superset-frontend/src/logger/LogUtils.ts index 986bde816fb69..60f0fe183301e 100644 --- a/superset-frontend/src/logger/LogUtils.ts +++ b/superset-frontend/src/logger/LogUtils.ts @@ -45,6 +45,8 @@ export const LOG_ACTIONS_DATASET_CREATION_TABLE_CANCELLATION = 'dataset_creation_table_cancellation'; export const LOG_ACTIONS_DATASET_CREATION_SUCCESS = 'dataset_creation_success'; export const LOG_ACTIONS_SPA_NAVIGATION = 'spa_navigation'; +export const LOG_ACTIONS_CONFIRM_OVERWRITE_DASHBOARD_METADATA = + 'confirm_overwrite_dashboard_metadata'; // Log event types -------------------------------------------------------------- export const LOG_EVENT_TYPE_TIMING = new Set([ @@ -64,6 +66,7 @@ export const LOG_EVENT_TYPE_USER = new Set([ LOG_ACTIONS_FORCE_REFRESH_DASHBOARD, LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, LOG_ACTIONS_MOUNT_EXPLORER, + LOG_ACTIONS_CONFIRM_OVERWRITE_DASHBOARD_METADATA, ]); export const LOG_EVENT_DATASET_TYPE_DATASET_CREATION = [