Important bug fixes

experiments/apps.py

@@ -6,8 +6,8 @@ class ExperimentsConfig(AppConfig):
     label = 'experiments'
 
     def ready(self):
-        from django.contrib.auth.signals import user_logged_in
-        from experiments.signal_handlers import transfer_enrollments_to_user
+        from django.contrib.auth.signals import user_logged_in, user_logged_out
+        from experiments.signal_handlers import transfer_enrollments_to_user, handle_user_logged_out
 
         user_logged_in.connect(transfer_enrollments_to_user, dispatch_uid="experiments_user_logged_in")
-
+        user_logged_out.connect(handle_user_logged_out, dispatch_uid="experiments_user_logged_out")

experiments/signal_handlers.py

@@ -1,7 +1,13 @@
-from experiments.utils import participant
+from experiments.utils import participant, clear_participant_cache
 
 
 def transfer_enrollments_to_user(sender, request, user, **kwargs):
     anon_user = participant(session=request.session)
     authenticated_user = participant(user=user)
     authenticated_user.incorporate(anon_user)
+
+    clear_participant_cache(request)
+
+
+def handle_user_logged_out(sender, request, user, **kwargs):
+    clear_participant_cache(request)

experiments/tests/test_webuser.py

@@ -1,17 +1,21 @@
 from __future__ import absolute_import
 
 from datetime import timedelta
+from django.http import HttpResponse
 
 from django.test import TestCase
 from django.test.client import RequestFactory
 from django.contrib.auth.models import AnonymousUser
 from django.contrib.auth import get_user_model
 from django.contrib.sessions.backends.db import SessionStore as DatabaseSession
 from django.utils import timezone
+from experiments import conf
 
 from experiments.experiment_counters import ExperimentCounter
-from experiments.models import Experiment, ENABLED_STATE
+from experiments.middleware import ExperimentsRetentionMiddleware
+from experiments.models import Experiment, ENABLED_STATE, Enrollment
 from experiments.conf import CONTROL_GROUP, VISIT_PRESENT_COUNT_GOAL, VISIT_NOT_PRESENT_COUNT_GOAL
+from experiments.signal_handlers import transfer_enrollments_to_user
 from experiments.utils import participant
 
 from mock import patch
@@ -169,3 +173,73 @@ def setUp(self):
         self.user.save()
 
         self.experiment_user = participant(user=self.user)
