Merge branch 'release/0.10.8'

Fix sampling for Dynamic segments

CHANGES

@@ -1,3 +1,8 @@
+0.10.8
+==================
+ - Don't add users to exclude list for dynamic segments
+ - Store segments a user is excluded from in the session
+
 0.10.7
 ==================
  - Bug Fix: Ensure static segment members are show the survey immediately

docs/conf.py

@@ -55,10 +55,10 @@ author = 'Lab Digital BV'
 # built documents.
 #
 # The short X.Y version.
-version = '0.10.7'
+version = '0.10.8'
 
 # The full version, including alpha/beta/rc tags.
-release = '0.10.7'
+release = '0.10.8'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

setup.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.10.7
+current_version = 0.10.8
 commit = true
 tag = true
 tag_name = {new_version}

setup.py

@@ -32,7 +32,7 @@ with open('README.rst') as fh:
 
 setup(
     name='wagtail-personalisation-molo',
-    version='0.10.7',
+    version='0.10.8',
     description='A forked version of Wagtail add-on for showing personalized content',
     author='Praekelt.org',
     author_email='dev@praekeltfoundation.org',

src/wagtail_personalisation/adapters.py

@@ -66,17 +66,21 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter):
         self.request.session.setdefault('segments', [])
         self._segment_cache = None
 
-    def get_segments(self):
+    def get_segments(self, key="segments"):
         """Return the persistent segments stored in the request session.
 
+        :param key: The key under which the segments are stored
+        :type key: String
         :returns: The segments in the request session
         :rtype: list of wagtail_personalisation.models.Segment or empty list
 
         """
-        if self._segment_cache is not None:
+        if key == "segments" and self._segment_cache is not None:
             return self._segment_cache
 
-        raw_segments = self.request.session['segments']
+        if key not in self.request.session:
+            return []
+        raw_segments = self.request.session[key]
         segment_ids = [segment['id'] for segment in raw_segments]
 
         segments = (
@@ -86,14 +90,17 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter):
             .in_bulk(segment_ids))
 
         retval = [segments[pk] for pk in segment_ids if pk in segments]
-        self._segment_cache = retval
+        if key == "segments":
+            self._segment_cache = retval
         return retval
 
-    def set_segments(self, segments):
+    def set_segments(self, segments, key="segments"):
         """Set the currently active segments
 
         :param segments: The segments to set for the current request
         :type segments: list of wagtail_personalisation.models.Segment
+        :param key: The key under which to store the segments. Optional
+        :type key: String
 
         """
         cache_segments = []
@@ -108,8 +115,9 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter):
             serialized_segments.append(serialized)
             segment_ids.add(segment.pk)
 
-        self.request.session['segments'] = serialized_segments
-        self._segment_cache = cache_segments
+        self.request.session[key] = serialized_segments
+        if key == "segments":
+            self._segment_cache = cache_segments
 
     def get_segment_by_id(self, segment_id):
         """Find and return a single segment from the request session.
@@ -171,13 +179,15 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter):
         rule_models = AbstractBaseRule.get_descendant_models()
 
         current_segments = self.get_segments()
+        excluded_segments = self.get_segments("excluded_segments")
 
         # Run tests on all remaining enabled segments to verify applicability.
         additional_segments = []
         for segment in enabled_segments:
             if segment.is_static and segment.static_users.filter(id=self.request.user.id).exists():
                 additional_segments.append(segment)
-            elif segment.excluded_users.filter(id=self.request.user.id).exists():
+            elif (segment.excluded_users.filter(id=self.request.user.id).exists() or
+                    segment in excluded_segments):
                 continue
             elif not segment.is_static or not segment.is_full:
                 segment_rules = []
@@ -193,10 +203,13 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter):
                             segment.static_users.add(self.request.user)
                     additional_segments.append(segment)
                 elif result:
-                    if self.request.user.is_authenticated():
+                    if segment.is_static and self.request.user.is_authenticated():
                         segment.excluded_users.add(self.request.user)
+                    else:
+                        excluded_segments += [segment]
 
         self.set_segments(current_segments + additional_segments)
+        self.set_segments(excluded_segments, "excluded_segments")
         self.update_visit_count()
 
 

tests/unit/test_static_dynamic_segments.py

@@ -332,6 +332,7 @@ def test_offered_dynamic_segment_if_random_is_below_percentage(site, client, moc
     session.save()
     client.get(site.root_page.url)
 
+    assert len(client.session['excluded_segments']) == 0
     assert instance.id == client.session['segments'][0]['id']
 
 
@@ -341,7 +342,7 @@ def test_not_offered_dynamic_segment_if_random_is_above_percentage(site, client,
                                    randomisation_percent=40)
     rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
-    form.save()
+    instance = form.save()
 
     mocker.patch('random.randint', return_value=41)
     session = client.session
@@ -349,6 +350,7 @@ def test_not_offered_dynamic_segment_if_random_is_above_percentage(site, client,
     client.get(site.root_page.url)
 
     assert len(client.session['segments']) == 0
+    assert instance.id == client.session['excluded_segments'][0]['id']
 
 
 @pytest.mark.django_db
@@ -445,6 +447,30 @@ def test_rules_check_skipped_if_user_in_excluded(site, client, mocker, user):
     assert user in instance.excluded_users.all()
 
 
+@pytest.mark.django_db
+def test_rules_check_skipped_if_dynamic_segment_in_excluded(site, client, mocker, user):
+    segment = SegmentFactory.build(type=Segment.TYPE_DYNAMIC,
+                                   randomisation_percent=100)
+    rule = VisitCountRule(counted_page=site.root_page)
+    form = form_with_data(segment, rule)
+    instance = form.save()
+    instance.persistent = True
+    instance.save()
+
+    session = client.session
+    session['excluded_segments'] = [{'id': instance.pk}]
+    session.save()
+
+    mock_test_rule = mocker.patch(
+        'wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules')
+
+    client.force_login(user)
+    client.get(site.root_page.url)
+
+    assert mock_test_rule.call_count == 0
+    assert len(client.session['segments']) == 0
+
+
 @pytest.mark.django_db
 def test_matched_user_count_added_to_segment_at_creation(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')