optimizely/optimizely.py -> Removed experiment_id and variation_id from legacy apis.

optimizely/project_config.py -> Enhanced comments for clarity.
tests/test_user_context.py -> Updated test assertions for experiments.

Author: FarhanAnjum-opti

optimizely/optimizely.py

@@ -340,9 +340,7 @@ def _get_feature_variable_for_type(
         user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
 
         decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
-        experiment_id = decision.experiment.id if decision.experiment else None
-        variation_id = decision.variation.id if decision.variation else None
-    
+
         if decision.variation:
 
             feature_enabled = decision.variation.featureEnabled
@@ -388,8 +386,6 @@ def _get_feature_variable_for_type(
                 'variable_value': actual_value,
                 'variable_type': variable_type,
                 'source_info': source_info,
-                'experiment_id': experiment_id,
-                'variation_id': variation_id
             },
         )
         return actual_value
@@ -431,9 +427,7 @@ def _get_all_feature_variables_for_type(
         user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
 
         decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
-        experiment_id = decision.experiment.id if decision.experiment else None
-        variation_id = decision.variation.id if decision.variation else None
-    
+
         if decision.variation:
 
             feature_enabled = decision.variation.featureEnabled
@@ -486,8 +480,6 @@ def _get_all_feature_variables_for_type(
                 'variable_values': all_variables,
                 'source': decision.source,
                 'source_info': source_info,
-                'experiment_id': experiment_id,
-                'variation_id': variation_id
             },
         )
         return all_variables
@@ -654,10 +646,7 @@ def get_variation(
             decision_notification_type = enums.DecisionNotificationTypes.FEATURE_TEST
         else:
             decision_notification_type = enums.DecisionNotificationTypes.AB_TEST
-            
-        experiment_id = experiment.id if experiment else None
-        variation_id = variation.id if variation else None
-        
+
         self.notification_center.send_notifications(
             enums.NotificationTypes.DECISION,
             decision_notification_type,
@@ -666,8 +655,6 @@ def get_variation(
             {
                 'experiment_key': experiment_key,
                 'variation_key': variation_key,
-                'experiment_id': experiment_id,
-                'variation_id': variation_id
             },
         )
 
@@ -754,8 +741,6 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona
                 'feature_enabled': feature_enabled,
                 'source': decision.source,
                 'source_info': source_info,
-                'experiment_id': decision.experiment.id,
-                'variation_id': decision.variation.id
             },
         )
 
@@ -1220,15 +1205,19 @@ def _create_optimizely_decision(
             if flag_decision is not None and flag_decision.variation is not None
             else None
         )
-        
-        rollout_id = feature_flag.rolloutId if decision_source == DecisionSources.ROLLOUT else None
-        experiment_id = project_config.get_experiment_id_by_key_or_rollout_id(rule_key, rollout_id)
+
+        rollout_id = None
+        if decision_source == DecisionSources.ROLLOUT and feature_flag is not None:
+            rollout_id = feature_flag.rolloutId
+        experiment_id = None
+        if rule_key is not None:
+            experiment_id = project_config.get_experiment_id_by_key_or_rollout_id(rule_key, rollout_id)
         variation_id = None
-        if variation_key:
+        if experiment_id and variation_key:
             variation = project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key)
             if variation:
