Merge branch 'release/0.11.1'

Populate static segments from database

CHANGES

@@ -1,3 +1,21 @@
+0.11.1
+==================
+ - Populate entirely static segments from registered Users not active Sessions
+
+0.11.0
+==================
+ - Bug Fix: Query rule should not be static
+ - Enable retrieval of user data for static rules through csv download
+
+0.10.9
+==================
+ - Bug Fix: Display the number of users in a static segment on dashboard
+
+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.11.1'
 
 # The full version, including alpha/beta/rc tags.
-release = '0.10.7'
+release = '0.11.1'
 
 # 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.11.1
 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.11.1',
     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()
 
 

src/wagtail_personalisation/admin_urls.py

@@ -13,4 +13,6 @@ urlpatterns = [
         views.copy_page_view, name='copy_page'),
     url(r'^segment/toggle_segment_view/$',
         views.toggle_segment_view, name='toggle_segment_view'),
+    url(r'^segment/users/(?P<segment_id>[0-9]+)$',
+        views.segment_user_data, name='segment_user_data'),
 ]

src/wagtail_personalisation/forms.py

@@ -2,12 +2,10 @@ from __future__ import absolute_import, unicode_literals
 
 from datetime import datetime
 from importlib import import_module
-from itertools import takewhile
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import AnonymousUser
-from django.contrib.sessions.models import Session
 from django.contrib.staticfiles.templatetags.staticfiles import static
 from django.test.client import RequestFactory
 from django.utils.lru_cache import lru_cache
@@ -87,7 +85,7 @@ class SegmentAdminForm(WagtailAdminModelForm):
         if not self.instance.is_static:
             self.instance.count = 0
 
-        if is_new:
+        if is_new and self.instance.is_static and not self.instance.all_rules_static:
             rules = [
                 form.instance for formset in self.formsets.values()
                 for form in formset
@@ -108,23 +106,24 @@ class SegmentAdminForm(WagtailAdminModelForm):
 
             users_to_add = []
             users_to_exclude = []
-            sessions = Session.objects.iterator()
-            take_session = takewhile(
-                lambda x: instance.count == 0 or len(users_to_add) <= instance.count,
-                sessions
-            )
-            for session in take_session:
-                session_data = session.get_decoded()
-                user = user_from_data(session_data.get('_auth_user_id'))
-                if user.is_authenticated():
-                    request.user = user
-                    request.session = SessionStore(session_key=session.session_key)
-                    passes = adapter._test_rules(instance.get_rules(), request, instance.match_any)
-                    if passes and instance.randomise_into_segment():
-                        users_to_add.append(user)
-                    elif passes:
-                        users_to_exclude.append(user)
 
+            User = get_user_model()
+            users = User.objects.filter(is_active=True, is_staff=False)
+
+            matched_count = 0
+            for user in users.iterator():
+                request.user = user
+                passes = adapter._test_rules(instance.get_rules(), request, instance.match_any)
+                if passes:
+                    matched_count += 1
+                    if instance.count == 0 or len(users_to_add) <= instance.count:
+                        if instance.randomise_into_segment():
+                            users_to_add.append(user)
+                        else:
+                            users_to_exclude.append(user)
+
+            instance.matched_users_count = matched_count
+            instance.matched_count_updated_at = datetime.now()
             instance.static_users.add(*users_to_add)
             instance.excluded_users.add(*users_to_exclude)
 

src/wagtail_personalisation/rules.py

@@ -2,17 +2,23 @@ from __future__ import absolute_import, unicode_literals
 
 import re
 from datetime import datetime
+from importlib import import_module
 
 from django.apps import apps
+from django.conf import settings
+from django.contrib.sessions.models import Session
 from django.db import models
 from django.template.defaultfilters import slugify
 from django.utils.encoding import force_text, python_2_unicode_compatible
 from django.utils.translation import ugettext_lazy as _
+from django.test.client import RequestFactory
 from modelcluster.fields import ParentalKey
 from user_agents import parse
 from wagtail.wagtailadmin.edit_handlers import (
     FieldPanel, FieldRowPanel, PageChooserPanel)
 
+SessionStore = import_module(settings.SESSION_ENGINE).SessionStore
+
 
 @python_2_unicode_compatible
 class AbstractBaseRule(models.Model):
@@ -220,18 +226,37 @@ class VisitCountRule(AbstractBaseRule):
     class Meta:
         verbose_name = _('Visit count Rule')
 
+    def _get_user_session(self, user):
+        sessions = Session.objects.iterator()
+        for session in sessions:
+            session_data = session.get_decoded()
+            if session_data.get('_auth_user_id') == str(user.id):
+                return SessionStore(session_key=session.session_key)
+        return SessionStore()
+
     def test_user(self, request, user=None):
+        # Local import for cyclic import
+        from wagtail_personalisation.adapters import (
+            get_segment_adapter, SessionSegmentsAdapter, SEGMENT_ADAPTER_CLASS)
+
         if user:
-            # This rule currently does not support testing a user directly
-            # TODO: Make this test a user directly when the rule uses
-            # historical data
+            # Create a fake request so we can use the adapter
+            request = RequestFactory().get('/')
+            request.user = user
+
+            # If we're using the session adapter check for an active session
+            if SEGMENT_ADAPTER_CLASS == SessionSegmentsAdapter:
+                request.session = self._get_user_session(user)
+            else:
+                request.session = SessionStore()
+
+        elif not request:
+            # Return false if we don't have a user or a request
             return False
+
         operator = self.operator
         segment_count = self.count
 
-        # Local import for cyclic import
-        from wagtail_personalisation.adapters import get_segment_adapter
-
         adapter = get_segment_adapter(request)
 
         visit_count = adapter.get_visit_count(self.counted_page)
@@ -257,6 +282,28 @@ class VisitCountRule(AbstractBaseRule):
             ),
         }
 
