fix(time_comparison): Allow deleting dates when using custom shift (#30848)

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';

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;
+          },
         },
       },
     ],

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(
+      <Provider store={store}>
+        <ThemeProvider theme={supersetTheme}>
+          <TimeOffsetControls {...props} />
+        </ThemeProvider>
+      </Provider>,
+    );
+
+    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);
+  });
+});

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<string | null>(null);
+  const [isDateSelected, setIsDateSelected] = useState<boolean>(true);
 
   const currentTimeRangeFilters = useSelector<RootState, BinaryAdhocFilter[]>(
     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}
       />
     </div>
   ) : null;