Merge pull request #98 from reddit/add_subreddit_ad_account_id_identifiers

Add `subreddit_id`, `ad_account_id`, & `business_id` identifiers

Author: mrlevitas

reddit_decider/__init__.py

@@ -34,7 +34,14 @@
 logger = logging.getLogger(__name__)
 
 EMPLOYEE_ROLES = ["employee", "contractor"]
-IDENTIFIERS = ["user_id", "device_id", "canonical_url"]
+IDENTIFIERS = [
+    "user_id",
+    "device_id",
+    "canonical_url",
+    "subreddit_id",
+    "ad_account_id",
+    "business_id",
+]
 TYPE_STR_LOOKUP = {bool: "boolean", int: "integer", float: "float", str: "string", dict: "map"}
 
 
@@ -411,7 +418,9 @@ def get_variant_for_identifier(
         self,
         experiment_name: str,
         identifier: str,
-        identifier_type: Literal["user_id", "device_id", "canonical_url"],
+        identifier_type: Literal[
+            "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id"
+        ],
         **exposure_kwargs: Optional[Dict[str, Any]],
     ) -> Optional[str]:
         """Return a bucketing variant, if any, with auto-exposure for a given :code:`identifier`.
@@ -471,7 +480,9 @@ def get_variant_for_identifier_without_expose(
         self,
         experiment_name: str,
         identifier: str,
-        identifier_type: Literal["user_id", "device_id", "canonical_url"],
+        identifier_type: Literal[
+            "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id"
+        ],
     ) -> Optional[str]:
         """Return a bucketing variant, if any, without emitting exposure event for a given :code:`identifier`.
 