+    def get_column_header(self):
+        return "Visit count - %s" % self.counted_page
+
+    def get_user_info_string(self, user):
+        # Local import for cyclic import
+        from wagtail_personalisation.adapters import (
+            get_segment_adapter, SessionSegmentsAdapter, SEGMENT_ADAPTER_CLASS)
+
+        # Create a fake request so we can use the adapter
+        request = RequestFactory().get('/')
+        request.user = user
+
+        # If we're using the session adapter check for an active session
+        if SEGMENT_ADAPTER_CLASS == SessionSegmentsAdapter:
+            request.session = self._get_user_session(user)
+        else:
+            request.session = SessionStore()
+
+        adapter = get_segment_adapter(request)
+        visit_count = adapter.get_visit_count(self.counted_page)
+        return str(visit_count)
+
 
 class QueryRule(AbstractBaseRule):
     """Query rule to segment users based on matching queries.
@@ -266,7 +313,6 @@ class QueryRule(AbstractBaseRule):
 
     """
     icon = 'fa-link'
-    static = True
 
     parameter = models.SlugField(_("The query parameter to search for"),
                                  max_length=20)
@@ -281,13 +327,7 @@ class QueryRule(AbstractBaseRule):
     class Meta:
         verbose_name = _('Query Rule')
 
-    def test_user(self, request, user=None):
-        if user:
-            # This rule currently does not support testing a user directly
-            # TODO: Make this test a user directly if/when the rule uses
-            # historical data
-            return False
-
+    def test_user(self, request):
         return request.GET.get(self.parameter, '') == self.value
 
     def description(self):

src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html

@@ -38,11 +38,11 @@
                                                 <li class="stat_card">
                                                     {% trans "This segment is Static" %}
                                                     <span class="icon icon-fa-user">
-                                                        {{ segment.sessions.count|localize }}
-                                                        {% if segment.sessions.count < segment.count %}
+                                                        {{ segment.static_users.count|localize }}
+                                                        {% if segment.static_users.count < segment.count %}
                                                         / {{ segment.count }} {% trans "member" %}{{ segment.count|pluralize }}
                                                         {% else %}
-                                                            {% trans "member" %}{{ segment.sessions.count|pluralize }}
+                                                            {% trans "member" %}{{ segment.count|pluralize }}
                                                         {% endif %}
                                                     </span>
                                                 </li>
