Skip to content

Commit

Permalink
fix: Duplicated numeric values in Select (#21480)
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina authored Sep 16, 2022
1 parent 7c3fc67 commit b739e27
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 5 deletions.
20 changes: 20 additions & 0 deletions superset-frontend/src/components/Select/AsyncSelect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ const findSelectOption = (text: string) =>
within(getElementByClassName('.rc-virtual-list')).getByText(text),
);

const querySelectOption = (text: string) =>
waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).queryByText(text),
);

const findAllSelectOptions = () =>
waitFor(() => getElementsByClassName('.ant-select-item-option-content'));

Expand Down Expand Up @@ -736,6 +741,21 @@ test('renders a helper text when one is provided', async () => {
expect(screen.queryByText(helperText)).toBeInTheDocument();
});

test('finds an element with a numeric value and does not duplicate the options', async () => {
const options = jest.fn(async () => ({
data: [
{ label: 'a', value: 11 },
{ label: 'b', value: 12 },
],
totalCount: 2,
}));
render(<AsyncSelect {...defaultProps} options={options} allowNewOptions />);
await open();
await type('11');
expect(await findSelectOption('a')).toBeInTheDocument();
expect(await querySelectOption('11')).not.toBeInTheDocument();
});

/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
Expand Down
17 changes: 17 additions & 0 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ const findSelectOption = (text: string) =>
within(getElementByClassName('.rc-virtual-list')).getByText(text),
);

const querySelectOption = (text: string) =>
waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).queryByText(text),
);

const findAllSelectOptions = () =>
waitFor(() => getElementsByClassName('.ant-select-item-option-content'));

Expand Down Expand Up @@ -549,6 +554,18 @@ test('renders a helper text when one is provided', async () => {
expect(screen.queryByText(helperText)).toBeInTheDocument();
});

test('finds an element with a numeric value and does not duplicate the options', async () => {
const options = [
{ label: 'a', value: 11 },
{ label: 'b', value: 12 },
];
render(<Select {...defaultProps} options={options} allowNewOptions />);
await open();
await type('11');
expect(await findSelectOption('a')).toBeInTheDocument();
expect(await querySelectOption('11')).not.toBeInTheDocument();
});

/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
Expand Down
16 changes: 11 additions & 5 deletions superset-frontend/src/components/Select/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,26 @@ export function getValue(
return isLabeledValue(option) ? option.value : option;
}

type LabeledValue<V> = { label?: ReactNode; value?: V };
type V = string | number | null | undefined;

export function hasOption<V>(
type LabeledValue = { label?: ReactNode; value?: V };

export function hasOption(
value: V,
options?: V | LabeledValue<V> | (V | LabeledValue<V>)[],
options?: V | LabeledValue | (V | LabeledValue)[],
checkLabel = false,
): boolean {
const optionsArray = ensureIsArray(options);
// When comparing the values we use the equality
// operator to automatically convert different types
return (
optionsArray.find(
x =>
x === value ||
// eslint-disable-next-line eqeqeq
x == value ||
(isObject(x) &&
(('value' in x && x.value === value) ||
// eslint-disable-next-line eqeqeq
(('value' in x && x.value == value) ||
(checkLabel && 'label' in x && x.label === value))),
) !== undefined
);
Expand Down

0 comments on commit b739e27

Please sign in to comment.