Merge branch 'release/0.11.0'

CHANGES

@@ -1,3 +1,12 @@
+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

docs/conf.py

@@ -55,10 +55,10 @@ author = 'Lab Digital BV'
 # built documents.
 #
 # The short X.Y version.
-version = '0.10.8'
+version = '0.11.0'
 
 # The full version, including alpha/beta/rc tags.
-release = '0.10.8'
+release = '0.11.0'
 
 # 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.8
+current_version = 0.11.0
 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.8',
+    version='0.11.0',
     description='A forked version of Wagtail add-on for showing personalized content',
     author='Praekelt.org',
     author_email='dev@praekeltfoundation.org',

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/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
+            # Create a fake request so we can use the adapter
-            # TODO: Make this test a user directly when the rule uses
+            request = RequestFactory().get('/')
-            # historical data
+            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):
+    def test_user(self, request):
-        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
-
         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 }}
+                                                        {{ segment.static_users.count|localize }}
-                                                        {% if segment.sessions.count < segment.count %}
+                                                        {% 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,
+from wagtail_personalisation.rules import TimeRule, VisitCountRule
-                                           VisitCountRule)
 
 
 def form_with_data(segment, *rules):
@@ -488,152 +487,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):
+def test_count_users_matching_static_rules(site, client, mocker, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
     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):
+def test_count_matching_users_excludes_staff(site, client, mocker, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
     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):
+def test_count_matching_users_excludes_inactive(site, client, mocker, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        static = True
-
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
     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):
+def test_count_matching_users_only_counts_static_rules(site, client, mocker, django_user_model):
-    class TestStaticRule(AbstractBaseRule):
-        class Meta:
-            app_label = 'wagtail_personalisation'
-
-        def test_user(self, request, user):
-            return True
-
     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):
+def test_count_matching_users_handles_match_any(site, client, mocker, 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
-
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    first_rule = TestStaticRuleFirst()
+    first_rule = VisitCountRule(counted_page=site.root_page)
-    second_rule = TestStaticRuleSecond()
+    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):
+def test_count_matching_users_handles_match_all(site, client, mocker, 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
-
     django_user_model.objects.create(username='first')
     django_user_model.objects.create(username='second')
 
     segment = SegmentFactory.build(type=Segment.TYPE_STATIC)
-    first_rule = TestStaticRuleFirst()
+    first_rule = VisitCountRule(counted_page=site.root_page)
-    s_rule = TestStaticRuleContainsS()
+    other_page = site.root_page.get_last_child()
-    form = form_with_data(segment, first_rule, s_rule)
+    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'