@@ -103,6 +103,9 @@
                                                 <li><a href="{% url 'segment:toggle' segment.pk %}" title="{% trans "Disable this segment" %}">disable</a></li>
                                             {% endif %}
                                             <li><a href="{% url 'wagtail_personalisation_segment_modeladmin_edit' segment.pk %}" title="{% trans "Configure this segment" %}">configure this</a></li>
+                                            {% if segment.is_static %}
+                                                <li><a href="{% url 'segment:segment_user_data' segment.pk %}" title="{% trans "Download user info" %}">download users csv</a></li>
+                                            {% endif %}
                                         </ul>
                                         {% endif %}
                                     </div>

src/wagtail_personalisation/views.py

@@ -1,8 +1,9 @@
 from __future__ import absolute_import, unicode_literals
+import csv
 
 from django import forms
 from django.core.urlresolvers import reverse
-from django.http import HttpResponseForbidden, HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseForbidden, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.utils.translation import ugettext_lazy as _
 from wagtail.contrib.modeladmin.options import ModelAdmin, modeladmin_register
@@ -139,3 +140,32 @@ def copy_page_view(request, page_id, segment_id):
         return HttpResponseRedirect(edit_url)
 
     return HttpResponseForbidden()
+
+
+# CSV download views
+def segment_user_data(request, segment_id):
+    if request.user.has_perm('wagtailadmin.access_admin'):
+        segment = get_object_or_404(Segment, pk=segment_id)
+
+        response = HttpResponse(content_type='text/csv; charset=utf-8')
+        response['Content-Disposition'] = \
+            'attachment;filename=segment-%s-users.csv' % str(segment_id)
+
+        headers = ['Username']
+        for rule in segment.get_rules():
+            if rule.static:
+                headers.append(rule.get_column_header())
+
+        writer = csv.writer(response)
+        writer.writerow(headers)
+
+        for user in segment.static_users.all():
+            row = [user.username]
+            for rule in segment.get_rules():
+                if rule.static:
+                    row.append(rule.get_user_info_string(user))
+            writer.writerow(row)
+
+        return response
+
+    return HttpResponseForbidden()

tests/unit/test_factories.py

@@ -45,4 +45,3 @@ def test_query_rule_create():
 
     assert query_rule.parameter == 'query'
     assert query_rule.value == 'value'
-    assert query_rule.static

tests/unit/test_rules_visitcount.py

@@ -1,5 +1,8 @@
 import pytest
 
+from tests.factories.rule import VisitCountRuleFactory
+from tests.factories.segment import SegmentFactory
+
 
 @pytest.mark.django_db
 def test_visit_count(site, client):
@@ -20,3 +23,50 @@ def test_visit_count(site, client):
     visit_count = client.session['visit_count']
     assert visit_count[0]['count'] == 2
     assert visit_count[1]['count'] == 1
+
+
+@pytest.mark.django_db
+def test_visit_count_call_test_user_with_user(site, client, user):
+    segment = SegmentFactory(name='VisitCount')
+    rule = VisitCountRuleFactory(counted_page=site.root_page, segment=segment)
+
+    session = client.session
+    session['visit_count'] = [{'path': '/', 'count': 2}]
+    session.save()
+    client.force_login(user)
+
+    assert rule.test_user(None, user)
+
+
+@pytest.mark.django_db
+def test_visit_count_call_test_user_with_user_or_request_fails(site, client, user):
+    segment = SegmentFactory(name='VisitCount')
+    rule = VisitCountRuleFactory(counted_page=site.root_page, segment=segment)
+
+    session = client.session
+    session['visit_count'] = [{'path': '/', 'count': 2}]
+    session.save()
+    client.force_login(user)
+
+    assert not rule.test_user(None)
+
+
+@pytest.mark.django_db
+def test_get_column_header(site):
+    segment = SegmentFactory(name='VisitCount')
+    rule = VisitCountRuleFactory(counted_page=site.root_page, segment=segment)
+
+    assert rule.get_column_header() == 'Visit count - Test page'
+
+
+@pytest.mark.django_db
+def test_get_user_info_string_returns_count(site, client, user):
+    segment = SegmentFactory(name='VisitCount')
+    rule = VisitCountRuleFactory(counted_page=site.root_page, segment=segment)
+
+    session = client.session
+    session['visit_count'] = [{'path': '/', 'count': 2}]
+    session.save()
+    client.force_login(user)
+
+    assert rule.get_user_info_string(user) == '2'

