diff --git a/reddit_decider/__init__.py b/reddit_decider/__init__.py index e65e532..00a6f86 100644 --- a/reddit_decider/__init__.py +++ b/reddit_decider/__init__.py @@ -57,6 +57,7 @@ "subreddit_id", "ad_account_id", "business_id", + "conversion_pixel_id", ] TYPE_STR_LOOKUP = {bool: "boolean", int: "integer", float: "float", str: "string", dict: "map"} @@ -94,6 +95,8 @@ class DeciderContext: :param cookie_created_timestamp: When the authentication cookie was created :param loid_created_timestamp: Epoch milliseconds when the current LoID cookie was created :param extracted_fields: Optional dict of additional fields, e.g. app_name & build_number + :param conversion_pixel_id: Conversion pixel ID for bucketing, targeting, and overrides. + When supplied, takes precedence over the value in extracted_fields. """ def __init__( @@ -109,6 +112,7 @@ def __init__( cookie_created_timestamp: Optional[float] = None, loid_created_timestamp: Optional[float] = None, extracted_fields: Optional[dict] = None, + conversion_pixel_id: Optional[str] = None, ): self._user_id = user_id self._device_id = device_id @@ -121,9 +125,12 @@ def __init__( self._cookie_created_timestamp = cookie_created_timestamp self._loid_created_timestamp = loid_created_timestamp self._extracted_fields = extracted_fields + self._conversion_pixel_id = conversion_pixel_id def to_dict(self) -> Dict: ef = deepcopy(self._extracted_fields or {}) + if self._conversion_pixel_id is not None: + ef["conversion_pixel_id"] = self._conversion_pixel_id return { "user_id": self._user_id, @@ -150,6 +157,9 @@ def to_event_dict(self) -> Dict: ef = deepcopy(self._extracted_fields or {}) + if self._conversion_pixel_id is not None: + ef["conversion_pixel_id"] = self._conversion_pixel_id + app_fields = {} if ef.get("app_name"): app_fields["name"] = ef["app_name"] @@ -445,14 +455,20 @@ def get_variant_for_identifier( experiment_name: str, identifier: str, identifier_type: Literal[ - "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id" + "user_id", + "device_id", + "canonical_url", + "subreddit_id", + "ad_account_id", + "business_id", + "conversion_pixel_id", ], **exposure_kwargs: Optional[Dict[str, Any]], ) -> Optional[str]: """Return a bucketing variant, if any, with auto-exposure for a given :code:`identifier`. Note: If the experiment's :code:`bucket_val` - (e.g. "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id") + (for example, "user_id" or "conversion_pixel_id") does not match the :code:`identifier_type` param, the :code:`identifier` will be ignored and not used to bucket (:code:`{identifier_type: identifier}` is added to internal :code:`DeciderContext` instance, but doesn't act like a bucketing override). @@ -489,6 +505,8 @@ def get_variant_for_identifier( ctx = self._decider_context.to_dict() ctx[identifier_type] = identifier + if identifier_type == "conversion_pixel_id": + ctx["other_fields"][identifier_type] = identifier decision = self._get_decision(experiment_name, ctx) @@ -508,13 +526,19 @@ def get_variant_for_identifier_without_expose( experiment_name: str, identifier: str, identifier_type: Literal[ - "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id" + "user_id", + "device_id", + "canonical_url", + "subreddit_id", + "ad_account_id", + "business_id", + "conversion_pixel_id", ], ) -> Optional[str]: """Return a bucketing variant, if any, without emitting exposure event for a given :code:`identifier`. Note: If the experiment's :code:`bucket_val` - (e.g. "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id") + (for example, "user_id" or "conversion_pixel_id") does not match the :code:`identifier_type` param, the :code:`identifier` will be ignored and not used to bucket (:code:`{identifier_type: identifier}` is added to internal :code:`DeciderContext` instance, but doesn't act like a bucketing override). @@ -552,6 +576,8 @@ def get_variant_for_identifier_without_expose( ctx = self._decider_context.to_dict() ctx[identifier_type] = identifier + if identifier_type == "conversion_pixel_id": + ctx["other_fields"][identifier_type] = identifier decision = self._get_decision(experiment_name, ctx) @@ -629,7 +655,13 @@ def get_all_variants_for_identifier_without_expose( self, identifier: str, identifier_type: Literal[ - "user_id", "device_id", "canonical_url", "subreddit_id", "ad_account_id", "business_id" + "user_id", + "device_id", + "canonical_url", + "subreddit_id", + "ad_account_id", + "business_id", + "conversion_pixel_id", ], ) -> List[Dict[str, Union[str, int]]]: """Return a list of experiment dicts for experiments having :code:`bucket_val` match @@ -672,6 +704,8 @@ def get_all_variants_for_identifier_without_expose( ctx = self._decider_context.to_dict() ctx[identifier_type] = identifier + if identifier_type == "conversion_pixel_id": + ctx["other_fields"][identifier_type] = identifier all_decisions = self._get_all_decisions(ctx=ctx, bucketing_field_filter=identifier_type) diff --git a/tests/decider_tests.py b/tests/decider_tests.py index 20f6c87..c54e942 100644 --- a/tests/decider_tests.py +++ b/tests/decider_tests.py @@ -149,6 +149,26 @@ def test_prefix(self, file_watcher_mock): ) +class DeciderContextTests(unittest.TestCase): + def test_conversion_pixel_id(self): + for pixel_id, fields, expected in ( + ("pixel_1", {}, "pixel_1"), + (None, {"conversion_pixel_id": "pixel_2"}, "pixel_2"), + ("pixel_1", {"conversion_pixel_id": "pixel_2"}, "pixel_1"), + (None, {}, None), + ): + with self.subTest(pixel_id=pixel_id, fields=fields): + original_fields = deepcopy(fields) + context = DeciderContext(conversion_pixel_id=pixel_id, extracted_fields=fields) + serialized = context.to_dict() + self.assertEqual(serialized.get("conversion_pixel_id"), expected) + self.assertEqual(serialized["other_fields"].get("conversion_pixel_id"), expected) + self.assertEqual(context.to_event_dict().get("conversion_pixel_id"), expected) + serialized["other_fields"]["conversion_pixel_id"] = "changed" + self.assertEqual(context.to_dict().get("conversion_pixel_id"), expected) + self.assertEqual(fields, original_fields) + + class DeciderContextFactoryTests(unittest.TestCase): def setUp(self): super().setUp() @@ -1069,6 +1089,48 @@ def test_get_variant_for_identifier_business_id(self): # `identifier` passed to correct event field of experiment's `bucket_val` config self.assertEqual(event_fields["business_id"], identifier) + def test_conversion_pixel_id_override(self): + experiment = self.exp_base_config["exp_1"]["experiment"] + experiment["bucket_val"] = "conversion_pixel_id" + experiment["variants"] = [ + {"range_start": 0.0, "range_end": 1.0, "name": "control"}, + {"range_start": 1.0, "range_end": 1.0, "name": "enabled"}, + ] + experiment["overrides"] = [ + {"enabled": {"EQ": {"field": "conversion_pixel_id", "values": ["t2_1"]}}} + ] + + for method_name in ( + "get_variant_for_identifier", + "get_variant_for_identifier_without_expose", + "get_all_variants_for_identifier_without_expose", + ): + for pixel_id, expected in (("t2_1", "enabled"), ("t2_2", "control")): + with self.subTest(method=method_name, pixel_id=pixel_id): + self.event_logger.reset_mock() + with create_temp_config_file(self.exp_base_config) as f: + decider = setup_decider(f, self.dc, self.mock_span, self.event_logger) + method = getattr(decider, method_name) + kwargs = { + "identifier": pixel_id, + "identifier_type": "conversion_pixel_id", + } + if method_name == "get_all_variants_for_identifier_without_expose": + variants = method(**kwargs) + self.assertEqual(len(variants), 1) + variant = variants[0]["name"] + else: + variant = method(experiment_name="exp_1", **kwargs) + self.assertEqual(variant, expected) + if method_name == "get_variant_for_identifier": + self.event_logger.log.assert_called_once() + self.assertEqual( + self.event_logger.log.call_args.kwargs["conversion_pixel_id"], + pixel_id, + ) + else: + self.event_logger.log.assert_not_called() + def test_get_variant_for_identifier_bogus_identifier_type(self): identifier = "anything" identifier_type = "blah" @@ -1087,7 +1149,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', 'subreddit_id', 'ad_account_id', 'business_id']." + "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url', 'subreddit_id', 'ad_account_id', 'business_id', 'conversion_pixel_id']." in x.getMessage() for x in captured.records ) @@ -1277,7 +1339,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', 'subreddit_id', 'ad_account_id', 'business_id']." + "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url', 'subreddit_id', 'ad_account_id', 'business_id', 'conversion_pixel_id']." in x.getMessage() for x in captured.records ) @@ -1691,7 +1753,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', 'subreddit_id', 'ad_account_id', 'business_id']." + "\"blah\" is not one of supported \"identifier_type\": ['user_id', 'device_id', 'canonical_url', 'subreddit_id', 'ad_account_id', 'business_id', 'conversion_pixel_id']." in x.getMessage() for x in captured.records )