Skip to content

Commit

Permalink
Resolves issue apache#21731 where date range in explore throws runtim…
Browse files Browse the repository at this point in the history
…e error when explore.common is undefined in Redux state

Changes made in superset/superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx in PR https://github.com/apache/superset/pull/20063/files#diff-ac08ce133d14a8a92607bf02bc368b6b52b8314ddef1b872e42746ba8d6038d0

introduced bug that expects redux state to contain a path that is undefined
  • Loading branch information
eric-briscoe committed Oct 11, 2022
1 parent bd3166b commit aaaa34b
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ const store = mockStore({
common: { locale: 'en' },
});

// case when common.locale is not populated
const mockEmptyStore = configureStore([thunk]);
const emptyStore = mockEmptyStore({});

// case when common.locale is populated with invalid locale
const mockInvalidStore = configureStore([thunk]);
const invalidStore = mockInvalidStore({ common: { locale: 'invalid_locale' } });

test('renders with default props', () => {
render(
<Provider store={store}>
Expand All @@ -53,6 +61,54 @@ test('renders with default props', () => {
expect(screen.getByRole('img', { name: 'calendar' })).toBeInTheDocument();
});

test('renders with empty store', () => {
render(
<Provider store={emptyStore}>
<CustomFrame onChange={jest.fn()} value={emptyValue} />
</Provider>,
);
expect(screen.getByText('Configure custom time range')).toBeInTheDocument();
expect(screen.getByText('Relative Date/Time')).toBeInTheDocument();
expect(screen.getByRole('spinbutton')).toBeInTheDocument();
expect(screen.getByText('Days Before')).toBeInTheDocument();
expect(screen.getByText('Specific Date/Time')).toBeInTheDocument();
expect(screen.getByRole('img', { name: 'calendar' })).toBeInTheDocument();
});

test('renders since and until with specific date/time with default locale', () => {
render(
<Provider store={emptyStore}>
<CustomFrame onChange={jest.fn()} value={specificValue} />
</Provider>,
);
expect(screen.getAllByText('Specific Date/Time').length).toBe(2);
expect(screen.getAllByRole('img', { name: 'calendar' }).length).toBe(2);
});

test('renders with invalid locale', () => {
render(
<Provider store={invalidStore}>
<CustomFrame onChange={jest.fn()} value={emptyValue} />
</Provider>,
);
expect(screen.getByText('Configure custom time range')).toBeInTheDocument();
expect(screen.getByText('Relative Date/Time')).toBeInTheDocument();
expect(screen.getByRole('spinbutton')).toBeInTheDocument();
expect(screen.getByText('Days Before')).toBeInTheDocument();
expect(screen.getByText('Specific Date/Time')).toBeInTheDocument();
expect(screen.getByRole('img', { name: 'calendar' })).toBeInTheDocument();
});

test('renders since and until with specific date/time with invalid locale', () => {
render(
<Provider store={invalidStore}>
<CustomFrame onChange={jest.fn()} value={specificValue} />
</Provider>,
);
expect(screen.getAllByText('Specific Date/Time').length).toBe(2);
expect(screen.getAllByRole('img', { name: 'calendar' }).length).toBe(2);
});

test('renders since and until with specific date/time', () => {
render(
<Provider store={store}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,15 @@ export function CustomFrame(props: FrameComponentProps) {
}
}

const localFromFlaskBabel =
useSelector((state: ExplorePageState) => state.common.locale) || 'en';
const currentLocale = locales[LOCALE_MAPPING[localFromFlaskBabel]].DatePicker;
// check if there is a locale defined for explore
const localFromFlaskBabel = useSelector(
(state: ExplorePageState) => state?.common?.locale,
);
// An undefined datePickerLocale is acceptable if no match is found in the LOCALE_MAPPING[localFromFlaskBabel] lookup
// and will fall back to antd's default locale when the antd DataPicker's prop locale === undefined
// This also protects us from the case where state is populated with a locale that antd locales does not recognize
const datePickerLocale =
locales[LOCALE_MAPPING[localFromFlaskBabel]]?.DatePicker;

return (
<div data-test="custom-frame">
Expand Down Expand Up @@ -141,7 +147,7 @@ export function CustomFrame(props: FrameComponentProps) {
onChange('sinceDatetime', datetime.format(MOMENT_FORMAT))
}
allowClear={false}
locale={currentLocale}
locale={datePickerLocale}
/>
</Row>
)}
Expand Down Expand Up @@ -194,7 +200,7 @@ export function CustomFrame(props: FrameComponentProps) {
onChange('untilDatetime', datetime.format(MOMENT_FORMAT))
}
allowClear={false}
locale={currentLocale}
locale={datePickerLocale}
/>
</Row>
)}
Expand Down Expand Up @@ -252,7 +258,7 @@ export function CustomFrame(props: FrameComponentProps) {
}
allowClear={false}
className="control-anchor-to-datetime"
locale={currentLocale}
locale={datePickerLocale}
/>
</Col>
)}
Expand Down

0 comments on commit aaaa34b

Please sign in to comment.