+
+
+class ParticipantCacheTestCase(TestCase):
+    def setUp(self):
+        self.experiment = Experiment.objects.create(name='test_experiment1', state=ENABLED_STATE)
+        self.experiment_counter = ExperimentCounter()
+
+    def tearDown(self):
+        self.experiment_counter.delete(self.experiment)
+
+    def test_transfer_enrollments(self):
+        User = get_user_model()
+        user = User.objects.create(username='test')
+        request = request_factory.get('/')
+        request.session = DatabaseSession()
+        participant(request).enroll('test_experiment1', ['alternative'])
+        request.user = user
+        transfer_enrollments_to_user(None, request, user)
+        # the call to the middleware will set last_seen on the experiment
+        # if the participant cache hasn't been wiped appropriately then the
+        # session experiment user will be impacted instead of the authenticated
+        # experiment user
+        ExperimentsRetentionMiddleware().process_response(request, HttpResponse())
+        self.assertIsNotNone(Enrollment.objects.all()[0].last_seen)
+
+
+class ConfirmHumanTestCase(TestCase):
+    def setUp(self):
+        self.experiment = Experiment.objects.create(name='test_experiment1', state=ENABLED_STATE)
+        self.experiment_counter = ExperimentCounter()
+        self.experiment_user = participant(session=DatabaseSession())
+        self.alternative = self.experiment_user.enroll(self.experiment.name, ['alternative'])
+        self.experiment_user.goal('my_goal')
+
+    def tearDown(self):
+        self.experiment_counter.delete(self.experiment)
+
+    def test_confirm_human_updates_experiment(self):
+        self.assertIn('experiments_goals', self.experiment_user.session)
+        self.assertEqual(self.experiment_counter.participant_count(self.experiment, self.alternative), 0)
+        self.assertEqual(self.experiment_counter.goal_count(self.experiment, self.alternative, 'my_goal'), 0)
+        self.experiment_user.confirm_human()
+        self.assertNotIn('experiments_goals', self.experiment_user.session)
+        self.assertEqual(self.experiment_counter.participant_count(self.experiment, self.alternative), 1)
+        self.assertEqual(self.experiment_counter.goal_count(self.experiment, self.alternative, 'my_goal'), 1)
+
+    def test_confirm_human_called_twice(self):
+        """
+        Ensuring that counters aren't incremented twice
+        """
+        self.assertEqual(self.experiment_counter.participant_count(self.experiment, self.alternative), 0)
+        self.assertEqual(self.experiment_counter.goal_count(self.experiment, self.alternative, 'my_goal'), 0)
+        self.experiment_user.confirm_human()
+        self.experiment_user.confirm_human()
+        self.assertEqual(self.experiment_counter.participant_count(self.experiment, self.alternative), 1)
+        self.assertEqual(self.experiment_counter.goal_count(self.experiment, self.alternative, 'my_goal'), 1)
+
+    def test_confirm_human_sets_session(self):
+        self.assertFalse(self.experiment_user.session.get(conf.CONFIRM_HUMAN_SESSION_KEY, False))
+        self.experiment_user.confirm_human()
+        self.assertTrue(self.experiment_user.session.get(conf.CONFIRM_HUMAN_SESSION_KEY, False))
+
+    def test_session_already_confirmed(self):
+        """
+        Testing that confirm_human works even if code outside of django-experiments updates the key
+        """
+        self.experiment_user.session[conf.CONFIRM_HUMAN_SESSION_KEY] = True
+        self.experiment_user.confirm_human()
+        self.assertEqual(self.experiment_counter.participant_count(self.experiment, self.alternative), 1)
+        self.assertEqual(self.experiment_counter.goal_count(self.experiment, self.alternative, 'my_goal'), 1)

experiments/tests/test_webuser_incorporate.py

@@ -1,10 +1,14 @@
-from django.test import TestCase
+from django.http import HttpResponse
+from django.test import TestCase, RequestFactory
 from django.utils.unittest import TestSuite
 from django.contrib.sessions.backends.db import SessionStore as DatabaseSession
+from experiments import conf
 
 from experiments.experiment_counters import ExperimentCounter
-from experiments.utils import DummyUser, SessionUser, AuthenticatedUser
-from experiments.models import Experiment, ENABLED_STATE
+from experiments.middleware import ExperimentsRetentionMiddleware
+from experiments.signal_handlers import transfer_enrollments_to_user
+from experiments.utils import DummyUser, SessionUser, AuthenticatedUser, participant
+from experiments.models import Experiment, ENABLED_STATE, Enrollment
 
 from django.contrib.auth import get_user_model
 
@@ -72,3 +76,52 @@ def setUp(self):
             self.incorporated = incorporated(False)
     InstantiatedTestCase.__name__ = "WebUserIncorporateTestCase_into_%s_from_%s" % (incorporating.__name__, incorporated.__name__)
     return InstantiatedTestCase
