Skip to content

Commit

Permalink
Erase useEffect, add tests, fix types
Browse files Browse the repository at this point in the history
  • Loading branch information
@rtexelm
rtexelm committed Apr 23, 2024
1 parent fabc086 commit eae1e1d
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 65 deletions.
167 changes: 134 additions & 33 deletions superset-frontend/src/components/AlteredSliceTag/AlteredSliceTag.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,16 @@ import userEvent from '@testing-library/user-event';
import AlteredSliceTag, {
alterForComparison,
formatValueHandler,
isEqualish,
} from 'src/components/AlteredSliceTag';
import { defaultProps, expectedRows } from './AlteredSliceTagMocks';
import { defaultProps } 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' },
b: { type: 'BoundsControl', label: 'Bounds' },
column_collection: { type: 'CollectionControl', label: 'Collection' },
metrics: { type: 'MetricsControl', label: 'Metrics' },
adhoc_filters: { type: 'AdhocFilterControl', label: 'Adhoc' },
other_control: { type: 'OtherControl', label: 'Other' },
};

test('renders the "Altered" label', () => {
Expand Down Expand Up @@ -120,61 +121,161 @@ test('alterForComparison returns value for non-empty object', () => {
expect(alterForComparison(value)).toEqual(value);
});

test('formatValueHandler formats AdhocFilterControl values correctly', () => {
const result = formatValueHandler(
defaultProps.origFormData.adhoc_filters,
'key1',
controlsMap,
);
expect(result).toEqual(expectedRows[0].before);
test('formatValueHandler handles undefined value', () => {
const value = undefined;
const key = 'b';
const formattedValue = formatValueHandler(value, key, controlsMap);
expect(formattedValue).toBe('N/A');
});

test('formatValueHandler handles null value', () => {
const value = null;
const key = 'b';
const formattedValue = formatValueHandler(value, key, controlsMap);
expect(formattedValue).toBe('null');
});

test('formatValueHandler returns "[]" for empty filters', () => {
const value = [];
const key = 'adhoc_filters';
const formattedValue = formatValueHandler(value, key, controlsMap);
expect(formattedValue).toBe('[]');
});

test('formatValueHandler 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 key = 'adhoc_filters';
const formattedValue = formatValueHandler(filters, key, controlsMap);
const expected = 'a IN [1, g, 7, ho], b NOT IN [hu, ho, ha]';
expect(formattedValue).toBe(expected);
});

test('formatValueHandler formats BoundsControl values correctly', () => {
test('formatValueHandler 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 key = 'adhoc_filters';
const expected = 'a == gucci, b LIKE moshi moshi';
const formattedValue = formatValueHandler(filters, key, controlsMap);
expect(formattedValue).toBe(expected);
});

test('formatValueHandler formats "Min" and "Max" for BoundsControl', () => {
const value = [1, 2];
const result = formatValueHandler(value, 'key2', controlsMap);
const key = 'b';
const result = formatValueHandler(value, key, controlsMap);
expect(result).toEqual('Min: 1, Max: 2');
});

test('formatValueHandler formats CollectionControl values correctly', () => {
test('formatValueHandler formats stringified objects for CollectionControl', () => {
const value = [{ a: 1 }, { b: 2 }];
const result = formatValueHandler(value, 'key3', controlsMap);
const key = 'column_collection';
const result = formatValueHandler(value, key, controlsMap);
expect(result).toEqual(
`${JSON.stringify(value[0])}, ${JSON.stringify(value[1])}`,
);
});

test('formatValueHandler formats MetricsControl values correctly', () => {
const value = [{ label: 'Metric1' }, { label: 'Metric2' }];
const result = formatValueHandler(value, 'key4', controlsMap);
expect(result).toEqual('Metric1, Metric2');
const value = [{ label: 'SUM(Sales)' }, { label: 'Metric2' }];
const key = 'metrics';
const result = formatValueHandler(value, key, controlsMap);
expect(result).toEqual('SUM(Sales), Metric2');
});

test('formatValueHandler formats boolean values correctly', () => {
const value = true;
const result = formatValueHandler(value, 'key5', controlsMap);
expect(result).toEqual('true');
test('formatValueHandler formats boolean values as string', () => {
const value1 = true;
const value2 = false;
const key = 'b';
const formattedValue1 = formatValueHandler(value1, key, controlsMap);
const formattedValue2 = formatValueHandler(value2, key, controlsMap);
expect(formattedValue1).toBe('true');
expect(formattedValue2).toBe('false');
});

test('formatValueHandler formats array values correctly', () => {
const value = [{ label: 'Label1' }, { label: 'Label2' }];
const result = formatValueHandler(value, 'key5', controlsMap);
expect(result).toEqual('Label1, Label2');
const value = [
{ label: 'Label1' },
{ label: 'Label2' },
5,
6,
7,
8,
'hello',
'goodbye',
];
const result = formatValueHandler(value, undefined, controlsMap);
const expected = 'Label1, Label2, 5, 6, 7, 8, hello, goodbye';
expect(result).toEqual(expected);
});

test('formatValueHandler formats string values correctly', () => {
const value = 'test';
const result = formatValueHandler(value, 'key5', controlsMap);
const key = 'other_control';
const result = formatValueHandler(value, key, controlsMap);
expect(result).toEqual('test');
});

test('formatValueHandler formats number values correctly', () => {
const value = 123;
const result = formatValueHandler(value, 'key5', controlsMap);
const key = 'other_control';
const result = formatValueHandler(value, key, controlsMap);
expect(result).toEqual(123);
});

test('formatValueHandler formats other values correctly', () => {
const value = { a: 1, b: 2 };
const result = formatValueHandler(value, 'key5', controlsMap);
expect(result).toEqual(JSON.stringify(value));
test('formatValueHandler formats object values correctly', () => {
const value = { 1: 2, alpha: 'bravo' };
const key = 'other_control';
const expected = '{"1":2,"alpha":"bravo"}';
const result = formatValueHandler(value, key, controlsMap);
expect(result).toEqual(expected);
});

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);
});

test('isEqualish considers empty strings equal to null', () => {
expect(isEqualish(undefined, '')).toBe(true);
expect(isEqualish(null, '')).toBe(true);
});

test('isEqualish considers deeply equal objects equal', () => {
const obj1 = { a: { b: { c: 1 } } };
const obj2 = { a: { b: { c: 1 } } };
expect(isEqualish('', '')).toBe(true);
expect(isEqualish(obj1, obj2)).toBe(true);
// Actually not equal
expect(isEqualish({ a: 1, b: 2, z: 9 }, { a: 1, b: 2, c: 3 })).toBe(false);
});
47 changes: 15 additions & 32 deletions superset-frontend/src/components/AlteredSliceTag/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useCallback, useEffect, useState } from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { isEqual, isEmpty } from 'lodash';
import { QueryFormData, styled, t, usePrevious } from '@superset-ui/core';
import { QueryFormData, styled, t } from '@superset-ui/core';
import { sanitizeFormData } from 'src/explore/exploreUtils/formData';
import getControlsForVizType from 'src/utils/getControlsForVizType';
import { safeStringify } from 'src/utils/safeStringify';
Expand Down Expand Up @@ -97,7 +97,7 @@ export const alterForComparison = (
};

export const formatValueHandler = (
value: any,
value: DiffItemType,
key: string,
controlsMap: ControlMap,
): string | number => {
Expand All @@ -107,12 +107,15 @@ export const formatValueHandler = (
if (value === null) {
return 'null';
}
if (typeof value === 'boolean') {
return value ? 'true' : 'false';
}
if (controlsMap[key]?.type === 'AdhocFilterControl' && Array.isArray(value)) {
if (!value.length) {
return '[]';
}
return value
.map((v: any) => {
.map((v: FilterItemType) => {
const filterVal =
v.comparator && v.comparator.constructor === Array
? `[${v.comparator.join(', ')}]`
Expand All @@ -125,20 +128,17 @@ export const formatValueHandler = (
return `Min: ${value[0]}, Max: ${value[1]}`;
}
if (controlsMap[key]?.type === 'CollectionControl' && Array.isArray(value)) {
return value.map((v: any) => safeStringify(v)).join(', ');
return value.map((v: FilterItemType) => safeStringify(v)).join(', ');
}
if (
controlsMap[key]?.type === 'MetricsControl' &&
value.constructor === Array
) {
const formattedValue = value.map((v: any) => v?.label ?? v);
const formattedValue = value.map((v: FilterItemType) => v?.label ?? v);
return formattedValue.length ? formattedValue.join(', ') : '[]';
}
if (typeof value === 'boolean') {
return value ? 'true' : 'false';
}
if (Array.isArray(value)) {
const formattedValue = value.map((v: any) => v?.label ?? v);
const formattedValue = value.map((v: FilterItemType) => v?.label ?? v);
return formattedValue.length ? formattedValue.join(', ') : '[]';
}
if (typeof value === 'string' || typeof value === 'number') {
Expand All @@ -161,7 +161,6 @@ export const isEqualish = (val1: string, val2: string): boolean =>
isEqual(alterForComparison(val1), alterForComparison(val2));

const AlteredSliceTag: React.FC<AlteredSliceTagProps> = props => {
const prevProps = usePrevious(props);
const [rows, setRows] = useState<RowType[]>([]);
const [hasDiffs, setHasDiffs] = useState<boolean>(false);

Expand Down Expand Up @@ -194,25 +193,9 @@ const AlteredSliceTag: React.FC<AlteredSliceTagProps> = props => {
) as ControlMap;
setRows(getRowsFromDiffs(diffs, controlsMap));
setHasDiffs(!isEmpty(diffs));
}, [getDiffs, props]);

useEffect(() => {
const diffs = getDiffs();

const updateStateWithDiffs = () => {
if (isEqual(prevProps, props)) {
return;
}
setRows(prevRows =>
getRowsFromDiffs(diffs, prevRows as unknown as ControlMap),
);
setHasDiffs(!isEmpty(diffs));
};

updateStateWithDiffs();
}, [getDiffs, props, prevProps]);
}, [getDiffs, props.origFormData?.viz_type]);

const renderModalBody = useCallback(() => {
const modalBody = useMemo(() => {
const columns = [
{
accessor: 'control',
Expand Down Expand Up @@ -241,7 +224,7 @@ const AlteredSliceTag: React.FC<AlteredSliceTagProps> = props => {
);
}, [rows]);

const renderTriggerNode = useCallback(
const triggerNode = useMemo(
() => (
<Tooltip id="difference-tooltip" title={t('Click to see difference')}>
<StyledLabel className="label">{t('Altered')}</StyledLabel>
Expand All @@ -256,9 +239,9 @@ const AlteredSliceTag: React.FC<AlteredSliceTagProps> = props => {

return (
<ModalTrigger
triggerNode={renderTriggerNode()}
triggerNode={triggerNode}
modalTitle={t('Chart changes')}
modalBody={renderModalBody()}
modalBody={modalBody}
responsive
/>
);
Expand Down

0 comments on commit eae1e1d

Please sign in to comment.