Merge pull request #12 from waveaccounting/DLF-371-duplicate-sixpack-…

…entries

[JIRA](DLF-371) Fix the duplicate SixpackParticipant records

djsixpack/__init__.py

@@ -1 +1 @@
-__version__ = "1.1.2-wave"
+__version__ = "1.1.3-wave"

djsixpack/djsixpack.py

@@ -75,6 +75,12 @@ def get_participant_bucket(self):
             participant = SixpackParticipant.objects.get(unique_attr=self.client_id, experiment_name=self._get_experiment_name())
         except SixpackParticipant.DoesNotExist:
             return None
+        except SixpackParticipant.MultipleObjectsReturned:
+            # clean up duplicate entries
+            duplicates = SixpackParticipant.objects.filter(unique_attr=self.client_id, experiment_name=self._get_experiment_name())
+            for dup in duplicates[1:]:
+                dup.delete()
+            return duplicates[0].bucket
         return participant.bucket
 
     def participate(self, force=None, user_agent=None, ip_address=None, prefetch=False, bucket=None):

djsixpack/tests/class_tests.py

@@ -3,6 +3,8 @@
 from django.test import TestCase
 
 from djsixpack.djsixpack import SixpackTest
+from djsixpack.models import SixpackParticipant
+from djsixpack.tests.factories import SixpackParticipantFactory
 
 
 class ClientIdTest(TestCase):
@@ -158,7 +160,7 @@ class DefaultTest(SixpackTest):
         )
         self.assertEqual(
             sp_mock.Session.return_value.participate.call_args_list,
-            [call('default', ('FIRST', 'SECOND'), force=None, prefetch=False)]
+            [call('default', ('FIRST', 'SECOND'), force=None, bucket=None, prefetch=False)]
         )
 
         class ConvertTest(TestCase):
@@ -217,3 +219,45 @@ class AltTest(SixpackTest):
 
         self.assertTrue(hasattr(AltTest, 'FIRST'))
         self.assertTrue(hasattr(AltTest, 'SECOND'))
+
+
+class GetParticipantBucketTest(TestCase):
+
+    def test_get_bucket__single_record(self):
+        mock_user = Mock(pk=10)
+
+        class GetBucketTest(SixpackTest):
+            alternatives = ('FIRST', 'SECOND')
+
+        expt = GetBucketTest(mock_user)
+        expt.local = True
+        expt.participate(bucket='FIRST')
+
+        self.assertEquals(expt.get_participant_bucket(), 'FIRST')
+
+    def test_get_bucket__zero_records(self):
+        mock_user = Mock(pk=10)
+
+        class GetBucketTest(SixpackTest):
+            alternatives = ('FIRST', 'SECOND')
+
+        expt = GetBucketTest(mock_user)
+        self.assertIsNone(expt.get_participant_bucket())
+
+    def test_get_bucket__multiple_records(self):
+        mock_user = Mock(pk=10)
+
+        class GetBucketTest(SixpackTest):
+            alternatives = ('FIRST', 'SECOND')
+
+        expt = GetBucketTest(mock_user)
+        expt.local = True
+        expt.participate(bucket='FIRST')
+
+        # Insert an extra record to mess things up.
+        SixpackParticipantFactory(experiment_name='get_bucket', unique_attr=10, bucket='SECOND')
+
+        self.assertEquals(expt.get_participant_bucket(), 'FIRST')
+
+        record_count = SixpackParticipant.objects.filter(experiment_name='get_bucket', unique_attr=10).count()
+        self.assertEquals(record_count, 1)

djsixpack/tests/factories.py

@@ -0,0 +1,12 @@
+from factory import Factory
+
+from djsixpack.models import SixpackParticipant
+
+
+class SixpackParticipantFactory(Factory):
+
+    FACTORY_FOR = SixpackParticipant
+
+    experiment_name = 'Science Experiment'
+    unique_attr = 1
+    bucket = 'Bucket A'

requirements.txt

@@ -0,0 +1 @@
+factory-boy==1.1.3