Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(CustomFrame): Resolves issue #21731 where date range in explore throws runtime error #21776

Merged
merged 2 commits into from
Oct 13, 2022
Merged

fix(CustomFrame): Resolves issue #21731 where date range in explore throws runtime error #21776

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aaaa34b
Resolves issue #21731 where date range in explore throws runtime erro…
eric-briscoe Oct 11, 2022
1fb3f67
Consolidates use of configureStore in test
eric-briscoe Oct 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions ...rontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.test.tsx
View file
Open in desktop
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' } });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the return object of configureStore but can you create two separate distinct stores with the same return of configureStore? In other words, can we create empty and invalid both from the mockEmptyStore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! I was able to consolidate one call to configureStore, and reuse calls to mockStore with the different values.


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
18 changes: 12 additions & 6 deletions ...set-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx
View file
Open in desktop
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