Fixing time comparison to look for past deltas (#7616)

* Fixing time comparison to look for past deltas

* Adding note to updating about time_compare

UPDATING.md

@@ -44,6 +44,11 @@ creation of permissions set `FAB_UPDATE_PERMS = False` on config.
 which adds missing non-nullable fields and uniqueness constraints to the metrics
 and sql_metrics tables. Depending on the integrity of the data, manual
 intervention may be required.
+* [7616](https://github.com/apache/incubator-superset/pull/7616): this bug fix
+changes time_compare deltas to correctly evaluate to the number of days prior
+instead of number of days in the future. It will change the data for advanced
+analytics time_compare so `1 year` from 5/1/2019 will be calculated as 365 days
+instead of 366 days.
 
 ## Superset 0.32.0
 

superset/utils/core.py

@@ -298,7 +298,7 @@ def default(self, o):
             return json.JSONEncoder.default(self, o)
 
 
-def parse_human_timedelta(s: str):
+def parse_human_timedelta(s: str) -> timedelta:
     """
     Returns ``datetime.datetime`` from natural language time deltas
 
@@ -312,6 +312,19 @@ def parse_human_timedelta(s: str):
     return d - dttm
 
 
+def parse_past_timedelta(delta_str: str) -> timedelta:
+    """
+    Takes a delta like '1 year' and finds the timedelta for that period in
+    the past, then represents that past timedelta in positive terms.
+
+    parse_human_timedelta('1 year') find the timedelta 1 year in the future.
+    parse_past_timedelta('1 year') returns -datetime.timedelta(-365)
+    or datetime.timedelta(365).
+    """
+    return -parse_human_timedelta(
+        delta_str if delta_str.startswith('-') else f'-{delta_str}')
+
+
 class JSONEncodedDict(TypeDecorator):
     """Represents an immutable structure as a json-encoded string."""
 
@@ -1003,9 +1016,9 @@ def get_since_until(time_range: Optional[str] = None,
         until = parse_human_datetime(until) if until else relative_end
 
     if time_shift:
-        time_shift = parse_human_timedelta(time_shift)
-        since = since if since is None else (since - time_shift)  # noqa: T400
-        until = until if until is None else (until - time_shift)  # noqa: T400
+        time_delta = parse_past_timedelta(time_shift)
+        since = since if since is None else (since - time_delta)  # noqa: T400
+        until = until if until is None else (until - time_delta)  # noqa: T400
 
     if since and until and since > until:
         raise ValueError(_('From date cannot be larger than to date'))

superset/viz.py

@@ -281,7 +281,7 @@ def query_obj(self):
                                              since=form_data.get('since'),
                                              until=form_data.get('until'))
         time_shift = form_data.get('time_shift', '')
-        self.time_shift = utils.parse_human_timedelta(time_shift)
+        self.time_shift = utils.parse_past_timedelta(time_shift)
         from_dttm = None if since is None else (since - self.time_shift)
         to_dttm = None if until is None else (until - self.time_shift)
         if from_dttm and to_dttm and from_dttm > to_dttm:
@@ -1210,7 +1210,7 @@ def run_extra_queries(self):
 
         for option in time_compare:
             query_object = self.query_obj()
-            delta = utils.parse_human_timedelta(option)
+            delta = utils.parse_past_timedelta(option)
             query_object['inner_from_dttm'] = query_object['from_dttm']
             query_object['inner_to_dttm'] = query_object['to_dttm']
 

tests/utils_tests.py

@@ -36,6 +36,7 @@
     merge_request_params,
     parse_human_timedelta,
     parse_js_uri_path_item,
+    parse_past_timedelta,
     validate_json,
     zlib_compress,
     zlib_decompress_to_string,
@@ -119,9 +120,21 @@ def test_base_json_conv(self):
         assert isinstance(base_json_conv(uuid.uuid4()), str) is True
 
     @patch('superset.utils.core.datetime')
-    def test_parse_human_timedelta(self, mock_now):
-        mock_now.return_value = datetime(2016, 12, 1)
+    def test_parse_human_timedelta(self, mock_datetime):
+        mock_datetime.now.return_value = datetime(2019, 4, 1)
+        mock_datetime.side_effect = lambda *args, **kw: datetime(*args, **kw)
         self.assertEquals(parse_human_timedelta('now'), timedelta(0))
+        self.assertEquals(parse_human_timedelta('1 year'), timedelta(366))
+        self.assertEquals(parse_human_timedelta('-1 year'), timedelta(-365))
+
+    @patch('superset.utils.core.datetime')
+    def test_parse_past_timedelta(self, mock_datetime):
+        mock_datetime.now.return_value = datetime(2019, 4, 1)
+        mock_datetime.side_effect = lambda *args, **kw: datetime(*args, **kw)
+        self.assertEquals(parse_past_timedelta('1 year'), timedelta(365))
+        self.assertEquals(parse_past_timedelta('-1 year'), timedelta(365))
+        self.assertEquals(parse_past_timedelta('52 weeks'), timedelta(364))
+        self.assertEquals(parse_past_timedelta('1 month'), timedelta(31))
 
     def test_zlib_compression(self):
         json_str = '{"test": 1}'