+
+
+class IncorporateTestCase(TestCase):
+    def setUp(self):
+        self.experiment = Experiment.objects.create(name=EXPERIMENT_NAME, state=ENABLED_STATE)
+        self.experiment_counter = ExperimentCounter()
+
+        User = get_user_model()
+        self.user = User.objects.create(username='incorporate_user')
+        self.user.is_confirmed_human = True
+
+        request_factory = RequestFactory()
+        self.request = request_factory.get('/')
+        self.request.session = DatabaseSession()
+        participant(self.request).confirm_human()
+
+    def tearDown(self):
+        self.experiment_counter.delete(self.experiment)
+
+    def _login(self):
+        self.request.user = self.user
+        transfer_enrollments_to_user(None, self.request, self.user)
+
+    def test_visit_incorporate(self):
+        alternative = participant(self.request).enroll(self.experiment.name, ['alternative'])
+
+        ExperimentsRetentionMiddleware().process_response(self.request, HttpResponse())
+
+        self.assertEqual(
+            dict(self.experiment_counter.participant_goal_frequencies(self.experiment,
+                                                                      alternative,
+                                                                      participant(self.request)._participant_identifier()))[conf.VISIT_NOT_PRESENT_COUNT_GOAL],
+            1
+        )
+
+        self.assertFalse(Enrollment.objects.all().exists())
+        self._login()
+
+        self.assertTrue(Enrollment.objects.all().exists())
+        self.assertIsNotNone(Enrollment.objects.all()[0].last_seen)
+        self.assertEqual(
+            dict(self.experiment_counter.participant_goal_frequencies(self.experiment,
+                                                                      alternative,
+                                                                      participant(self.request)._participant_identifier()))[conf.VISIT_NOT_PRESENT_COUNT_GOAL],
+            1
+        )
+        self.assertEqual(self.experiment_counter.goal_count(self.experiment, alternative, conf.VISIT_NOT_PRESENT_COUNT_GOAL), 1)
+        self.assertEqual(self.experiment_counter.participant_count(self.experiment, alternative), 1)
+

experiments/utils.py

@@ -17,7 +17,10 @@
 
 logger = logging.getLogger('experiments')
 
+
 def participant(request=None, session=None, user=None):
+    # This caches the experiment user on the request object because AuthenticatedUser can involve database lookups that
+    # it caches. Signals are attached to login/logout to clear the cache using clear_participant_cache
     if request and hasattr(request, '_experiments_user'):
         return request._experiments_user
     else:
@@ -27,6 +30,11 @@ def participant(request=None, session=None, user=None):
         return result
 
 
+def clear_participant_cache(request):
+    if hasattr(request, '_experiments_user'):
+        del request._experiments_user
+
+
 def _get_participant(request, session, user):
     if request and hasattr(request, 'user') and not user:
         user = request.user
@@ -45,6 +53,7 @@ def _get_participant(request, session, user):
     else:
         return DummyUser()
 
+
 EnrollmentData = namedtuple('EnrollmentData', ['experiment', 'alternative', 'enrollment_date', 'last_seen'])
 
 
@@ -335,11 +344,8 @@ def _set_enrollment(self, experiment, alternative, enrollment_date=None, last_se
         user_enrolled.send(self, experiment=experiment.name, alternative=alternative, user=None, session=self.session)
 
     def confirm_human(self):
-        if self.session.get(conf.CONFIRM_HUMAN_SESSION_KEY, False):
-            return
-
         self.session[conf.CONFIRM_HUMAN_SESSION_KEY] = True
-        logger.info(json.dumps({'type':'confirm_human', 'participant': self._participant_identifier()}))
+        logger.info(json.dumps({'type': 'confirm_human', 'participant': self._participant_identifier()}))
 
         # Replay enrollments
         for enrollment in self._get_all_enrollments():
@@ -385,6 +391,7 @@ def _cancel_enrollment(self, experiment):
             self.experiment_counter.remove_participant(experiment, alternative, self._participant_identifier())
             enrollments = self.session.get('experiments_enrollments', None)
             del enrollments[experiment.name]
+            self.session['experiments_enrollments'] = enrollments
 
     def _experiment_goal(self, experiment, alternative, goal_name, count):
         if self._is_verified_human():
@@ -393,8 +400,7 @@ def _experiment_goal(self, experiment, alternative, goal_name, count):
             goals = self.session.get('experiments_goals', [])
             goals.append((experiment.name, alternative, goal_name, count))
             self.session['experiments_goals'] = goals
-            logger.info(json.dumps({'type':'goal_hit_unconfirmed', 'goal': goal_name, 'goal_count': count, 'experiment': experiment.name, 'alternative': alternative, 'participant': self._participant_identifier()}))
-
+            logger.info(json.dumps({'type': 'goal_hit_unconfirmed', 'goal': goal_name, 'goal_count': count, 'experiment': experiment.name, 'alternative': alternative, 'participant': self._participant_identifier()}))
 
     def _set_last_seen(self, experiment, last_seen):
         enrollments = self.session.get('experiments_enrollments', {})