From 8c7a3bf85a2b535e03030ad0d195f65a25a75843 Mon Sep 17 00:00:00 2001 From: Antonio Rivero <38889534+Antonio-RiveroMartnez@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:26:11 +0100 Subject: [PATCH] fix(time_comparison): Allow deleting dates when using custom shift (#30848) --- .../src/constants.ts | 3 + .../src/sections/timeComparison.tsx | 28 ++++++- .../controls/TimeOffsetControl.test.tsx | 80 +++++++++++++++++++ .../components/controls/TimeOffsetControl.tsx | 23 ++++-- 4 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 superset-frontend/src/explore/components/controls/TimeOffsetControl.test.tsx diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts index d31843690..6534258c6 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts @@ -80,3 +80,6 @@ export const DEFAULT_XAXIS_SORT_SERIES_DATA: SortSeriesData = { }; export const DEFAULT_DATE_PATTERN = /\d{4}-\d{2}-\d{2}/g; + +// When moment fails to parse a date +export const INVALID_DATE = 'Invalid date'; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx index 17239de87..901c34abc 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx @@ -18,7 +18,12 @@ */ import { t, ComparisonType } from '@superset-ui/core'; -import { ControlPanelSectionConfig } from '../types'; +import { + ControlPanelSectionConfig, + ControlPanelState, + ControlState, +} from '../types'; +import { INVALID_DATE } from '..'; const fullChoices = [ ['1 day ago', t('1 day ago')], @@ -94,9 +99,28 @@ export const timeComparisonControls: ({ name: 'start_date_offset', config: { type: 'TimeOffsetControl', - label: t('shift start date'), + label: t('Shift start date'), visibility: ({ controls }) => controls?.time_compare.value === 'custom', + mapStateToProps: ( + state: ControlPanelState, + controlState: ControlState, + ) => { + const { form_data } = state; + const { time_compare } = form_data; + const newState = { ...controlState }; + if ( + time_compare === 'custom' && + (controlState.value === '' || controlState.value === INVALID_DATE) + ) { + newState.externalValidationErrors = [ + t('A date is required when using custom date shift'), + ]; + } else { + newState.externalValidationErrors = []; + } + return newState; + }, }, }, ], diff --git a/superset-frontend/src/explore/components/controls/TimeOffsetControl.test.tsx b/superset-frontend/src/explore/components/controls/TimeOffsetControl.test.tsx new file mode 100644 index 000000000..6745e34cf --- /dev/null +++ b/superset-frontend/src/explore/components/controls/TimeOffsetControl.test.tsx @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import configureStore from 'redux-mock-store'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { Provider } from 'react-redux'; +import { ThemeProvider, supersetTheme } from '@superset-ui/core'; +import moment from 'moment'; +import { INVALID_DATE } from '@superset-ui/chart-controls'; +import TimeOffsetControls, { + TimeOffsetControlsProps, +} from './TimeOffsetControl'; + +const mockStore = configureStore([]); + +const defaultProps: TimeOffsetControlsProps = { + onChange: jest.fn(), +}; + +describe('TimeOffsetControls', () => { + const setup = (initialState = {}) => { + const store = mockStore({ + explore: { + form_data: { + adhoc_filters: [ + { + operator: 'TEMPORAL_RANGE', + subject: 'date', + comparator: '2023-01-01 : 2023-12-31', + }, + ], + start_date_offset: '2023-01-01', + ...initialState, + }, + }, + }); + + const props = { ...defaultProps }; + + render( + + + + + , + ); + + return { store, props }; + }; + + it('TimeOffsetControl renders DatePicker when startDate is set', () => { + setup(); + const datePickerInput = screen.getByRole('textbox'); + expect(datePickerInput).toBeInTheDocument(); + expect(datePickerInput).toHaveValue('2023-01-01'); + }); + + // Our Time comparison control depends on this string for supporting date deletion on date picker + // That's why this test is linked to the TimeOffsetControl component + it('Moment should return "Invalid date" when parsing an invalid date string', () => { + const invalidDate = moment('not-a-date'); + expect(invalidDate.format()).toBe(INVALID_DATE); + }); +}); diff --git a/superset-frontend/src/explore/components/controls/TimeOffsetControl.tsx b/superset-frontend/src/explore/components/controls/TimeOffsetControl.tsx index e58a87497..d2959483a 100644 --- a/superset-frontend/src/explore/components/controls/TimeOffsetControl.tsx +++ b/superset-frontend/src/explore/components/controls/TimeOffsetControl.tsx @@ -34,7 +34,10 @@ import { useSelector } from 'react-redux'; import ControlHeader from 'src/explore/components/ControlHeader'; import { RootState } from 'src/views/store'; -import { DEFAULT_DATE_PATTERN } from '@superset-ui/chart-controls'; +import { + DEFAULT_DATE_PATTERN, + INVALID_DATE, +} from '@superset-ui/chart-controls'; export interface TimeOffsetControlsProps { label?: ReactNode; @@ -68,6 +71,7 @@ export default function TimeOffsetControls({ moment.Moment | undefined >(undefined); const [savedStartDate, setSavedStartDate] = useState(null); + const [isDateSelected, setIsDateSelected] = useState(true); const currentTimeRangeFilters = useSelector( state => @@ -86,7 +90,12 @@ export default function TimeOffsetControls({ useEffect(() => { if (savedStartDate !== currentStartDate) { setSavedStartDate(currentStartDate); - onChange(moment(currentStartDate).format(MOMENT_FORMAT)); + if (currentStartDate !== INVALID_DATE) { + onChange(moment(currentStartDate).format(MOMENT_FORMAT)); + setIsDateSelected(true); + } else { + setIsDateSelected(false); + } } }, [currentStartDate]); @@ -165,8 +174,10 @@ export default function TimeOffsetControls({ setFormatedDate(moment(parseDttmToDate(date))); } } else if (savedStartDate) { - setStartDate(savedStartDate); - setFormatedDate(moment(parseDttmToDate(savedStartDate))); + if (savedStartDate !== INVALID_DATE) { + setStartDate(savedStartDate); + setFormatedDate(moment(parseDttmToDate(savedStartDate))); + } } }, [previousCustomFilter, savedStartDate, customStartDateInFilter]); @@ -180,6 +191,7 @@ export default function TimeOffsetControls({ setStartDate(resetDate.toString()); setFormatedDate(resetDate); onChange(moment.utc(resetDate).format(MOMENT_FORMAT)); + setIsDateSelected(true); } } if ( @@ -191,6 +203,7 @@ export default function TimeOffsetControls({ setStartDate(resetDate.toString()); setFormatedDate(resetDate); onChange(moment.utc(resetDate).format(MOMENT_FORMAT)); + setIsDateSelected(true); } }, [formatedFilterDate, formatedDate, customStartDateInFilter]); @@ -218,7 +231,7 @@ export default function TimeOffsetControls({ } disabledDate={disabledDate} defaultValue={moment(formatedDate)} - value={moment(formatedDate)} + value={isDateSelected ? moment(formatedDate) : null} /> ) : null;