@@ -587,7 +598,11 @@ def _decision_to_dict(self, decision: Decision) -> Dict[str, Any]:
         }
 
     def get_all_variants_for_identifier_without_expose(
-        self, identifier: str, identifier_type: Literal["user_id", "device_id", "canonical_url"]
+        self,
+        identifier: str,
+        identifier_type: Literal[
+            "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id"
+        ],
     ) -> List[Dict[str, Union[str, int]]]:
         """Return a list of experiment dicts for experiments having :code:`bucket_val` match
         :code:`identifier_type`, for a given :code:`identifier`, in this format:

requirements.txt

@@ -1,7 +1,7 @@
 -r requirements-transitive.txt
 baseplate>=2.0.0a1,<3.0
 black==21.4b2
-reddit-decider==1.2.31
+reddit-decider==1.2.32
 flake8==3.9.1
 mypy==0.790
 pyramid==2.0   # required for `from baseplate.frameworks.pyramid import BaseplateRequest` which calls `import pyramid.events`

setup.py

@@ -19,7 +19,7 @@
     install_requires=[
         "baseplate>=2.0.0a1,<3.0",
         "reddit-edgecontext>=1.0.0a3,<2.0",
-        "reddit-decider~=1.2.31",
+        "reddit-decider~=1.2.32",
         "typing_extensions>=3.10.0.0,<5.0",
     ],
     package_data={"reddit_experiments": ["py.typed"], "reddit_decider": ["py.typed"]},

tests/decider_tests.py

@@ -24,22 +24,25 @@
 
 USER_ID = "t2_1234"
 IS_LOGGED_IN = True
-AUTH_CLIENT_ID = "token"
-COUNTRY_CODE = "US"
-DEVICE_ID = "abc"
-COOKIE_CREATED_TIMESTAMP = 1234
-LOID_CREATED_TIMESTAMP = 123456
-LOCALE_CODE = "us_en"
-ORIGIN_SERVICE = "origin"
+AD_ACCOUNT_ID = "t2_4321"
 APP_NAME = "ios"
 APP_VERSION = "0.0.0.0"
+AUTH_CLIENT_ID = "token"
 BUILD_NUMBER = 1
+BUSINESS_ID = "t_some"
 CANONICAL_URL = "www.test.com"
+COOKIE_CREATED_TIMESTAMP = 1234
+COUNTRY_CODE = "US"
+DEVICE_ID = "abc"
 EVENT_FIELDS = {
     "user_id": USER_ID,
     "logged_in": IS_LOGGED_IN,
     "cookie_created_timestamp": COOKIE_CREATED_TIMESTAMP,
 }
+LOID_CREATED_TIMESTAMP = 123456
+LOCALE_CODE = "us_en"
+ORIGIN_SERVICE = "origin"
+SUBREDDIT_ID = "t5_123abc"
 
 
 @contextlib.contextmanager
@@ -706,6 +709,90 @@ def test_get_variant_for_identifier_device_id(self):
             # `identifier` passed to correct event field of experiment's `bucket_val` config
             self.assertEqual(event_fields["device_id"], identifier)
 
+    def test_get_variant_for_identifier_subreddit_id(self):
+        identifier = SUBREDDIT_ID
+        bucket_val = "subreddit_id"
+        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
+
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+            variant = decider.get_variant_for_identifier(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "control_1")
+
+            # exposure assertions
+            self.assertEqual(self.event_logger.log.call_count, 1)
+            event_fields = self.event_logger.log.call_args[1]
+            self.assert_minimal_exposure_event_fields(
+                experiment_name="exp_1",
+                variant=variant,
+                event_fields=event_fields,
+                bucket_val=bucket_val,
+                identifier=identifier,
+            )
+
+            # `identifier` passed to correct event field of experiment's `bucket_val` config
+            self.assertEqual(event_fields["subreddit_id"], identifier)
+
+    def test_get_variant_for_identifier_ad_account_id(self):
+        identifier = AD_ACCOUNT_ID
+        bucket_val = "ad_account_id"
+        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
+
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+            variant = decider.get_variant_for_identifier(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "variant_2")
+
+            # exposure assertions
+            self.assertEqual(self.event_logger.log.call_count, 1)
+            event_fields = self.event_logger.log.call_args[1]
+            self.assert_minimal_exposure_event_fields(
+                experiment_name="exp_1",
+                variant=variant,
+                event_fields=event_fields,
+                bucket_val=bucket_val,
+                identifier=identifier,
+            )
+
+            # `identifier` passed to correct event field of experiment's `bucket_val` config
+            self.assertEqual(event_fields["ad_account_id"], identifier)
+
+    def test_get_variant_for_identifier_business_id(self):
+        identifier = BUSINESS_ID
+        bucket_val = "business_id"
+        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
+
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+            variant = decider.get_variant_for_identifier(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "control_2")
+
+            # exposure assertions
+            self.assertEqual(self.event_logger.log.call_count, 1)
+            event_fields = self.event_logger.log.call_args[1]
+            self.assert_minimal_exposure_event_fields(
+                experiment_name="exp_1",
+                variant=variant,
+                event_fields=event_fields,
+                bucket_val=bucket_val,
+                identifier=identifier,
+            )
+
+            # `identifier` passed to correct event field of experiment's `bucket_val` config
+            self.assertEqual(event_fields["business_id"], identifier)
+
     def test_get_variant_for_identifier_bogus_identifier_type(self):
         identifier = "anything"
         identifier_type = "blah"
@@ -724,7 +811,7 @@ def test_get_variant_for_identifier_bogus_identifier_type(self):
                 self.assertEqual(variant, None)
 
                 assert any(
-                    "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url']."
+                    "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url', 'subreddit_id', 'ad_account_id', 'business_id']."
                     in x.getMessage()
                     for x in captured.records
                 )
@@ -847,6 +934,57 @@ def test_get_variant_for_identifier_without_expose_device_id(self):
             # no exposures should be triggered
             self.assertEqual(self.event_logger.log.call_count, 0)
 
+    def test_get_variant_for_identifier_without_expose_subreddit_id(self):
+        identifier = SUBREDDIT_ID
+        bucket_val = "subreddit_id"
+        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
+
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+            variant = decider.get_variant_for_identifier_without_expose(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "control_1")
+
+            # no exposures should be triggered
+            self.assertEqual(self.event_logger.log.call_count, 0)
+
+    def test_get_variant_for_identifier_without_expose_ad_account_id(self):
+        identifier = AD_ACCOUNT_ID
+        bucket_val = "ad_account_id"
+        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
+
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+            variant = decider.get_variant_for_identifier_without_expose(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "variant_2")
+
+            # no exposures should be triggered
+            self.assertEqual(self.event_logger.log.call_count, 0)
+
+    def test_get_variant_for_identifier_without_expose_business_id(self):
+        identifier = BUSINESS_ID
+        bucket_val = "business_id"
+        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
+
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+            variant = decider.get_variant_for_identifier_without_expose(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "control_2")
+
+            # no exposures should be triggered
+            self.assertEqual(self.event_logger.log.call_count, 0)
+
     def test_get_variant_for_identifier_without_expose_bogus_identifier_type(self):
         identifier = "anything"
         identifier_type = "blah"
@@ -863,7 +1001,7 @@ def test_get_variant_for_identifier_without_expose_bogus_identifier_type(self):
                 self.assertEqual(variant, None)
 
                 assert any(
-                    "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url']."
+                    "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url', 'subreddit_id', 'ad_account_id', 'business_id']."
                     in x.getMessage()
                     for x in captured.records
                 )
@@ -1277,7 +1415,7 @@ def test_get_all_variants_for_identifier_without_expose_bogus_identifier_type(se
                 self.assertEqual(len(variant_arr), 0)
 
                 assert any(
-                    "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url']."
+                    "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url', 'subreddit_id', 'ad_account_id', 'business_id']."
                     in x.getMessage()
                     for x in captured.records
                 )