tests/unit/test_static_dynamic_segments.py

@@ -8,8 +8,7 @@ from django.forms.models import model_to_dict
 from tests.factories.segment import SegmentFactory
 from wagtail_personalisation.forms import SegmentAdminForm
 from wagtail_personalisation.models import Segment
-from wagtail_personalisation.rules import (AbstractBaseRule, TimeRule,
-                                           VisitCountRule)
+from wagtail_personalisation.rules import TimeRule, VisitCountRule
 
 
 def form_with_data(segment, *rules):
@@ -36,22 +35,18 @@ def form_with_data(segment, *rules):
 
 
 @pytest.mark.django_db
-def test_session_added_to_static_segment_at_creation(site, client, user):
-    session = client.session
-    session.save()
-    client.force_login(user)
-    client.get(site.root_page.url)
-
+def test_user_added_to_static_segment_at_creation(site, user, mocker):
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
     rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
     instance = form.save()
 
     assert user in instance.static_users.all()
 
 
 @pytest.mark.django_db
-def test_anonymous_user_not_added_to_static_segment_at_creation(site, client):
+def test_anonymous_user_not_added_to_static_segment_at_creation(site, client, mocker):
     session = client.session
     session.save()
     client.get(site.root_page.url)
@@ -59,43 +54,32 @@ def test_anonymous_user_not_added_to_static_segment_at_creation(site, client):
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
     rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules')
     instance = form.save()
 
     assert not instance.static_users.all()
+    assert mock_test_rule.call_count == 0
 
 
 @pytest.mark.django_db
-def test_match_any_correct_populates(site, client, django_user_model):
+def test_match_any_correct_populates(site, django_user_model, mocker):
     user = django_user_model.objects.create(username='first')
-    session = client.session
-    client.force_login(user)
-    client.get(site.root_page.url)
-
     other_user = django_user_model.objects.create(username='second')
-    client.cookies.clear()
-    second_session = client.session
     other_page = site.root_page.get_last_child()
-    client.force_login(other_user)
-    client.get(other_page.url)
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC, match_any=True)
     rule_1 = VisitCountRule(counted_page=site.root_page)
     rule_2 = VisitCountRule(counted_page=other_page)
     form = form_with_data(segment, rule_1, rule_2)
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', side_effect=[True, False, True, False])
     instance = form.save()
 
-    assert session.session_key != second_session.session_key
     assert user in instance.static_users.all()
     assert other_user in instance.static_users.all()
 
 
 @pytest.mark.django_db
-def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, client, user):
-    session = client.session
-    session.save()
-    client.force_login(user)
-    client.get(site.root_page.url)
-
+def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, mocker):
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1)
     static_rule = VisitCountRule(counted_page=site.root_page)
     non_static_rule = TimeRule(
@@ -103,9 +87,12 @@ def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, client,
         end_time=datetime.time(23, 59, 59),
     )
     form = form_with_data(segment, static_rule, non_static_rule)
+
+    mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules')
     instance = form.save()
 
     assert not instance.static_users.all()
+    assert mock_test_rule.call_count == 0
 
 
 @pytest.mark.django_db
@@ -181,12 +168,7 @@ def test_session_not_added_to_static_segment_after_full(site, client, django_use
 
 
 @pytest.mark.django_db
-def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site, user):
-    session = client.session
-    session.save()
-    client.force_login(user)
-    client.get(site.root_page.url)
-
+def test_sessions_not_added_to_static_segment_if_rule_not_static(mocker):
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1)
     rule = TimeRule(
         start_time=datetime.time(0, 0, 0),
@@ -194,26 +176,27 @@ def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site, u
         segment=segment,
     )
     form = form_with_data(segment, rule)
+    mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules')
     instance = form.save()
 
     assert not instance.static_users.all()
+    assert mock_test_rule.call_count == 0
 
 
 @pytest.mark.django_db
 def test_does_not_calculate_the_segment_again(site, client, mocker, user):
