You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2023/12/01 01:08:05 UTC
(superset) branch grit/8d87ae06-afaa-469d-abfb-1846398e01eb created (now 7f0b7826a4)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a change to branch grit/8d87ae06-afaa-469d-abfb-1846398e01eb
in repository https://gitbox.apache.org/repos/asf/superset.git
at 7f0b7826a4 update tests
This branch includes the following new commits:
new c341c76c7c [bot] migrate files
new 68f0ec11f3 lint
new 7f0b7826a4 update tests
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 02/03: lint
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch grit/8d87ae06-afaa-469d-abfb-1846398e01eb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 68f0ec11f3851380d700395657d037d71c8ddd9b
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Tue Nov 28 15:49:54 2023 -0800
lint
---
.../src/components/AlteredSliceTag/index.jsx | 150 +++++++++++----------
1 file changed, 78 insertions(+), 72 deletions(-)
diff --git a/superset-frontend/src/components/AlteredSliceTag/index.jsx b/superset-frontend/src/components/AlteredSliceTag/index.jsx
index 417a6a1562..a268625393 100644
--- a/superset-frontend/src/components/AlteredSliceTag/index.jsx
+++ b/superset-frontend/src/components/AlteredSliceTag/index.jsx
@@ -63,56 +63,8 @@ function alterForComparison(value) {
return value;
}
-const AlteredSliceTag = (props) => {
-const diffs = getDiffsHandler(props);
- const controlsMap = getControlsForVizType(props.origFormData.viz_type);
- const rows = getRowsFromDiffsHandler(diffs, controlsMap);
-
- const [hasDiffs, setHasDiffs] = useState(!isEmpty(diffs));
-
- const UNSAFE_componentWillReceivePropsHandler = useCallback((newProps) => {
- // Update differences if need be
- if (isEqual(props, newProps)) {
- return;
- }
- const diffs = getDiffsHandler(newProps);
- setStateHandler(prevState => ({
- rows: getRowsFromDiffsHandler(diffs, prevState.controlsMap),
- hasDiffs: !isEmpty(diffs),
- }));
- }, []);
- const getRowsFromDiffsHandler = useCallback((diffs, controlsMap) => {
- return Object.entries(diffs).map(([key, diff]) => ({
- control: (controlsMap[key] && controlsMap[key].label) || key,
- before: formatValueHandler(diff.before, key, controlsMap),
- after: formatValueHandler(diff.after, key, controlsMap),
- }));
- }, []);
- const getDiffsHandler = useCallback((props) => {
- // Returns all properties that differ in the
- // current form data and the saved form data
- 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', 'where'].includes(fdKey)) {
- return;
- }
- if (!isEqualishHandler(ofd[fdKey], cfd[fdKey])) {
- diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
- }
- });
- return diffs;
- }, []);
- const isEqualishHandler = useCallback((val1, val2) => {
- return isEqual(alterForComparison(val1), alterForComparison(val2));
- }, []);
- const formatValueHandler = useCallback((value, key, controlsMap) => {
+const AlteredSliceTag = props => {
+ const formatValueHandler = useCallback((value, key, controlsMap) => {
// Format display value based on the control type
// or the value type
if (value === undefined) {
@@ -160,7 +112,63 @@ const diffs = getDiffsHandler(props);
}
return safeStringify(value);
}, []);
- const renderModalBodyHandler = useCallback(() => {
+
+ const getRowsFromDiffsHandler = useCallback(
+ (diffs, controlsMap) =>
+ Object.entries(diffs).map(([key, diff]) => ({
+ control: (controlsMap[key] && controlsMap[key].label) || key,
+ before: formatValueHandler(diff.before, key, controlsMap),
+ after: formatValueHandler(diff.after, key, controlsMap),
+ })),
+ [],
+ );
+
+ const isEqualishHandler = useCallback(
+ (val1, val2) => isEqual(alterForComparison(val1), alterForComparison(val2)),
+ [],
+ );
+
+ const getDiffsHandler = useCallback(props => {
+ // Returns all properties that differ in the
+ // current form data and the saved form data
+ 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', 'where'].includes(fdKey)) {
+ return;
+ }
+ if (!isEqualishHandler(ofd[fdKey], cfd[fdKey])) {
+ diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
+ }
+ });
+ return diffs;
+ }, []);
+
+ const diffs = getDiffsHandler(props);
+ const controlsMap = getControlsForVizType(props.origFormData.viz_type);
+ const rows = getRowsFromDiffsHandler(diffs, controlsMap);
+
+ const [hasDiffs] = useState(!isEmpty(diffs));
+
+ // const UNSAFE_componentWillReceivePropsHandler = useCallback(newProps => {
+ // // Update differences if need be
+ // if (isEqual(props, newProps)) {
+ // return;
+ // }
+ // const diffs = getDiffsHandler(newProps);
+ // setStateHandler(prevState => ({
+ // rows: getRowsFromDiffsHandler(diffs, prevState.controlsMap),
+ // hasDiffs: !isEmpty(diffs),
+ // }));
+ // }, []);
+
+ const renderModalBodyHandler = useCallback(() => {
const columns = [
{
accessor: 'control',
@@ -188,33 +196,31 @@ const diffs = getDiffsHandler(props);
/>
);
}, []);
- const renderTriggerNodeHandler = useCallback(() => {
- return (
+ const renderTriggerNodeHandler = useCallback(
+ () => (
<Tooltip id="difference-tooltip" title={t('Click to see difference')}>
<StyledLabel className="label">{t('Altered')}</StyledLabel>
</Tooltip>
- );
- }, []);
+ ),
+ [],
+ );
- if (!hasDiffs) {
- return null;
- }
- // Render the label-warning 'Altered' tag which the user may
- // click to open a modal containing a table summarizing the
- // differences in the slice
- return (
- <ModalTrigger
- triggerNode={renderTriggerNodeHandler()}
- modalTitle={t('Chart changes')}
- modalBody={renderModalBodyHandler()}
- responsive
- />
- );
+ if (!hasDiffs) {
+ return null;
+ }
+ // Render the label-warning 'Altered' tag which the user may
+ // click to open a modal containing a table summarizing the
+ // differences in the slice
+ return (
+ <ModalTrigger
+ triggerNode={renderTriggerNodeHandler()}
+ modalTitle={t('Chart changes')}
+ modalBody={renderModalBodyHandler()}
+ responsive
+ />
+ );
};
export default AlteredSliceTag;
-
-
-
AlteredSliceTag.propTypes = propTypes;
(superset) 01/03: [bot] migrate files
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch grit/8d87ae06-afaa-469d-abfb-1846398e01eb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c341c76c7c19d7dcb02d936e2bb6f8ecb366f2ab
Author: grit-app[bot] <gr...@users.noreply.github.com>
AuthorDate: Wed Nov 22 01:05:07 2023 +0000
[bot] migrate files
---
.../src/components/AlteredSliceTag/index.jsx | 84 ++++++++++------------
1 file changed, 39 insertions(+), 45 deletions(-)
diff --git a/superset-frontend/src/components/AlteredSliceTag/index.jsx b/superset-frontend/src/components/AlteredSliceTag/index.jsx
index 83458fc0a4..417a6a1562 100644
--- a/superset-frontend/src/components/AlteredSliceTag/index.jsx
+++ b/superset-frontend/src/components/AlteredSliceTag/index.jsx
@@ -16,7 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React from 'react';
+
+import React, { useState, useCallback } from 'react';
import PropTypes from 'prop-types';
import { isEqual, isEmpty } from 'lodash';
import { styled, t } from '@superset-ui/core';
@@ -62,37 +63,32 @@ function alterForComparison(value) {
return value;
}
-export default class AlteredSliceTag extends React.Component {
- constructor(props) {
- super(props);
- const diffs = this.getDiffs(props);
- const controlsMap = getControlsForVizType(this.props.origFormData.viz_type);
- const rows = this.getRowsFromDiffs(diffs, controlsMap);
+const AlteredSliceTag = (props) => {
+const diffs = getDiffsHandler(props);
+ const controlsMap = getControlsForVizType(props.origFormData.viz_type);
+ const rows = getRowsFromDiffsHandler(diffs, controlsMap);
- this.state = { rows, hasDiffs: !isEmpty(diffs), controlsMap };
- }
+ const [hasDiffs, setHasDiffs] = useState(!isEmpty(diffs));
- UNSAFE_componentWillReceiveProps(newProps) {
+ const UNSAFE_componentWillReceivePropsHandler = useCallback((newProps) => {
// Update differences if need be
- if (isEqual(this.props, newProps)) {
+ if (isEqual(props, newProps)) {
return;
}
- const diffs = this.getDiffs(newProps);
- this.setState(prevState => ({
- rows: this.getRowsFromDiffs(diffs, prevState.controlsMap),
+ const diffs = getDiffsHandler(newProps);
+ setStateHandler(prevState => ({
+ rows: getRowsFromDiffsHandler(diffs, prevState.controlsMap),
hasDiffs: !isEmpty(diffs),
}));
- }
-
- getRowsFromDiffs(diffs, controlsMap) {
+ }, []);
+ const getRowsFromDiffsHandler = useCallback((diffs, controlsMap) => {
return Object.entries(diffs).map(([key, diff]) => ({
control: (controlsMap[key] && controlsMap[key].label) || key,
- before: this.formatValue(diff.before, key, controlsMap),
- after: this.formatValue(diff.after, key, controlsMap),
+ before: formatValueHandler(diff.before, key, controlsMap),
+ after: formatValueHandler(diff.after, key, controlsMap),
}));
- }
-
- getDiffs(props) {
+ }, []);
+ const getDiffsHandler = useCallback((props) => {
// Returns all properties that differ in the
// current form data and the saved form data
const ofd = sanitizeFormData(props.origFormData);
@@ -107,18 +103,16 @@ export default class AlteredSliceTag extends React.Component {
if (['filters', 'having', 'where'].includes(fdKey)) {
return;
}
- if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) {
+ if (!isEqualishHandler(ofd[fdKey], cfd[fdKey])) {
diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
}
});
return diffs;
- }
-
- isEqualish(val1, val2) {
+ }, []);
+ const isEqualishHandler = useCallback((val1, val2) => {
return isEqual(alterForComparison(val1), alterForComparison(val2));
- }
-
- formatValue(value, key, controlsMap) {
+ }, []);
+ const formatValueHandler = useCallback((value, key, controlsMap) => {
// Format display value based on the control type
// or the value type
if (value === undefined) {
@@ -165,9 +159,8 @@ export default class AlteredSliceTag extends React.Component {
return value;
}
return safeStringify(value);
- }
-
- renderModalBody() {
+ }, []);
+ const renderModalBodyHandler = useCallback(() => {
const columns = [
{
accessor: 'control',
@@ -188,25 +181,22 @@ export default class AlteredSliceTag extends React.Component {
return (
<TableView
columns={columns}
- data={this.state.rows}
+ data={rows}
pageSize={50}
className="table-condensed"
columnsForWrapText={columnsForWrapText}
/>
);
- }
-
- renderTriggerNode() {
+ }, []);
+ const renderTriggerNodeHandler = useCallback(() => {
return (
<Tooltip id="difference-tooltip" title={t('Click to see difference')}>
<StyledLabel className="label">{t('Altered')}</StyledLabel>
</Tooltip>
);
- }
+ }, []);
- render() {
- // Return nothing if there are no differences
- if (!this.state.hasDiffs) {
+ if (!hasDiffs) {
return null;
}
// Render the label-warning 'Altered' tag which the user may
@@ -214,13 +204,17 @@ export default class AlteredSliceTag extends React.Component {
// differences in the slice
return (
<ModalTrigger
- triggerNode={this.renderTriggerNode()}
+ triggerNode={renderTriggerNodeHandler()}
modalTitle={t('Chart changes')}
- modalBody={this.renderModalBody()}
+ modalBody={renderModalBodyHandler()}
responsive
/>
- );
- }
-}
+ );
+};
+
+export default AlteredSliceTag;
+
+
+
AlteredSliceTag.propTypes = propTypes;
(superset) 03/03: update tests
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch grit/8d87ae06-afaa-469d-abfb-1846398e01eb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7f0b7826a4ba1699287ed44997f9adbadd4fcc74
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Thu Nov 30 15:29:45 2023 -0800
update tests
---
.../AlteredSliceTag/AlteredSliceTag.test.jsx | 436 +++++++--------------
.../src/components/AlteredSliceTag/index.jsx | 170 ++++----
2 files changed, 220 insertions(+), 386 deletions(-)
diff --git a/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx b/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx
index 7501ce6382..85d8afb447 100644
--- a/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx
+++ b/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx
@@ -1,341 +1,169 @@
-/**
- * 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 { styledMount as mount } from 'spec/helpers/theming';
-import { getChartControlPanelRegistry } from '@superset-ui/core';
+import '@testing-library/jest-dom/extend-expect';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import AlteredSliceTag, {
+ alterForComparison,
+ formatValueHandler,
+} from 'src/components/AlteredSliceTag';
+import { ThemeProvider, supersetTheme } from '@superset-ui/core';
+import { defaultProps, expectedRows } from './AlteredSliceTagMocks';
+
+const renderWithTheme = ui =>
+ render(<ThemeProvider theme={supersetTheme}>{ui}</ThemeProvider>);
-import AlteredSliceTag from 'src/components/AlteredSliceTag';
-import ModalTrigger from 'src/components/ModalTrigger';
-import { Tooltip } from 'src/components/Tooltip';
-import TableCollection from 'src/components/TableCollection';
-import TableView from 'src/components/TableView';
+describe('AlteredSliceTag', () => {
+ it('renders the "Altered" label', () => {
+ renderWithTheme(<AlteredSliceTag {...defaultProps} />);
-import {
- defaultProps,
- expectedDiffs,
- expectedRows,
- fakePluginControls,
-} from './AlteredSliceTagMocks';
+ const alteredLabel = screen.getByText('Altered');
+ expect(alteredLabel).toBeInTheDocument();
+ });
-const getTableWrapperFromModalBody = modalBody =>
- modalBody.find(TableView).find(TableCollection);
+ it('opens the modal on click', () => {
+ renderWithTheme(<AlteredSliceTag {...defaultProps} />);
-describe('AlteredSliceTag', () => {
- let wrapper;
- let props;
- let controlsMap;
+ const alteredLabel = screen.getByText('Altered');
+ userEvent.click(alteredLabel);
- beforeEach(() => {
- getChartControlPanelRegistry().registerValue(
- 'altered_slice_tag_spec',
- fakePluginControls,
- );
- props = { ...defaultProps };
- wrapper = mount(<AlteredSliceTag {...props} />);
- ({ controlsMap } = wrapper.instance().state);
+ const modalTitle = screen.getByText('Chart changes');
+ expect(modalTitle).toBeInTheDocument();
});
- it('correctly determines form data differences', () => {
- const diffs = wrapper.instance().getDiffs(props);
- expect(diffs).toEqual(expectedDiffs);
- expect(wrapper.instance().state.rows).toEqual(expectedRows);
- expect(wrapper.instance().state.hasDiffs).toBe(true);
- });
+ it('displays the differences in the modal', () => {
+ renderWithTheme(<AlteredSliceTag {...defaultProps} />);
- it('does not run when there are no differences', () => {
- props = {
- origFormData: props.origFormData,
- currentFormData: props.origFormData,
- };
- wrapper = mount(<AlteredSliceTag {...props} />);
- expect(wrapper.instance().state.rows).toEqual([]);
- expect(wrapper.instance().state.hasDiffs).toBe(false);
- expect(wrapper.instance().render()).toBeNull();
- });
+ const alteredLabel = screen.getByText('Altered');
+ userEvent.click(alteredLabel);
- it('does not run when temporary controls have changes', () => {
- props = {
- origFormData: { ...props.origFormData, url_params: { foo: 'foo' } },
- currentFormData: { ...props.origFormData, url_params: { bar: 'bar' } },
- };
- wrapper = mount(<AlteredSliceTag {...props} />);
- expect(wrapper.instance().state.rows).toEqual([]);
- expect(wrapper.instance().state.hasDiffs).toBe(false);
- expect(wrapper.instance().render()).toBeNull();
+ const beforeValue = screen.getByText('1, 2, 3, 4');
+ const afterValue = screen.getByText('a, b, c, d');
+ expect(beforeValue).toBeInTheDocument();
+ expect(afterValue).toBeInTheDocument();
});
- it('sets new rows when receiving new props', () => {
- const testRows = ['testValue'];
- const getRowsFromDiffsStub = jest
- .spyOn(AlteredSliceTag.prototype, 'getRowsFromDiffs')
- .mockReturnValueOnce(testRows);
- const newProps = {
- currentFormData: { ...props.currentFormData },
- origFormData: { ...props.origFormData },
- };
- wrapper = mount(<AlteredSliceTag {...props} />);
- const wrapperInstance = wrapper.instance();
- wrapperInstance.UNSAFE_componentWillReceiveProps(newProps);
- expect(getRowsFromDiffsStub).toHaveBeenCalled();
- expect(wrapperInstance.state.rows).toEqual(testRows);
- });
+ it('does not render anything if there are no differences', () => {
+ renderWithTheme(
+ <AlteredSliceTag
+ {...defaultProps}
+ currentFormData={defaultProps.origFormData}
+ />,
+ );
- it('does not set new state when props are the same', () => {
- const currentRows = wrapper.instance().state.rows;
- wrapper.instance().UNSAFE_componentWillReceiveProps(props);
- // Check equal references
- expect(wrapper.instance().state.rows).toBe(currentRows);
+ const alteredLabel = screen.queryByText('Altered');
+ expect(alteredLabel).not.toBeInTheDocument();
});
+});
- it('renders a ModalTrigger', () => {
- expect(wrapper.find(ModalTrigger)).toExist();
+describe('alterForComparison', () => {
+ it('returns null for undefined value', () => {
+ const value = undefined;
+ const result = alterForComparison(value);
+ expect(result).toBeNull();
});
- describe('renderTriggerNode', () => {
- it('renders a Tooltip', () => {
- const triggerNode = mount(
- <div>{wrapper.instance().renderTriggerNode()}</div>,
- );
- expect(triggerNode.find(Tooltip)).toHaveLength(1);
- });
+ it('returns null for null value', () => {
+ const value = null;
+ const result = alterForComparison(value);
+ expect(result).toBeNull();
});
- describe('renderModalBody', () => {
- it('renders a Table', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- expect(modalBody.find(TableView)).toHaveLength(1);
- });
-
- it('renders a thead', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- expect(
- getTableWrapperFromModalBody(modalBody).find('thead'),
- ).toHaveLength(1);
- });
-
- it('renders th', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- const th = getTableWrapperFromModalBody(modalBody).find('th');
- expect(th).toHaveLength(3);
- ['Control', 'Before', 'After'].forEach(async (v, i) => {
- await expect(th.at(i).find('span').get(0).props.children[0]).toBe(v);
- });
- });
-
- it('renders the correct number of Tr', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- const tr = getTableWrapperFromModalBody(modalBody).find('tr');
- expect(tr).toHaveLength(8);
- });
-
- it('renders the correct number of td', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- const td = getTableWrapperFromModalBody(modalBody).find('td');
- expect(td).toHaveLength(21);
- ['control', 'before', 'after'].forEach((v, i) => {
- expect(td.find('defaultRenderer').get(0).props.columns[i].id).toBe(v);
- });
- });
+ it('returns null for empty string value', () => {
+ const value = '';
+ const result = alterForComparison(value);
+ expect(result).toBeNull();
});
- describe('renderRows', () => {
- it('returns an array of rows with one tr and three td', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- const rows = getTableWrapperFromModalBody(modalBody).find('tr');
- expect(rows).toHaveLength(8);
- const slice = mount(
- <table>
- <tbody>{rows.get(1)}</tbody>
- </table>,
- );
- expect(slice.find('tr')).toHaveLength(1);
- expect(slice.find('td')).toHaveLength(3);
- });
+ it('returns null for empty array value', () => {
+ const value = [];
+ const result = alterForComparison(value);
+ expect(result).toBeNull();
});
- describe('formatValue', () => {
- it('returns "N/A" for undefined values', () => {
- expect(wrapper.instance().formatValue(undefined, 'b', controlsMap)).toBe(
- 'N/A',
- );
- });
-
- it('returns "null" for null values', () => {
- expect(wrapper.instance().formatValue(null, 'b', controlsMap)).toBe(
- 'null',
- );
- });
+ it('returns null for empty object value', () => {
+ const value = {};
+ const result = alterForComparison(value);
+ expect(result).toBeNull();
+ });
- it('returns "Max" and "Min" for BoundsControl', () => {
- // need to pass the viz type to the wrapper
- expect(
- wrapper.instance().formatValue([5, 6], 'y_axis_bounds', controlsMap),
- ).toBe('Min: 5, Max: 6');
- });
+ it('returns value for non-empty array', () => {
+ const value = [1, 2, 3];
+ const result = alterForComparison(value);
+ expect(result).toEqual(value);
+ });
- it('returns stringified objects for CollectionControl', () => {
- const value = [
- { 1: 2, alpha: 'bravo' },
- { sent: 'imental', w0ke: 5 },
- ];
- const expected = '{"1":2,"alpha":"bravo"}, {"sent":"imental","w0ke":5}';
- expect(
- wrapper.instance().formatValue(value, 'column_collection', controlsMap),
- ).toBe(expected);
- });
+ it('returns value for non-empty object', () => {
+ const value = { key: 'value' };
+ const result = alterForComparison(value);
+ expect(result).toEqual(value);
+ });
+});
- it('returns boolean values as string', () => {
- expect(wrapper.instance().formatValue(true, 'b', controlsMap)).toBe(
- 'true',
- );
- expect(wrapper.instance().formatValue(false, 'b', controlsMap)).toBe(
- 'false',
- );
- });
+describe('formatValueHandler', () => {
+ const controlsMap = {
+ key1: { type: 'AdhocFilterControl', label: 'Label1' },
+ key2: { type: 'BoundsControl', label: 'Label2' },
+ key3: { type: 'CollectionControl', label: 'Label3' },
+ key4: { type: 'MetricsControl', label: 'Label4' },
+ key5: { type: 'OtherControl', label: 'Label5' },
+ };
+
+ it('formats AdhocFilterControl values correctly', () => {
+ const result = formatValueHandler(
+ defaultProps.origFormData.adhoc_filters,
+ 'key1',
+ controlsMap,
+ );
+ expect(result).toEqual(expectedRows[0].before);
+ });
- it('returns Array joined by commas', () => {
- const value = [5, 6, 7, 8, 'hello', 'goodbye'];
- const expected = '5, 6, 7, 8, hello, goodbye';
- expect(
- wrapper.instance().formatValue(value, undefined, controlsMap),
- ).toBe(expected);
- });
+ it('formats BoundsControl values correctly', () => {
+ const value = [1, 2];
+ const result = formatValueHandler(value, 'key2', controlsMap);
+ expect(result).toEqual('Min: 1, Max: 2');
+ });
- it('returns Metrics if the field type is metrics', () => {
- const value = [
- {
- label: 'SUM(Sales)',
- },
- ];
- const expected = 'SUM(Sales)';
- expect(
- wrapper.instance().formatValue(value, 'metrics', controlsMap),
- ).toBe(expected);
- });
+ it('formats CollectionControl values correctly', () => {
+ const value = [{ a: 1 }, { b: 2 }];
+ const result = formatValueHandler(value, 'key3', controlsMap);
+ expect(result).toEqual(
+ `${JSON.stringify(value[0])}, ${JSON.stringify(value[1])}`,
+ );
+ });
- it('stringifies objects', () => {
- const value = { 1: 2, alpha: 'bravo' };
- const expected = '{"1":2,"alpha":"bravo"}';
- expect(
- wrapper.instance().formatValue(value, undefined, controlsMap),
- ).toBe(expected);
- });
+ it('formats MetricsControl values correctly', () => {
+ const value = [{ label: 'Metric1' }, { label: 'Metric2' }];
+ const result = formatValueHandler(value, 'key4', controlsMap);
+ expect(result).toEqual('Metric1, Metric2');
+ });
- it('does nothing to strings and numbers', () => {
- expect(wrapper.instance().formatValue(5, undefined, controlsMap)).toBe(5);
- expect(
- wrapper.instance().formatValue('hello', undefined, controlsMap),
- ).toBe('hello');
- });
+ it('formats boolean values correctly', () => {
+ const value = true;
+ const result = formatValueHandler(value, 'key5', controlsMap);
+ expect(result).toEqual('true');
+ });
- it('returns "[]" for empty filters', () => {
- expect(
- wrapper.instance().formatValue([], 'adhoc_filters', controlsMap),
- ).toBe('[]');
- });
+ it('formats array values correctly', () => {
+ const value = [{ label: 'Label1' }, { label: 'Label2' }];
+ const result = formatValueHandler(value, 'key5', controlsMap);
+ expect(result).toEqual('Label1, Label2');
+ });
- it('correctly formats filters with array values', () => {
- const filters = [
- {
- clause: 'WHERE',
- comparator: ['1', 'g', '7', 'ho'],
- expressionType: 'SIMPLE',
- operator: 'IN',
- subject: 'a',
- },
- {
- clause: 'WHERE',
- comparator: ['hu', 'ho', 'ha'],
- expressionType: 'SIMPLE',
- operator: 'NOT IN',
- subject: 'b',
- },
- ];
- const expected = 'a IN [1, g, 7, ho], b NOT IN [hu, ho, ha]';
- expect(
- wrapper.instance().formatValue(filters, 'adhoc_filters', controlsMap),
- ).toBe(expected);
- });
+ it('formats string values correctly', () => {
+ const value = 'test';
+ const result = formatValueHandler(value, 'key5', controlsMap);
+ expect(result).toEqual('test');
+ });
- it('correctly formats filters with string values', () => {
- const filters = [
- {
- clause: 'WHERE',
- comparator: 'gucci',
- expressionType: 'SIMPLE',
- operator: '==',
- subject: 'a',
- },
- {
- clause: 'WHERE',
- comparator: 'moshi moshi',
- expressionType: 'SIMPLE',
- operator: 'LIKE',
- subject: 'b',
- },
- ];
- const expected = 'a == gucci, b LIKE moshi moshi';
- expect(
- wrapper.instance().formatValue(filters, 'adhoc_filters', controlsMap),
- ).toBe(expected);
- });
+ it('formats number values correctly', () => {
+ const value = 123;
+ const result = formatValueHandler(value, 'key5', controlsMap);
+ expect(result).toEqual(123);
});
- 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,
- );
- });
+ it('formats other values correctly', () => {
+ const value = { a: 1, b: 2 };
+ const result = formatValueHandler(value, 'key5', controlsMap);
+ expect(result).toEqual(JSON.stringify(value));
});
});
diff --git a/superset-frontend/src/components/AlteredSliceTag/index.jsx b/superset-frontend/src/components/AlteredSliceTag/index.jsx
index a268625393..6dc7d33761 100644
--- a/superset-frontend/src/components/AlteredSliceTag/index.jsx
+++ b/superset-frontend/src/components/AlteredSliceTag/index.jsx
@@ -17,7 +17,7 @@
* under the License.
*/
-import React, { useState, useCallback } from 'react';
+import React, { useState, useCallback, useEffect, useRef } from 'react';
import PropTypes from 'prop-types';
import { isEqual, isEmpty } from 'lodash';
import { styled, t } from '@superset-ui/core';
@@ -45,7 +45,7 @@ const StyledLabel = styled.span`
`}
`;
-function alterForComparison(value) {
+export const alterForComparison = value => {
// Considering `[]`, `{}`, `null` and `undefined` as identical
// for this purpose
if (value === undefined || value === null || value === '') {
@@ -61,74 +61,73 @@ function alterForComparison(value) {
}
}
return value;
-}
+};
-const AlteredSliceTag = props => {
- const formatValueHandler = useCallback((value, key, controlsMap) => {
- // Format display value based on the control type
- // or the value type
- if (value === undefined) {
- return 'N/A';
- }
- if (value === null) {
- return 'null';
- }
- if (controlsMap[key]?.type === 'AdhocFilterControl') {
- if (!value.length) {
- return '[]';
- }
- return value
- .map(v => {
- const filterVal =
- v.comparator && v.comparator.constructor === Array
- ? `[${v.comparator.join(', ')}]`
- : v.comparator;
- return `${v.subject} ${v.operator} ${filterVal}`;
- })
- .join(', ');
- }
- if (controlsMap[key]?.type === 'BoundsControl') {
- return `Min: ${value[0]}, Max: ${value[1]}`;
- }
- if (controlsMap[key]?.type === 'CollectionControl') {
- return value.map(v => safeStringify(v)).join(', ');
- }
- if (
- controlsMap[key]?.type === 'MetricsControl' &&
- value.constructor === Array
- ) {
- const formattedValue = value.map(v => v?.label ?? v);
- return formattedValue.length ? formattedValue.join(', ') : '[]';
- }
- if (typeof value === 'boolean') {
- return value ? 'true' : 'false';
- }
- if (value.constructor === Array) {
- const formattedValue = value.map(v => v?.label ?? v);
- return formattedValue.length ? formattedValue.join(', ') : '[]';
- }
- if (typeof value === 'string' || typeof value === 'number') {
- return value;
+export const formatValueHandler = (value, key, controlsMap) => {
+ // Format display value based on the control type
+ // or the value type
+ if (value === undefined) {
+ return 'N/A';
+ }
+ if (value === null) {
+ return 'null';
+ }
+ if (controlsMap[key]?.type === 'AdhocFilterControl') {
+ if (!value.length) {
+ return '[]';
}
- return safeStringify(value);
- }, []);
+ return value
+ .map(v => {
+ const filterVal =
+ v.comparator && v.comparator.constructor === Array
+ ? `[${v.comparator.join(', ')}]`
+ : v.comparator;
+ return `${v.subject} ${v.operator} ${filterVal}`;
+ })
+ .join(', ');
+ }
+ if (controlsMap[key]?.type === 'BoundsControl') {
+ return `Min: ${value[0]}, Max: ${value[1]}`;
+ }
+ if (controlsMap[key]?.type === 'CollectionControl') {
+ return value.map(v => safeStringify(v)).join(', ');
+ }
+ if (
+ controlsMap[key]?.type === 'MetricsControl' &&
+ value.constructor === Array
+ ) {
+ const formattedValue = value.map(v => v?.label ?? v);
+ return formattedValue.length ? formattedValue.join(', ') : '[]';
+ }
+ if (typeof value === 'boolean') {
+ return value ? 'true' : 'false';
+ }
+ if (value.constructor === Array) {
+ const formattedValue = value.map(v => v?.label ?? v);
+ return formattedValue.length ? formattedValue.join(', ') : '[]';
+ }
+ if (typeof value === 'string' || typeof value === 'number') {
+ return value;
+ }
+ return safeStringify(value);
+};
- const getRowsFromDiffsHandler = useCallback(
- (diffs, controlsMap) =>
- Object.entries(diffs).map(([key, diff]) => ({
- control: (controlsMap[key] && controlsMap[key].label) || key,
- before: formatValueHandler(diff.before, key, controlsMap),
- after: formatValueHandler(diff.after, key, controlsMap),
- })),
- [],
- );
+export const getRowsFromDiffs = (diffs, controlsMap) =>
+ Object.entries(diffs).map(([key, diff]) => ({
+ control: (controlsMap[key] && controlsMap[key].label) || key,
+ before: formatValueHandler(diff.before, key, controlsMap),
+ after: formatValueHandler(diff.after, key, controlsMap),
+ }));
- const isEqualishHandler = useCallback(
- (val1, val2) => isEqual(alterForComparison(val1), alterForComparison(val2)),
- [],
- );
+export const isEqualish = (val1, val2) =>
+ isEqual(alterForComparison(val1), alterForComparison(val2));
- const getDiffsHandler = useCallback(props => {
+const AlteredSliceTag = props => {
+ const prevProps = useRef({});
+ const [rows, setRows] = useState([]);
+ const [hasDiffs, setHasDiffs] = useState(false);
+
+ const getDiffs = useCallback(props => {
// Returns all properties that differ in the
// current form data and the saved form data
const ofd = sanitizeFormData(props.origFormData);
@@ -143,30 +142,36 @@ const AlteredSliceTag = props => {
if (['filters', 'having', 'where'].includes(fdKey)) {
return;
}
- if (!isEqualishHandler(ofd[fdKey], cfd[fdKey])) {
+ if (!isEqualish(ofd[fdKey], cfd[fdKey])) {
diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
}
});
return diffs;
}, []);
- const diffs = getDiffsHandler(props);
- const controlsMap = getControlsForVizType(props.origFormData.viz_type);
- const rows = getRowsFromDiffsHandler(diffs, controlsMap);
+ useEffect(() => {
+ const diffs = getDiffs(props);
+ const controlsMap = getControlsForVizType(props.origFormData?.viz_type);
+ const rows = getRowsFromDiffs(diffs, controlsMap);
+ const hasDiffs = !isEmpty(diffs);
+ setRows(rows);
+ setHasDiffs(hasDiffs);
+ }, []);
+
+ useEffect(() => {
+ const diffs = getDiffs(props);
- const [hasDiffs] = useState(!isEmpty(diffs));
+ const updateStateWithDiffs = newProps => {
+ if (isEqual(prevProps.current, newProps)) {
+ return;
+ }
+ setRows(prevRows => getRowsFromDiffs(diffs, prevRows));
+ setHasDiffs(!isEmpty(diffs));
+ };
- // const UNSAFE_componentWillReceivePropsHandler = useCallback(newProps => {
- // // Update differences if need be
- // if (isEqual(props, newProps)) {
- // return;
- // }
- // const diffs = getDiffsHandler(newProps);
- // setStateHandler(prevState => ({
- // rows: getRowsFromDiffsHandler(diffs, prevState.controlsMap),
- // hasDiffs: !isEmpty(diffs),
- // }));
- // }, []);
+ updateStateWithDiffs(props);
+ prevProps.current = props;
+ }, [getDiffs, props]);
const renderModalBodyHandler = useCallback(() => {
const columns = [
@@ -195,7 +200,8 @@ const AlteredSliceTag = props => {
columnsForWrapText={columnsForWrapText}
/>
);
- }, []);
+ }, [rows]);
+
const renderTriggerNodeHandler = useCallback(
() => (
<Tooltip id="difference-tooltip" title={t('Click to see difference')}>