rtexelm commented on code in PR #27891:
URL: https://github.com/apache/superset/pull/27891#discussion_r1575562809
##########
superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx:
##########
@@ -17,325 +17,164 @@
* under the License.
*/
import React from 'react';
-import { styledMount as mount } from 'spec/helpers/theming';
-import { getChartControlPanelRegistry } from '@superset-ui/core';
-
-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';
-
-import {
- defaultProps,
- expectedDiffs,
- expectedRows,
- fakePluginControls,
-} from './AlteredSliceTagMocks';
-
-const getTableWrapperFromModalBody = modalBody =>
- modalBody.find(TableView).find(TableCollection);
-
-describe('AlteredSliceTag', () => {
- let wrapper;
- let props;
- let controlsMap;
-
- beforeEach(() => {
- getChartControlPanelRegistry().registerValue(
- 'altered_slice_tag_spec',
- fakePluginControls,
- );
- props = { ...defaultProps };
- wrapper = mount(<AlteredSliceTag {...props} />);
- ({ controlsMap } = wrapper.instance().state);
- });
-
- 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('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();
- });
-
- 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();
- });
-
- 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);
- });
+import '@testing-library/jest-dom/extend-expect';
+import { render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+import AlteredSliceTag, {
+ alterForComparison,
+ formatValueHandler,
+} from 'src/components/AlteredSliceTag';
+import { defaultProps, expectedRows } from './AlteredSliceTagMocks';
+
+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' },
+};
+
+test('renders the "Altered" label', () => {
+ render(
+ <AlteredSliceTag
+ origFormData={defaultProps.origFormData}
+ currentFormData={defaultProps.currentFormData}
+ />,
+ );
+
+ const alteredLabel = screen.getByText('Altered');
+ expect(alteredLabel).toBeInTheDocument();
+});
- 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);
- });
+test('opens the modal on click', () => {
+ render(
+ <AlteredSliceTag
+ origFormData={defaultProps.origFormData}
+ currentFormData={defaultProps.currentFormData}
+ />,
+ );
- it('renders a ModalTrigger', () => {
- expect(wrapper.find(ModalTrigger)).toExist();
- });
+ const alteredLabel = screen.getByText('Altered');
+ userEvent.click(alteredLabel);
- describe('renderTriggerNode', () => {
- it('renders a Tooltip', () => {
- const triggerNode = mount(
- <div>{wrapper.instance().renderTriggerNode()}</div>,
- );
- expect(triggerNode.find(Tooltip)).toHaveLength(1);
- });
- });
+ const modalTitle = screen.getByText('Chart changes');
+ expect(modalTitle).toBeInTheDocument();
+});
- describe('renderModalBody', () => {
- it('renders a Table', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- expect(modalBody.find(TableView)).toHaveLength(1);
- });
+test('displays the differences in the modal', () => {
+ render(
+ <AlteredSliceTag
+ origFormData={defaultProps.origFormData}
+ currentFormData={defaultProps.currentFormData}
+ />,
+ );
+
+ const alteredLabel = screen.getByText('Altered');
+ userEvent.click(alteredLabel);
+
+ const beforeValue = screen.getByText('1, 2, 3, 4');
+ const afterValue = screen.getByText('a, b, c, d');
+ expect(beforeValue).toBeInTheDocument();
+ expect(afterValue).toBeInTheDocument();
+});
- it('renders a thead', () => {
- const modalBody = mount(
- <div>{wrapper.instance().renderModalBody()}</div>,
- );
- expect(
- getTableWrapperFromModalBody(modalBody).find('thead'),
- ).toHaveLength(1);
- });
+test('does not render anything if there are no differences', () => {
+ render(
+ <AlteredSliceTag
+ origFormData={defaultProps.origFormData}
+ currentFormData={defaultProps.origFormData}
+ />,
+ );
- 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);
- });
- });
+ const alteredLabel = screen.queryByText('Altered');
+ expect(alteredLabel).not.toBeInTheDocument();
+});
- 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);
- });
+test('alterForComparison returns null for undefined value', () => {
+ expect(alterForComparison(undefined)).toBeNull();
+});
- 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);
- });
- });
- });
+test('alterForComparison returns null for null value', () => {
+ expect(alterForComparison(null)).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);
- });
- });
+test('alterForComparison returns null for empty string value', () => {
+ expect(alterForComparison('')).toBeNull();
+});
- describe('formatValue', () => {
- it('returns "N/A" for undefined values', () => {
- expect(wrapper.instance().formatValue(undefined, 'b', controlsMap)).toBe(
- 'N/A',
- );
- });
+test('alterForComparison returns null for empty array value', () => {
+ expect(alterForComparison([])).toBeNull();
+});
- it('returns "null" for null values', () => {
- expect(wrapper.instance().formatValue(null, 'b', controlsMap)).toBe(
- 'null',
- );
- });
+test('alterForComparison returns null for empty object value', () => {
+ expect(alterForComparison({})).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');
- });
+test('alterForComparison returns value for non-empty array', () => {
+ const value = [1, 2, 3];
+ expect(alterForComparison(value)).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);
- });
+test('alterForComparison returns value for non-empty object', () => {
+ const value = { key: 'value' };
+ expect(alterForComparison(value)).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',
- );
- });
+test('formatValueHandler 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);
- });
+test('formatValueHandler 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);
- });
+test('formatValueHandler 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);
- });
+test('formatValueHandler 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');
- });
+test('formatValueHandler 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('[]');
- });
+test('formatValueHandler 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);
- });
+test('formatValueHandler formats string values correctly', () => {
+ const value = 'test';
+ const result = formatValueHandler(value, 'key5', controlsMap);
+ expect(result).toEqual('test');
+});
- it('correctly formats filters with string values', () => {
Review Comment:
This was actually being handled but only for string filters by the
AdhocFilter test. It was pretty abstract so I made it more explicit in the
revision
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
