fix: datetime comparison issues (#75)

CHANGELOG.md

@@ -1,3 +1,20 @@
+## 2.1.2 - 2022-09-15
+
+Changes:
+
+1. Fixes issues with date comparison.
+## 2.1.1 - 2022-09-14
+
+Changes:
+
+1. Feature flags local evaluation now supports date property filters as well. Accepts both strings and datetime objects.
+
+## 2.1.0 - 2022-08-11
+
+Changes:
+
+1. Feature flag defaults have been removed
+2. Setup logging only when debug mode is enabled.
 ## 2.0.1 - 2022-08-04
 
 - Make poll_interval configurable
@@ -9,7 +26,8 @@
 Breaking changes:
 
 1. The minimum version requirement for PostHog servers is now 1.38. If you're using PostHog Cloud, you satisfy this requirement automatically.
-2. Feature flag defaults apply only when there's an error fetching feature flag results. Earlier, if the default was set to `True`, even if a flag resolved to `False`, the default would override this.
+2. Feature flag defaults apply only when there's an error fetching feature flag results. Earlier, if the default was set to `True`, even if a flag resolved to `False`, the default would override this. 
+**Note: These are removed in 2.0.2**
 3. Feature flag remote evaluation doesn't require a personal API key.
 
 New Changes:

posthog/__init__.py

@@ -235,7 +235,6 @@ def alias(
 def feature_enabled(
     key,  # type: str,
     distinct_id,  # type: str,
-    default=False,  # type: bool
     groups={},  # type: dict
     person_properties={},  # type: dict
     group_properties={},  # type: dict
@@ -260,7 +259,6 @@ def feature_enabled(
         "feature_enabled",
         key=key,
         distinct_id=distinct_id,
-        default=default,
         groups=groups,
         person_properties=person_properties,
         group_properties=group_properties,
@@ -272,7 +270,6 @@ def feature_enabled(
 def get_feature_flag(
     key,  # type: str,
     distinct_id,  # type: str,
-    default=False,  # type: bool
     groups={},  # type: dict
     person_properties={},  # type: dict
     group_properties={},  # type: dict
@@ -305,7 +302,6 @@ def get_feature_flag(
         "get_feature_flag",
         key=key,
         distinct_id=distinct_id,
-        default=default,
         groups=groups,
         person_properties=person_properties,
         group_properties=group_properties,

posthog/client.py

@@ -23,10 +23,6 @@ except ImportError:
 ID_TYPES = (numbers.Number, string_types, UUID)
 MAX_DICT_SIZE = 50_000
 
-# Ensures that debug level messages are logged when debug mode is on.
-# Otherwise, defaults to WARNING level. See https://docs.python.org/3/howto/logging.html#what-happens-if-no-configuration-is-provided
-logging.basicConfig()
-
 
 class Client(object):
     """Create a new PostHog client."""
@@ -76,6 +72,9 @@ class Client(object):
         # personal_api_key: This should be a generated Personal API Key, private
         self.personal_api_key = personal_api_key
         if debug:
+            # Ensures that debug level messages are logged when debug mode is on.
+            # Otherwise, defaults to WARNING level. See https://docs.python.org/3/howto/logging.html#what-happens-if-no-configuration-is-provided
+            logging.basicConfig()
             self.log.setLevel(logging.DEBUG)
         else:
             self.log.setLevel(logging.WARNING)
@@ -432,7 +431,6 @@ class Client(object):
         self,
         key,
         distinct_id,
-        default=False,
         *,
         groups={},
         person_properties={},
@@ -440,24 +438,24 @@ class Client(object):
         only_evaluate_locally=False,
         send_feature_flag_events=True,
     ):
-        return bool(
-            self.get_feature_flag(
-                key,
-                distinct_id,
-                default,
-                groups=groups,
-                person_properties=person_properties,
-                group_properties=group_properties,
-                only_evaluate_locally=only_evaluate_locally,
-                send_feature_flag_events=send_feature_flag_events,
-            )
+        response = self.get_feature_flag(
+            key,
+            distinct_id,
+            groups=groups,
+            person_properties=person_properties,
+            group_properties=group_properties,
+            only_evaluate_locally=only_evaluate_locally,
+            send_feature_flag_events=send_feature_flag_events,
         )
 
+        if response is None:
+            return None
+        return bool(response)
+
     def get_feature_flag(
         self,
         key,
         distinct_id,
-        default=False,
         *,
         groups={},
         person_properties={},
@@ -500,10 +498,11 @@ class Client(object):
                     distinct_id, groups=groups, person_properties=person_properties, group_properties=group_properties
                 )
                 response = feature_flags.get(key)
+                if response is None:
+                    response = False
                 self.log.debug(f"Successfully computed flag remotely: #{key} -> #{response}")
             except Exception as e:
                 self.log.exception(f"[FEATURE FLAGS] Unable to get flag remotely: {e}")
-                response = default
 
         feature_flag_reported_key = f"{key}_{str(response)}"
         if (

posthog/feature_flags.py

@@ -1,7 +1,10 @@
+import datetime
 import hashlib
 import re
 
-from posthog.utils import is_valid_regex
+from dateutil import parser
+
+from posthog.utils import convert_to_datetime_aware, is_valid_regex
 
 __LONG_SCALE__ = float(0xFFFFFFFFFFFFFFF)
 
@@ -126,4 +129,35 @@ def match_property(property, property_values) -> bool:
     if operator == "lte":
         return type(override_value) == type(value) and override_value <= value
 
+    if operator in ["is_date_before", "is_date_after"]:
+        try:
+            parsed_date = parser.parse(value)
+            parsed_date = convert_to_datetime_aware(parsed_date)
+        except Exception:
+            raise InconclusiveMatchError("The date set on the flag is not a valid format")
+
+        if isinstance(override_value, datetime.datetime):
+            override_date = convert_to_datetime_aware(override_value)
+            if operator == "is_date_before":
+                return override_date < parsed_date
+            else:
+                return override_date > parsed_date
+        elif isinstance(override_value, datetime.date):
+            if operator == "is_date_before":
+                return override_value < parsed_date.date()
+            else:
+                return override_value > parsed_date.date()
+        elif isinstance(override_value, str):
+            try:
+                override_date = parser.parse(override_value)
+                override_date = convert_to_datetime_aware(override_date)
+                if operator == "is_date_before":
+                    return override_date < parsed_date
+                else:
+                    return override_date > parsed_date
+            except Exception:
+                raise InconclusiveMatchError("The date provided is not a valid format")
+        else:
+            raise InconclusiveMatchError("The date provided must be a string or date object")
+
     return False

posthog/test/test_feature_flags.py

@@ -1,6 +1,8 @@
+import datetime
 import unittest
 
 import mock
+from dateutil import parser, tz
 from freezegun import freeze_time
 
 from posthog.client import Client
@@ -385,7 +387,7 @@ class TestLocalEvaluation(unittest.TestCase):
 
         feature_flag_match = client.feature_enabled("beta-feature", "some-distinct-id", only_evaluate_locally=True)
 
-        self.assertEqual(feature_flag_match, False)
+        self.assertEqual(feature_flag_match, None)
         self.assertEqual(patch_decide.call_count, 0)
 
         # beta-feature2 should fallback to decide because region property not given with call
@@ -394,13 +396,13 @@ class TestLocalEvaluation(unittest.TestCase):
         self.assertEqual(feature_flag_match, None)
 
         feature_flag_match = client.feature_enabled("beta-feature2", "some-distinct-id", only_evaluate_locally=True)
-        self.assertEqual(feature_flag_match, False)
+        self.assertEqual(feature_flag_match, None)
 
         self.assertEqual(patch_decide.call_count, 0)
 
     @mock.patch("posthog.client.decide")
     @mock.patch("posthog.client.get")
-    def test_feature_flag_defaults_dont_hinder_regular_evaluation(self, patch_get, patch_decide):
+    def test_feature_flag_never_returns_undefined_during_regular_evaluation(self, patch_get, patch_decide):
         patch_decide.return_value = {"featureFlags": {}}
         client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
         client.feature_flags = [
@@ -422,28 +424,28 @@ class TestLocalEvaluation(unittest.TestCase):
         ]
 
         # beta-feature resolves to False, so no matter the default, stays False
-        self.assertFalse(client.get_feature_flag("beta-feature", "some-distinct-id", default=True))
-        self.assertFalse(client.get_feature_flag("beta-feature", "some-distinct-id", default=False))
+        self.assertFalse(client.get_feature_flag("beta-feature", "some-distinct-id"))
+        self.assertFalse(client.feature_enabled("beta-feature", "some-distinct-id"))
 
         # beta-feature2 falls back to decide, and whatever decide returns is the value
-        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id", default=False))
+        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 1)
 
-        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id", default=True))
+        self.assertFalse(client.feature_enabled("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 2)
 
     @mock.patch("posthog.client.decide")
     @mock.patch("posthog.client.get")
-    def test_feature_flag_defaults_come_into_play_only_when_decide_errors_out(self, patch_get, patch_decide):
+    def test_feature_flag_return_none_when_decide_errors_out(self, patch_get, patch_decide):
         patch_decide.side_effect = APIError(400, "Decide error")
         client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
         client.feature_flags = []
 
-        # beta-feature2 falls back to decide, which on error falls back to default
-        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id", default=False))
+        # beta-feature2 falls back to decide, which on error returns None
+        self.assertIsNone(client.get_feature_flag("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 1)
 
-        self.assertTrue(client.get_feature_flag("beta-feature2", "some-distinct-id", default=True))
+        self.assertIsNone(client.feature_enabled("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 2)
 
     @mock.patch("posthog.client.decide")
@@ -943,7 +945,7 @@ class TestLocalEvaluation(unittest.TestCase):
         self.assertFalse(client.feature_enabled("doesnt-exist", "distinct_id"))
 
         patch_decide.side_effect = APIError(401, "decide error")
-        self.assertTrue(client.feature_enabled("doesnt-exist", "distinct_id", True))
+        self.assertIsNone(client.feature_enabled("doesnt-exist", "distinct_id"))
 
     @mock.patch("posthog.client.Poller")
     @mock.patch("posthog.client.decide")
@@ -1118,6 +1120,58 @@ class TestMatchProperties(unittest.TestCase):
         self.assertFalse(match_property(property_d, {"key": "44"}))
         self.assertFalse(match_property(property_d, {"key": 44}))
 
+    def test_match_property_date_operators(self):
+        property_a = self.property(key="key", value="2022-05-01", operator="is_date_before")
+        self.assertTrue(match_property(property_a, {"key": "2022-03-01"}))
+        self.assertTrue(match_property(property_a, {"key": "2022-04-30"}))
+        self.assertTrue(match_property(property_a, {"key": datetime.date(2022, 4, 30)}))
+        self.assertTrue(match_property(property_a, {"key": datetime.datetime(2022, 4, 30, 1, 2, 3)}))
+        self.assertTrue(
+            match_property(
+                property_a, {"key": datetime.datetime(2022, 4, 30, 1, 2, 3, tzinfo=tz.gettz("Europe/Madrid"))}
+            )
+        )
+        self.assertTrue(match_property(property_a, {"key": parser.parse("2022-04-30")}))
+        self.assertFalse(match_property(property_a, {"key": "2022-05-30"}))
+
+        # Can't be a number
+        with self.assertRaises(InconclusiveMatchError):
+            match_property(property_a, {"key": 1})
+
+        # can't be invalid string
+        with self.assertRaises(InconclusiveMatchError):
+            match_property(property_a, {"key": "abcdef"})
+
+        property_b = self.property(key="key", value="2022-05-01", operator="is_date_after")
+        self.assertTrue(match_property(property_b, {"key": "2022-05-02"}))
+        self.assertTrue(match_property(property_b, {"key": "2022-05-30"}))
+        self.assertTrue(match_property(property_b, {"key": datetime.datetime(2022, 5, 30)}))
+        self.assertTrue(match_property(property_b, {"key": parser.parse("2022-05-30")}))
+        self.assertFalse(match_property(property_b, {"key": "2022-04-30"}))
+
+        # can't be invalid string
+        with self.assertRaises(InconclusiveMatchError):
+            match_property(property_b, {"key": "abcdef"})
+
+        # Invalid flag property
+        property_c = self.property(key="key", value=1234, operator="is_date_before")
+
+        with self.assertRaises(InconclusiveMatchError):
+            match_property(property_c, {"key": 1})
+
+        # Timezone aware property
+        property_d = self.property(key="key", value="2022-04-05 12:34:12 +01:00", operator="is_date_before")
+        self.assertFalse(match_property(property_d, {"key": "2022-05-30"}))
+
+        self.assertTrue(match_property(property_d, {"key": "2022-03-30"}))
+        self.assertTrue(match_property(property_d, {"key": "2022-04-05 12:34:11 +01:00"}))
+        self.assertTrue(match_property(property_d, {"key": "2022-04-05 12:34:11 +01:00"}))
+
+        self.assertFalse(match_property(property_d, {"key": "2022-04-05 12:34:13 +01:00"}))
+
+        self.assertTrue(match_property(property_d, {"key": "2022-04-05 11:34:11 +00:00"}))
+        self.assertFalse(match_property(property_d, {"key": "2022-04-05 11:34:13 +00:00"}))
+
 
 class TestCaptureCalls(unittest.TestCase):
     @mock.patch.object(Client, "capture")

posthog/utils.py

@@ -2,7 +2,7 @@ import logging
 import numbers
 import re
 from collections import defaultdict
-from datetime import date, datetime
+from datetime import date, datetime, timezone
 from decimal import Decimal
 from uuid import UUID
 
@@ -109,3 +109,9 @@ class SizeLimitedDict(defaultdict):
             self.clear()
 
         super().__setitem__(key, value)
+
+
+def convert_to_datetime_aware(date_obj):
+    if date_obj.tzinfo is None:
+        date_obj = date_obj.replace(tzinfo=timezone.utc)
+    return date_obj

posthog/version.py

@@ -1,4 +1,4 @@
-VERSION = "2.0.1"
+VERSION = "2.1.2"
 
 if __name__ == "__main__":
     print(VERSION, end="")