[superset] branch master updated: fix: regression on Select component when handling null values (#19326)

[michaelsmolina]

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 9e58916  fix: regression on Select component when handling null values 
(#19326)
9e58916 is described below

commit 9e58916d935cf15c3fbe1949dd81f7acec2514c3
Author: Diego Medina <diegomedin...@gmail.com>
AuthorDate: Wed Mar 23 09:31:38 2022 -0400

    fix: regression on Select component when handling null values (#19326)
---
 .../src/components/Select/Select.test.tsx          | 28 ++++++++++++++++++++++
 superset-frontend/src/components/Select/utils.ts   | 12 ++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/src/components/Select/Select.test.tsx 
b/superset-frontend/src/components/Select/Select.test.tsx
index b247950..37a3920 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -50,6 +50,10 @@ const OPTIONS = [
   { label: 'Cher', value: 22, gender: 'Female' },
   { label: 'Her', value: 23, gender: 'Male' },
 ].sort((option1, option2) => option1.label.localeCompare(option2.label));
+const NULL_OPTION = { label: '<NULL>', value: null } as unknown as {
+  label: string;
+  value: number;
+};
 
 const loadOptions = async (search: string, page: number, pageSize: number) => {
   const totalCount = OPTIONS.length;
@@ -384,6 +388,30 @@ test('does not add a new option if allowNewOptions is 
false', async () => {
   expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
 });
 
+test('adds the null option when selected in single mode', async () => {
+  render(<Select {...defaultProps} options={[OPTIONS[0], NULL_OPTION]} />);
+  await open();
+  userEvent.click(await findSelectOption(NULL_OPTION.label));
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(NULL_OPTION.label);
+});
+
+test('adds the null option when selected in multiple mode', async () => {
+  render(
+    <Select
+      {...defaultProps}
+      options={[OPTIONS[0], NULL_OPTION, OPTIONS[2]]}
+      mode="multiple"
+    />,
+  );
+  await open();
+  userEvent.click(await findSelectOption(OPTIONS[0].label));
+  userEvent.click(await findSelectOption(NULL_OPTION.label));
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(OPTIONS[0].label);
+  expect(values[1]).toHaveTextContent(NULL_OPTION.label);
+});
+
 test('static - renders the select with default props', () => {
   render(<Select {...defaultProps} />);
   expect(getSelect()).toBeInTheDocument();
diff --git a/superset-frontend/src/components/Select/utils.ts 
b/superset-frontend/src/components/Select/utils.ts
index f3880f5..4eda84a 100644
--- a/superset-frontend/src/components/Select/utils.ts
+++ b/superset-frontend/src/components/Select/utils.ts
@@ -25,6 +25,14 @@ import {
   GroupedOptionsType,
 } from 'react-select';
 
+function isObject(value: unknown): value is Record<string, unknown> {
+  return (
+    value !== null &&
+    typeof value === 'object' &&
+    Array.isArray(value) === false
+  );
+}
+
 /**
  * Find Option value that matches a possibly string value.
  *
@@ -63,7 +71,7 @@ export function findValue<OptionType extends OptionTypeBase>(
 export function getValue(
   option: string | number | { value: string | number | null } | null,
 ) {
-  return option && typeof option === 'object' ? option.value : option;
+  return isObject(option) ? option.value : option;
 }
 
 type LabeledValue<V> = { label?: ReactNode; value?: V };
@@ -78,7 +86,7 @@ export function hasOption<V>(
     optionsArray.find(
       x =>
         x === value ||
-        (typeof x === 'object' &&
+        (isObject(x) &&
           (('value' in x && x.value === value) ||
             (checkLabel && 'label' in x && x.label === value))),
     ) !== undefined