-    session = client.session
-    session.save()
-    client.force_login(user)
-    client.get(site.root_page.url)
-
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=2)
     rule = VisitCountRule(counted_page=site.root_page, segment=segment)
     form = form_with_data(segment, rule)
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
     instance = form.save()
 
     assert user in instance.static_users.all()
 
     mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules')
+    session = client.session
+    session.save()
+    client.force_login(user)
     client.get(site.root_page.url)
     assert mock_test_rule.call_count == 0
 
@@ -332,6 +315,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 +325,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 +333,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
@@ -388,16 +373,12 @@ def test_always_in_segment_if_percentage_is_100(site, client, mocker, user):
 
 
 @pytest.mark.django_db
-def test_not_added_to_static_segment_at_creation_if_random_above_percent(site, client, mocker, user):
-    session = client.session
-    session.save()
-    client.force_login(user)
-    client.get(site.root_page.url)
-
+def test_not_added_to_static_segment_at_creation_if_random_above_percent(site, mocker, user):
     mocker.patch('random.randint', return_value=41)
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC, randomisation_percent=40)
     rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
     instance = form.save()
 
     assert user not in instance.static_users.all()
@@ -405,16 +386,12 @@ def test_not_added_to_static_segment_at_creation_if_random_above_percent(site, c
 
 
 @pytest.mark.django_db
-def test_added_to_static_segment_at_creation_if_random_below_percent(site, client, mocker, user):
-    session = client.session
-    session.save()
-    client.force_login(user)
-    client.get(site.root_page.url)
-
+def test_added_to_static_segment_at_creation_if_random_below_percent(site, mocker, user):
     mocker.patch('random.randint', return_value=39)
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC, randomisation_percent=40)
     rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
     instance = form.save()
 
     assert user in instance.static_users.all()
@@ -446,7 +423,31 @@ def test_rules_check_skipped_if_user_in_excluded(site, client, mocker, user):
 
 
 @pytest.mark.django_db