-                variation_id = variation.id     
-        
+                variation_id = variation.id
+
         # Send notification
         self.notification_center.send_notifications(
             enums.NotificationTypes.DECISION,

optimizely/project_config.py

@@ -722,8 +722,8 @@ def get_experiment_id_by_key_or_rollout_id(self, key: str, rollout_id: Optional[
         Retrieves the experiment ID associated with a given rule key or a specific rollout.
 
         Args:
-            key: The key associated with the experiment rule.
-            rollout_id: The ID of the rollout to search if the key is not found.
+            key: The key associated with the experiment rule. It can be experiment key or rule key.
+            rollout_id: The ID of the rollout to be searched if the key is not found in the experiment key map.
 
         Returns:
             Optional[str]: The experiment ID if found, otherwise None.
@@ -738,9 +738,9 @@ def get_experiment_id_by_key_or_rollout_id(self, key: str, rollout_id: Optional[
         if rollout_id:
             rollout = self.get_rollout_from_id(rollout_id)
             if rollout:
-                for experiment in rollout.experiments:
-                    experiment = entities.Experiment(**experiment)
+                for experiment_data in rollout.experiments:
+                    experiment = entities.Experiment(**experiment_data)
                     if experiment.key == key:
-                        return experiment.id  
+                        return experiment.id
 
         return None

tests/test_user_context.py

@@ -283,6 +283,8 @@ def test_decide__feature_test(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': True,
                 'variables': expected.variables,
+                'experiment_id': mock_experiment.id,
+                'variation_id': mock_variation.id
             },
         )
 
@@ -391,6 +393,24 @@ def test_decide_feature_rollout(self):
 
         self.compare_opt_decisions(expected, actual)
 
+        # assert event count
+        self.assertEqual(1, mock_send_event.call_count)
+
+        # assert event payload
+        expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
+        expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key)
+        mock_send_event.assert_called_with(
+            project_config,
+            expected_experiment,
+            expected_var,
+            expected.flag_key,
+            expected.rule_key,
+            'rollout',
+            expected.enabled,
+            'test_user',
+            user_attributes
+        )
+
         # assert notification count
         self.assertEqual(1, mock_broadcast_decision.call_count)
 
@@ -408,27 +428,11 @@ def test_decide_feature_rollout(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': True,
                 'variables': expected.variables,
+                'experiment_id': expected_experiment.id,
+                'variation_id': expected_var.id
             },
         )
 
-        # assert event count
-        self.assertEqual(1, mock_send_event.call_count)
-
-        # assert event payload
-        expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
-        expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key)
-        mock_send_event.assert_called_with(
-            project_config,
-            expected_experiment,
-            expected_var,
-            expected.flag_key,
-            expected.rule_key,
-            'rollout',
-            expected.enabled,
-            'test_user',
-            user_attributes
-        )
-
     def test_decide_feature_rollout__send_flag_decision_false(self):
         opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
         project_config = opt_obj.config_manager.get_config()
@@ -467,6 +471,8 @@ def test_decide_feature_rollout__send_flag_decision_false(self):
         self.assertEqual(1, mock_broadcast_decision.call_count)
 
         # assert notification
+        expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
+        expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key)
         mock_broadcast_decision.assert_called_with(
             enums.NotificationTypes.DECISION,
             'flag',
@@ -480,6 +486,8 @@ def test_decide_feature_rollout__send_flag_decision_false(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': False,
                 'variables': expected.variables,
+                'experiment_id': expected_experiment.id,
+                'variation_id': expected_var.id
             },
         )
 
@@ -549,7 +557,9 @@ def test_decide_feature_null_variation(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': True,
                 'variables': expected.variables,
-            },
+                'experiment_id': None,
+                'variation_id': None
+            }
         )
 
         # assert event count
@@ -632,6 +642,8 @@ def test_decide_feature_null_variation__send_flag_decision_false(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': False,
                 'variables': expected.variables,
+                'experiment_id': None,
+                'variation_id': None
             },
         )
 
@@ -701,6 +713,8 @@ def test_decide__option__disable_decision_event(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': False,
                 'variables': expected.variables,
+                'experiment_id': mock_experiment.id,
+                'variation_id': mock_variation.id,
             },
         )
 
@@ -773,6 +787,8 @@ def test_decide__default_option__disable_decision_event(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': False,
                 'variables': expected.variables,
+                'experiment_id': mock_experiment.id,
+                'variation_id': mock_variation.id
             },
         )
 
@@ -834,6 +850,8 @@ def test_decide__option__exclude_variables(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': True,
                 'variables': expected.variables,
+                'experiment_id': mock_experiment.id,
+                'variation_id': mock_variation.id,
             },
         )
 
@@ -948,6 +966,8 @@ def test_decide__option__enabled_flags_only(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': True,
                 'variables': expected.variables,
+                'experiment_id': expected_experiment.id,
+                'variation_id': expected_var.id,
             },
         )
 
@@ -1006,7 +1026,7 @@ def test_decide__default_options__with__options(self):
             enabled=True,
             variables=expected_variables,
             flag_key='test_feature_in_experiment',
-            user_context=user_context
+            user_context=user_context,
         )
 
         self.compare_opt_decisions(expected, actual)
@@ -1025,6 +1045,8 @@ def test_decide__default_options__with__options(self):
                 'reasons': expected.reasons,
                 'decision_event_dispatched': False,
                 'variables': expected.variables,
+                'experiment_id': mock_experiment.id,
+                'variation_id': mock_variation.id
             },
         )
 
@@ -1490,6 +1512,9 @@ def test_should_return_valid_decision_after_setting_and_removing_forced_decision
                      'User "test_user" is in variation "control" of experiment test_experiment.']
         )
 
+        expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
+        expected_var = project_config.get_variation_from_key('test_experiment', expected.variation_key)
+
         # assert notification count
         self.assertEqual(1, mock_broadcast_decision.call_count)
 
@@ -1507,12 +1532,11 @@ def test_should_return_valid_decision_after_setting_and_removing_forced_decision
                 'reasons': expected.reasons,
                 'decision_event_dispatched': True,
                 'variables': expected.variables,
+                'experiment_id': expected_experiment.id,
+                'variation_id': expected_var.id
             },
         )
 
-        expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
-        expected_var = project_config.get_variation_from_key('test_experiment', expected.variation_key)
-
         mock_send_event.assert_called_with(
             project_config,
             expected_experiment,