fix(explore): deprecated x periods pattern in new time picker value (#…

…12552)

superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx

@@ -197,16 +197,15 @@ export default function DateFilterControl(props: DateFilterLabelProps) {
           +--------------+------+----------+--------+----------+-----------+
           |              | Last | Previous | Custom | Advanced | No Filter |
           +--------------+------+----------+--------+----------+-----------+
-          | control pill | HRT  | HRT      | ADR    | ADR      |   ADR     |
+          | control pill | HRT  | HRT      | ADR    | ADR      |   HRT     |
           +--------------+------+----------+--------+----------+-----------+
-          | tooltip      | ADR  | ADR      | HRT    | HRT      |   HRT     |
+          | tooltip      | ADR  | ADR      | HRT    | HRT      |   ADR     |
           +--------------+------+----------+--------+----------+-----------+
         */
-        const valueToLower = value.toLowerCase();
         if (
-          valueToLower.startsWith('last') ||
-          valueToLower.startsWith('next') ||
-          valueToLower.startsWith('previous')
+          frame === 'Common' ||
+          frame === 'Calendar' ||
+          frame === 'No filter'
         ) {
           setActualTimeRange(value);
           setTooltipTitle(actualRange || '');

superset-frontend/src/explore/components/controls/DateFilterControl/frame/AdvancedFrame.tsx

@@ -23,7 +23,11 @@ import { Input } from 'src/common/components';
 import { FrameComponentProps } from '../types';
 
 export function AdvancedFrame(props: FrameComponentProps) {
-  const [since, until] = getAdvancedRange(props.value || '').split(SEPARATOR);
+  const advancedRange = getAdvancedRange(props.value || '');
+  const [since, until] = advancedRange.split(SEPARATOR);
+  if (advancedRange !== props.value) {
+    props.onChange(getAdvancedRange(props.value || ''));
+  }
 
   function getAdvancedRange(value: string): string {
     if (value.includes(SEPARATOR)) {

superset/migrations/versions/260bf0649a77_migrate_x_dateunit_in_time_range.py

@@ -0,0 +1,106 @@
+# 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.
+"""migrate [x dateunit] to [x dateunit ago/later]
+
+Revision ID: 260bf0649a77
+Revises: c878781977c6
+Create Date: 2021-01-23 16:25:14.496774
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "260bf0649a77"
+down_revision = "c878781977c6"
+
+import json
+import re
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy import Column, Integer, or_, Text
+from sqlalchemy.dialects.mysql.base import MySQLDialect
+from sqlalchemy.dialects.sqlite.base import SQLiteDialect
+from sqlalchemy.exc import OperationalError
+from sqlalchemy.ext.declarative import declarative_base
+
+from superset import db
+from superset.utils.date_parser import DateRangeMigration
+
+Base = declarative_base()
+
+
+class Slice(Base):
+    __tablename__ = "slices"
+
+    id = Column(Integer, primary_key=True)
+    slice_name = Column(Text)
+    params = Column(Text)
+
+
+def upgrade():
+    bind = op.get_bind()
+    session = db.Session(bind=bind)
+    x_dateunit_in_since = DateRangeMigration.x_dateunit_in_since
+    x_dateunit_in_until = DateRangeMigration.x_dateunit_in_until
+
+    if isinstance(bind.dialect, SQLiteDialect):
+        # The REGEXP operator is a special syntax for the regexp() user function.
+        # https://www.sqlite.org/lang_expr.html#regexp
+        to_lower = sa.func.LOWER
+        where_clause = or_(
+            sa.func.REGEXP(to_lower(Slice.params), x_dateunit_in_since),
+            sa.func.REGEXP(to_lower(Slice.params), x_dateunit_in_until),
+        )
+    elif isinstance(bind.dialect, MySQLDialect):
+        to_lower = sa.func.LOWER
+        where_clause = or_(
+            to_lower(Slice.params).op("REGEXP")(x_dateunit_in_since),
+            to_lower(Slice.params).op("REGEXP")(x_dateunit_in_until),
+        )
+    else:
+        # isinstance(bind.dialect, PGDialect):
+        where_clause = or_(
+            Slice.params.op("~*")(x_dateunit_in_since),
+            Slice.params.op("~*")(x_dateunit_in_until),
+        )
+
+    try:
+        slices = session.query(Slice).filter(where_clause).all()
+        sep = " : "
+        pattern = DateRangeMigration.x_dateunit
+        for idx, slc in enumerate(slices):
+            print(f"Upgrading ({idx + 1}/{len(slices)}): {slc.slice_name}#{slc.id}")
+            params = json.loads(slc.params)
+            time_range = params["time_range"]
+            if sep in time_range:
+                start, end = time_range.split(sep)
+                if re.match(pattern, start):
+                    start = f"{start.strip()} ago"
+                if re.match(pattern, end):
+                    end = f"{end.strip()} later"
+                params["time_range"] = f"{start}{sep}{end}"
+
+                slc.params = json.dumps(params, sort_keys=True, indent=4)
+                session.commit()
+    except OperationalError:
+        pass
+
+    session.close()
+
+
+def downgrade():
+    pass

superset/utils/date_parser.py

@@ -71,19 +71,35 @@ def parse_human_datetime(human_readable: str) -> datetime:
     >>> year_after_1 == year_after_2
     True
     """
+    x_periods = r"^\s*([0-9]+)\s+(second|minute|hour|day|week|month|quarter|year)s?\s*$"
+    if re.search(x_periods, human_readable, re.IGNORECASE):
+        raise ValueError(
+            _(
+                "Date string is unclear."
+                " Please specify [%(human_readable)s ago]"
+                " or [%(human_readable)s later]",
+                human_readable=human_readable,
+            )
+        )
+
     try:
         dttm = parse(human_readable)
-    except Exception:  # pylint: disable=broad-except
-        try:
-            cal = parsedatetime.Calendar()
-            parsed_dttm, parsed_flags = cal.parseDT(human_readable)
-            # when time is not extracted, we 'reset to midnight'
-            if parsed_flags & 2 == 0:
-                parsed_dttm = parsed_dttm.replace(hour=0, minute=0, second=0)
-            dttm = dttm_from_timetuple(parsed_dttm.utctimetuple())
-        except Exception as ex:
+    except (ValueError, OverflowError) as ex:
+        cal = parsedatetime.Calendar()
+        parsed_dttm, parsed_flags = cal.parseDT(human_readable)
+        # 0 == not parsed at all
+        if parsed_flags == 0:
             logger.exception(ex)
-            raise ValueError("Couldn't parse date string [{}]".format(human_readable))
+            raise ValueError(
+                _(
+                    "Couldn't parse date string [%(human_readable)s]",
+                    human_readable=human_readable,
+                )
+            )
+        # when time is not extracted, we 'reset to midnight'
+        if parsed_flags & 2 == 0:
+            parsed_dttm = parsed_dttm.replace(hour=0, minute=0, second=0)
+        dttm = dttm_from_timetuple(parsed_dttm.utctimetuple())
     return dttm
 
 
@@ -375,7 +391,9 @@ def eval(self) -> datetime:
         searched_result = holiday_lookup.get_named(holiday)
         if len(searched_result) == 1:
             return dttm_from_timetuple(searched_result[0].timetuple())
-        raise ValueError(_("Unable to find such a holiday: [{}]").format(holiday))
+        raise ValueError(
+            _("Unable to find such a holiday: [%(holiday)s]", holiday=holiday)
+        )
 
 
 @memoized
@@ -470,3 +488,13 @@ def datetime_eval(datetime_expression: Optional[str] = None) -> Optional[datetim
         except ParseException as error:
             raise ValueError(error)
     return None
+
+
+class DateRangeMigration:  # pylint: disable=too-few-public-methods
+    x_dateunit_in_since = (
+        r'"time_range":\s"\s*[0-9]+\s(day|week|month|quarter|year)s?\s*\s:\s'
+    )
+    x_dateunit_in_until = (
+        r'"time_range":\s".*\s:\s\s*[0-9]+\s(day|week|month|quarter|year)s?\s*"'
+    )
+    x_dateunit = r"\s*[0-9]+\s(day|week|month|quarter|year)s?\s*"

tests/druid_tests.py

@@ -176,7 +176,7 @@ def test_client(self, PyDruid):
             "viz_type": "table",
             "granularity": "one+day",
             "druid_time_origin": "",
-            "since": "7+days+ago",
+            "since": "7 days ago",
             "until": "now",
             "row_limit": 5000,
             "include_search": "false",
@@ -193,7 +193,7 @@ def test_client(self, PyDruid):
             "viz_type": "table",
             "granularity": "one+day",
             "druid_time_origin": "",
-            "since": "7+days+ago",
+            "since": "7 days ago",
             "until": "now",
             "row_limit": 5000,
             "include_search": "false",
@@ -535,7 +535,7 @@ def test_druid_time_granularities(self, PyDruid):
 
         form_data = {
             "viz_type": "table",
-            "since": "7+days+ago",
+            "since": "7 days ago",
             "until": "now",
             "metrics": ["count"],
             "groupby": [],

tests/utils/date_parser_tests.py

@@ -18,8 +18,10 @@
 from unittest.mock import patch
 
 from superset.utils.date_parser import (
+    DateRangeMigration,
     datetime_eval,
     get_since_until,
+    parse_human_datetime,
     parse_human_timedelta,
     parse_past_timedelta,
 )
@@ -261,3 +263,31 @@ def test_parse_past_timedelta(self, mock_datetime):
         self.assertEqual(parse_past_timedelta("-1 year"), timedelta(365))
         self.assertEqual(parse_past_timedelta("52 weeks"), timedelta(364))
         self.assertEqual(parse_past_timedelta("1 month"), timedelta(31))
+
+    def test_parse_human_datetime(self):
+        with self.assertRaises(ValueError):
+            parse_human_datetime("  2 days  ")
+
+        with self.assertRaises(ValueError):
+            parse_human_datetime("2 day")
+
+        with self.assertRaises(ValueError):
+            parse_human_datetime("xxxxxxx")
+
+    def test_DateRangeMigration(self):
+        params = '{"time_range": "   8 days     : 2020-03-10T00:00:00"}'
+        self.assertRegex(params, DateRangeMigration.x_dateunit_in_since)
+
+        params = '{"time_range": "2020-03-10T00:00:00 :    8 days    "}'
+        self.assertRegex(params, DateRangeMigration.x_dateunit_in_until)
+
+        params = '{"time_range": "   2 weeks    :    8 days    "}'
+        self.assertRegex(params, DateRangeMigration.x_dateunit_in_since)
+        self.assertRegex(params, DateRangeMigration.x_dateunit_in_until)
+
+        params = '{"time_range": "2 weeks ago : 8 days later"}'
+        self.assertNotRegex(params, DateRangeMigration.x_dateunit_in_since)
+        self.assertNotRegex(params, DateRangeMigration.x_dateunit_in_until)
+
+        field = "   8 days   "
+        self.assertRegex(field, DateRangeMigration.x_dateunit)