-def test_matched_user_count_added_to_segment_at_creation(site, client, mocker, django_user_model):
+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, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
@@ -454,6 +455,7 @@ def test_matched_user_count_added_to_segment_at_creation(site, client, mocker, d
     rule = VisitCountRule()
 
     form = form_with_data(segment, rule)
+    form.instance.type = Segment.TYPE_STATIC
     mock_test_user = mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
     instance = form.save()
 
@@ -462,152 +464,97 @@ def test_matched_user_count_added_to_segment_at_creation(site, client, mocker, d
 
 
 @pytest.mark.django_db
-def test_count_users_matching_static_rules(site, client, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
+def test_count_users_matching_static_rules(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    rule = TestStaticRule()
+    rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
 
     assert form.count_matching_users([rule], True) is 2
 
 
 @pytest.mark.django_db
-def test_count_matching_users_excludes_staff(site, client, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
+def test_count_matching_users_excludes_staff(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second', is_staff=True)
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    rule = TestStaticRule()
+    rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mock_test_user = mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
 
     assert form.count_matching_users([rule], True) is 1
+    assert mock_test_user.call_count == 1
 
 
 @pytest.mark.django_db
-def test_count_matching_users_excludes_inactive(site, client, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
+def test_count_matching_users_excludes_inactive(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second', is_active=False)
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    rule = TestStaticRule()
+    rule = VisitCountRule(counted_page=site.root_page)
     form = form_with_data(segment, rule)
+    mock_test_user = mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True)
 
     assert form.count_matching_users([rule], True) is 1
+    assert mock_test_user.call_count == 1
 
 
 @pytest.mark.django_db
-def test_count_matching_users_only_counts_static_rules(site, client, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
+def test_count_matching_users_only_counts_static_rules(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    rule = TestStaticRule()
+    rule = TimeRule(
+        start_time=datetime.time(0, 0, 0),
+        end_time=datetime.time(23, 59, 59),
+        segment=segment,
+    )
     form = form_with_data(segment, rule)
+    mock_test_user = mocker.patch('wagtail_personalisation.rules.TimeRule.test_user')
 
     assert form.count_matching_users([rule], True) is 0
+    assert mock_test_user.call_count == 0
 
 
 @pytest.mark.django_db
-def test_count_matching_users_handles_match_any(site, client, django_user_model):
-    class TestStaticRuleFirst(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            if user.username == 'first':
-                return True
-            return False
-
-    class TestStaticRuleSecond(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            if user.username == 'second':
-                return True
-            return False
-
+def test_count_matching_users_handles_match_any(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    first_rule = TestStaticRuleFirst()
-    second_rule = TestStaticRuleSecond()
+    first_rule = VisitCountRule(counted_page=site.root_page)
+    other_page = site.root_page.get_last_child()
+    second_rule = VisitCountRule(counted_page=other_page)
     form = form_with_data(segment, first_rule, second_rule)
 
+    mock_test_user = mocker.patch(
+        'wagtail_personalisation.rules.VisitCountRule.test_user',
+        side_effect=[True, False, True, False])
+
     assert form.count_matching_users([first_rule, second_rule], True) is 2
+    mock_test_user.call_count == 4
 
 
 @pytest.mark.django_db
-def test_count_matching_users_handles_match_all(site, client, django_user_model):
-    class TestStaticRuleFirst(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            if user.username == 'first':
-                return True
-            return False
-
-    class TestStaticRuleContainsS(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            if 's' in user.username:
-                return True
-            return False
-
+def test_count_matching_users_handles_match_all(site, client, mocker, django_user_model):
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    first_rule = TestStaticRuleFirst()
-    s_rule = TestStaticRuleContainsS()
-    form = form_with_data(segment, first_rule, s_rule)
+    first_rule = VisitCountRule(counted_page=site.root_page)
+    other_page = site.root_page.get_last_child()
+    second_rule = VisitCountRule(counted_page=other_page)
+    form = form_with_data(segment, first_rule, second_rule)
 
-    assert form.count_matching_users([first_rule, s_rule], False) is 1
+    mock_test_user = mocker.patch(
+        'wagtail_personalisation.rules.VisitCountRule.test_user',
+        side_effect=[True, True, False, True])
+
+    assert form.count_matching_users([first_rule, second_rule], False) is 1
+    mock_test_user.call_count == 4

tests/unit/test_views.py

@@ -0,0 +1,55 @@
+import pytest
+
+from django.contrib.auth.models import Permission
+from django.core.urlresolvers import reverse
+from wagtail_personalisation.models import Segment
+from wagtail_personalisation.rules import VisitCountRule
+
+
+@pytest.mark.django_db
+def test_segment_user_data_view_requires_admin_access(site, client, django_user_model):
+    user = django_user_model.objects.create(username='first')
+
+    segment = Segment(type=Segment.TYPE_STATIC, count=1)
+    segment.save()
+
+    client.force_login(user)
+    url = reverse('segment:segment_user_data', args=(segment.id,))
+    response = client.get(url)
+
+    assert response.status_code == 302
+    assert response.url == '/admin/login/?next=%s' % url
+
+
+@pytest.mark.django_db
+def test_segment_user_data_view(site, client, mocker, django_user_model):
+    user1 = django_user_model.objects.create(username='first')
+    user2 = django_user_model.objects.create(username='second')
+    admin_user = django_user_model.objects.create(username='admin')
+    permission = Permission.objects.get(codename='access_admin')
+    admin_user.user_permissions.add(permission)
+
+    segment = Segment(type=Segment.TYPE_STATIC, count=1)
+    segment.save()
+    segment.static_users.add(user1)
+    segment.static_users.add(user2)
+
+    rule1 = VisitCountRule(counted_page=site.root_page, segment=segment)
+    rule2 = VisitCountRule(counted_page=site.root_page.get_last_child(),
+                           segment=segment)
+    rule1.save()
+    rule2.save()
+
+    mocker.patch('wagtail_personalisation.rules.VisitCountRule.get_user_info_string',
+                 side_effect=[3, 9, 0, 1])
+
+    client.force_login(admin_user)
+    response = client.get(
+        reverse('segment:segment_user_data', args=(segment.id,)))
+
+    assert response.status_code == 200
+    data_lines = response.content.decode().split("\n")
+
+    assert data_lines[0] == 'Username,Visit count - Test page,Visit count - Regular page\r'
+    assert data_lines[1] == 'first,3,9\r'
+    assert data_lines[2] == 'second,0,1\r'