From 675d219f1fdd44328250d67f61042ae1984dfcff Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Tue, 17 Oct 2017 16:57:07 +0100 Subject: [PATCH 01/97] Add the logic for static segments --- src/wagtail_personalisation/adapters.py | 27 +++++-- .../0013_add_dynamic_static_to_segment.py | 31 ++++++++ src/wagtail_personalisation/models.py | 73 ++++++++++++++++++ src/wagtail_personalisation/rules.py | 2 + tests/unit/test_static_dynamic_segments.py | 75 +++++++++++++++++++ 5 files changed, 200 insertions(+), 8 deletions(-) create mode 100644 src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py create mode 100644 tests/unit/test_static_dynamic_segments.py diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 933f744a..5bde2353 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -3,6 +3,7 @@ from django.conf import settings from django.db.models import F from django.utils.module_loading import import_string +from django.utils import timezone from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import AbstractBaseRule @@ -174,15 +175,25 @@ def refresh(self): # Run tests on all remaining enabled segments to verify applicability. additional_segments = [] for segment in enabled_segments: - segment_rules = [] - for rule_model in rule_models: - segment_rules.extend(rule_model.objects.filter(segment=segment)) - - result = self._test_rules(segment_rules, self.request, - match_any=segment.match_any) - - if result: + if segment.is_static and self.request.session in segment.sessions.all(): additional_segments.append(segment) + elif not segment.is_static or not segment.is_full: + segment_rules = [] + for rule_model in rule_models: + segment_rules.extend(rule_model.objects.filter(segment=segment)) + + result = self._test_rules(segment_rules, self.request, + match_any=segment.match_any) + + if result and segment.is_static and not segment.is_full: + session = self.request.session.model.objects.get( + session_key=self.request.session.session_key, + expire_date__gt=timezone.now(), + ) + segment.sessions.add(session) + + if result: + additional_segments.append(segment) self.set_segments(current_segments + additional_segments) self.update_visit_count() diff --git a/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py b/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py new file mode 100644 index 00000000..7ab4e114 --- /dev/null +++ b/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2017-10-17 11:18 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('sessions', '0001_initial'), + ('wagtail_personalisation', '0012_remove_personalisablepagemetadata_is_segmented'), + ] + + operations = [ + migrations.AddField( + model_name='segment', + name='count', + field=models.PositiveSmallIntegerField(default=0, help_text='If this number is set for a static segment users will be added to the set until the number is reached. After this no more users will be added.'), + ), + migrations.AddField( + model_name='segment', + name='sessions', + field=models.ManyToManyField(to='sessions.Session'), + ), + migrations.AddField( + model_name='segment', + name='type', + field=models.CharField(choices=[('dynamic', 'Dynamic'), ('static', 'Static')], default='dynamic', help_text='The users in a dynamic segment will change as more or less users meet the rules specified in the segment. Static segments will contain the members that existed at creation.', max_length=20), + ), + ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 82719d77..3df7731b 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -1,9 +1,19 @@ from __future__ import absolute_import, unicode_literals +from importlib import import_module + +from django import forms +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.db import models, transaction from django.template.defaultfilters import slugify +from django.test.client import RequestFactory +from django.utils import timezone from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property +from django.utils.lru_cache import lru_cache from django.utils.translation import ugettext_lazy as _ from modelcluster.models import ClusterableModel from wagtail.wagtailadmin.edit_handlers import ( @@ -14,11 +24,23 @@ from wagtail_personalisation.utils import count_active_days +SessionStore = import_module(settings.SESSION_ENGINE).SessionStore + + class SegmentQuerySet(models.QuerySet): def enabled(self): return self.filter(status=self.model.STATUS_ENABLED) +@lru_cache(maxsize=1000) +def user_from_data(user_id): + User = get_user_model() + try: + return User.objects.get(id=user_id) + except User.DoesNotExist: + return AnonymousUser + + @python_2_unicode_compatible class Segment(ClusterableModel): """The segment model.""" @@ -30,6 +52,14 @@ class Segment(ClusterableModel): (STATUS_DISABLED, _('Disabled')), ) + TYPE_DYNAMIC = 'dynamic' + TYPE_STATIC = 'static' + + TYPE_CHOICES = ( + (TYPE_DYNAMIC, _('Dynamic')), + (TYPE_STATIC, _('Static')), + ) + name = models.CharField(max_length=255) create_date = models.DateTimeField(auto_now_add=True) edit_date = models.DateTimeField(auto_now=True) @@ -44,6 +74,24 @@ class Segment(ClusterableModel): default=False, help_text=_("Should the segment match all the rules or just one of them?") ) + type = models.CharField( + max_length=20, + choices=TYPE_CHOICES, + default=TYPE_DYNAMIC, + help_text=_( + "The users in a dynamic segment will change as more or less users meet " + "the rules specified in the segment. Static segments will contain the " + "members that existed at creation." + ) + ) + count = models.PositiveSmallIntegerField( + default=0, + help_text=_( + "If this number is set for a static segment users will be added to the " + "set until the number is reached. After this no more users will be added." + ) + ) + sessions = models.ManyToManyField(Session) objects = SegmentQuerySet.as_manager() @@ -56,6 +104,8 @@ def __init__(self, *args, **kwargs): FieldPanel('persistent'), ]), FieldPanel('match_any'), + FieldPanel('type', widget=forms.RadioSelect), + FieldPanel('count'), ], heading="Segment"), MultiFieldPanel([ InlinePanel( @@ -70,6 +120,14 @@ def __init__(self, *args, **kwargs): def __str__(self): return self.name + @property + def is_static(self): + return self.type == self.TYPE_STATIC + + @property + def is_full(self): + return self.sessions.count() >= self.count + def encoded_name(self): """Return a string with a slug for the segment.""" return slugify(self.name.lower()) @@ -106,6 +164,21 @@ def toggle(self, save=True): if save: self.save() + def save(self, *args, **kwargs): + super(Segment, self).save(*args, **kwargs) + + if self.is_static: + request = RequestFactory().get('/') + for session in Session.objects.filter( + expire_date__gt=timezone.now(), + ).iterator(): + session_data = session.get_decoded() + user = user_from_data(session_data.get('_auth_id')) + request.user = user + request.session = SessionStore(session_key=session.session_key) + if all(rule.test_user(request) for rule in self.get_rules() if rule.static): + self.sessions.add(session) + class PersonalisablePageMetadata(ClusterableModel): """The personalisable page model. Allows creation of variants with linked diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index 5d5b94e9..ee98e36b 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -18,6 +18,7 @@ class AbstractBaseRule(models.Model): """Base for creating rules to segment users with.""" icon = 'fa-circle-o' + static = False segment = ParentalKey( 'wagtail_personalisation.Segment', @@ -190,6 +191,7 @@ class VisitCountRule(AbstractBaseRule): """ icon = 'fa-calculator' + static = True OPERATOR_CHOICES = ( ('more_than', _("More than")), diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py new file mode 100644 index 00000000..53e30b19 --- /dev/null +++ b/tests/unit/test_static_dynamic_segments.py @@ -0,0 +1,75 @@ +from __future__ import absolute_import, unicode_literals + +import datetime + +import pytest + +from tests.factories.segment import SegmentFactory +from wagtail_personalisation.models import Segment +from wagtail_personalisation.rules import TimeRule, VisitCountRule + + +@pytest.mark.django_db +def test_session_added_to_static_segment_at_creation(rf, site, client): + session = client.session + session.save() + client.get(site.root_page.url) + + segment = SegmentFactory(type=Segment.TYPE_STATIC) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + + assert session.session_key in segment.sessions.values_list('session_key', flat=True) + + +@pytest.mark.django_db +def test_session_not_added_to_static_segment_after_creation(rf, site, client): + segment = SegmentFactory(type=Segment.TYPE_STATIC) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + + session = client.session + session.save() + client.get(site.root_page.url) + + assert not segment.sessions.all() + + +@pytest.mark.django_db +def test_session_added_to_static_segment_after_creation(rf, site, client): + segment = SegmentFactory(type=Segment.TYPE_STATIC, count=1) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + + session = client.session + session.save() + client.get(site.root_page.url) + + assert session.session_key in segment.sessions.values_list('session_key', flat=True) + + +@pytest.mark.django_db +def test_session_not_added_to_static_segment_after_full(rf, site, client): + segment = SegmentFactory(type=Segment.TYPE_STATIC, count=1) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + + session = client.session + session.save() + client.get(site.root_page.url) + + second_session = client.session + second_session.create() + client.get(site.root_page.url) + + assert session.session_key != second_session.session_key + assert segment.sessions.count() == 1 + assert session.session_key in segment.sessions.values_list('session_key', flat=True) + assert second_session.session_key not in segment.sessions.values_list('session_key', flat=True) + + +@pytest.mark.django_db +def test_sessions_not_added_to_static_segment_if_rule_not_static(): + segment = SegmentFactory(type=Segment.TYPE_STATIC) + TimeRule.objects.create( + start_time=datetime.time(8, 0, 0), + end_time=datetime.time(23, 0, 0), + segment=segment) + + assert not segment.sessions.all() From 8c96fffd4ed1a53cdc68703c487e02f7697a0a9d Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Tue, 17 Oct 2017 17:35:57 +0100 Subject: [PATCH 02/97] Ensure that the session is checked correctly --- setup.py | 1 + src/wagtail_personalisation/adapters.py | 2 +- tests/unit/test_static_dynamic_segments.py | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 52d12f77..f9f7c770 100644 --- a/setup.py +++ b/setup.py @@ -18,6 +18,7 @@ 'pytest-cov==2.4.0', 'pytest-django==3.1.2', 'pytest-sugar==0.7.1', + 'pytest-mock==1.6.3', 'pytest==3.1.0', 'wagtail_factories==0.3.0', ] diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 5bde2353..16260ec9 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -175,7 +175,7 @@ def refresh(self): # Run tests on all remaining enabled segments to verify applicability. additional_segments = [] for segment in enabled_segments: - if segment.is_static and self.request.session in segment.sessions.all(): + if segment.is_static and self.request.session.session_key in segment.sessions.values_list('session_key', flat=True): additional_segments.append(segment) elif not segment.is_static or not segment.is_full: segment_rules = [] diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 53e30b19..7815d8e0 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -73,3 +73,17 @@ def test_sessions_not_added_to_static_segment_if_rule_not_static(): segment=segment) assert not segment.sessions.all() + + +@pytest.mark.django_db +def test_does_not_calculate_the_segment_again(rf, site, client, mocker): + session = client.session + session.save() + client.get(site.root_page.url) + + segment = SegmentFactory(type=Segment.TYPE_STATIC, count=2) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + + mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules') + client.get(site.root_page.url) + assert mock_test_rule.call_count == 0 From aa2a239aecfd1d54e2fc097d13a67a0d20b76873 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Thu, 19 Oct 2017 17:19:53 +0100 Subject: [PATCH 03/97] Update the dashboard to display information about static segments --- frontend/scss/dashboard.scss | 10 ++++---- .../static/css/dashboard.css | 2 +- .../static/css/dashboard.css.map | 2 +- .../segment/dashboard.html | 23 +++++++++++++++---- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/frontend/scss/dashboard.scss b/frontend/scss/dashboard.scss index 6f5147c5..bb49e691 100644 --- a/frontend/scss/dashboard.scss +++ b/frontend/scss/dashboard.scss @@ -86,6 +86,11 @@ padding: 0; margin: 0; list-style: none; + .stat_card { + display: inline-block; + margin-bottom: 5px; + margin-right: 10px; + } } .block_container .block span.icon::before { @@ -93,11 +98,6 @@ vertical-align: bottom; } - .block_container .block .inspect_container .inspect li { - display: inline-block; - margin-bottom: 5px; - } - .block_container .block .inspect_container .inspect li span { display: block; font-size: 20px; diff --git a/src/wagtail_personalisation/static/css/dashboard.css b/src/wagtail_personalisation/static/css/dashboard.css index b1dc0edc..37315097 100644 --- a/src/wagtail_personalisation/static/css/dashboard.css +++ b/src/wagtail_personalisation/static/css/dashboard.css @@ -1,2 +1,2 @@ -.nice-padding{padding-left:50px;padding-right:50px}.block_container{display:block;margin-top:30px}.block_container .block{display:block;float:left;-webkit-box-sizing:border-box;box-sizing:border-box;position:relative;width:calc(50% - 10px);min-height:216px;padding:10px 20px;margin-bottom:20px;border:1px solid #d9d9d9;border-radius:3px;background-color:#fff;-webkit-box-shadow:0 1px 3px transparent,0 1px 2px transparent;box-shadow:0 1px 3px transparent,0 1px 2px transparent;-webkit-transition:border .3s cubic-bezier(.25,.8,.25,1),-webkit-box-shadow .3s cubic-bezier(.25,.8,.25,1);transition:border .3s cubic-bezier(.25,.8,.25,1),-webkit-box-shadow .3s cubic-bezier(.25,.8,.25,1);transition:box-shadow .3s cubic-bezier(.25,.8,.25,1),border .3s cubic-bezier(.25,.8,.25,1);transition:box-shadow .3s cubic-bezier(.25,.8,.25,1),border .3s cubic-bezier(.25,.8,.25,1),-webkit-box-shadow .3s cubic-bezier(.25,.8,.25,1);cursor:pointer}.block_container .block--disabled .inspect_container,.block_container .block--disabled h2{opacity:.5}.block_container .block h2{display:inline-block;width:auto}.block_container .block:nth-child(odd){margin-right:20px}.block_container .block .block_actions{list-style:none;margin:0;padding:0}.block_container .block .block_actions li{float:left;margin-right:10px}.block_container .block .block_actions li:last-child{margin-right:0}.block_container .block.suggestion{border:1px dashed #d9d9d9}.block_container .block:hover{border:1px solid #fff;-webkit-box-shadow:0 3px 6px rgba(0,0,0,.16),0 3px 6px rgba(0,0,0,.23);box-shadow:0 3px 6px rgba(0,0,0,.16),0 3px 6px rgba(0,0,0,.23)}@media (max-width:699px){.block_container .block{width:100%;margin-right:0}}.block_container .block .inspect_container{display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-orient:horizontal;-webkit-box-direction:normal;-ms-flex-direction:row;flex-direction:row;-ms-flex-wrap:nowrap;flex-wrap:nowrap;-webkit-box-pack:justify;-ms-flex-pack:justify;justify-content:space-between;-webkit-box-align:stretch;-ms-flex-align:stretch;align-items:stretch;margin-bottom:10px}.block_container .block .inspect_container .inspect{display:block;float:left;width:calc(50% - 10px);padding:0;margin:0;list-style:none}.block_container .block span.icon:before{margin-right:.3em;vertical-align:bottom}.block_container .block .inspect_container .inspect li{display:inline-block;margin-bottom:5px}.block_container .block .inspect_container .inspect li span{display:block;font-size:20px;font-weight:700;margin:5px 0;overflow-wrap:break-word}.block_container .block .inspect_container .inspect li pre{position:relative;-webkit-box-sizing:border-box;box-sizing:border-box;width:auto;background-color:#eee;border:1px solid #ccc;margin:5px 0 5px 21px;padding:2px 5px;word-wrap:break-word;word-break:break-all;border-radius:3px}.block_container .block.suggestion .suggestive_text{display:block;position:absolute;width:calc(100% - 40px);text-align:center;top:50%;-webkit-transform:translateY(-50%);transform:translateY(-50%);color:#d9d9d9;font-size:20px;font-weight:100} +.nice-padding{padding-left:50px;padding-right:50px}.block_container{display:block;margin-top:30px}.block_container .block{display:block;float:left;-webkit-box-sizing:border-box;box-sizing:border-box;position:relative;width:calc(50% - 10px);min-height:216px;padding:10px 20px;margin-bottom:20px;border:1px solid #d9d9d9;border-radius:3px;background-color:#fff;-webkit-box-shadow:0 1px 3px transparent,0 1px 2px transparent;box-shadow:0 1px 3px transparent,0 1px 2px transparent;-webkit-transition:border .3s cubic-bezier(.25,.8,.25,1),-webkit-box-shadow .3s cubic-bezier(.25,.8,.25,1);transition:border .3s cubic-bezier(.25,.8,.25,1),-webkit-box-shadow .3s cubic-bezier(.25,.8,.25,1);transition:box-shadow .3s cubic-bezier(.25,.8,.25,1),border .3s cubic-bezier(.25,.8,.25,1);transition:box-shadow .3s cubic-bezier(.25,.8,.25,1),border .3s cubic-bezier(.25,.8,.25,1),-webkit-box-shadow .3s cubic-bezier(.25,.8,.25,1);cursor:pointer}.block_container .block--disabled .inspect_container,.block_container .block--disabled h2{opacity:.5}.block_container .block h2{display:inline-block;width:auto}.block_container .block:nth-child(odd){margin-right:20px}.block_container .block .block_actions{list-style:none;margin:0;padding:0}.block_container .block .block_actions li{float:left;margin-right:10px}.block_container .block .block_actions li:last-child{margin-right:0}.block_container .block.suggestion{border:1px dashed #d9d9d9}.block_container .block:hover{border:1px solid #fff;-webkit-box-shadow:0 3px 6px rgba(0,0,0,.16),0 3px 6px rgba(0,0,0,.23);box-shadow:0 3px 6px rgba(0,0,0,.16),0 3px 6px rgba(0,0,0,.23)}@media (max-width:699px){.block_container .block{width:100%;margin-right:0}}.block_container .block .inspect_container{display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-orient:horizontal;-webkit-box-direction:normal;-ms-flex-direction:row;flex-direction:row;-ms-flex-wrap:nowrap;flex-wrap:nowrap;-webkit-box-pack:justify;-ms-flex-pack:justify;justify-content:space-between;-webkit-box-align:stretch;-ms-flex-align:stretch;align-items:stretch;margin-bottom:10px}.block_container .block .inspect_container .inspect{display:block;float:left;width:calc(50% - 10px);padding:0;margin:0;list-style:none}.block_container .block .inspect_container .inspect .stat_card{display:inline-block;margin-bottom:5px;margin-right:10px}.block_container .block span.icon:before{margin-right:.3em;vertical-align:bottom}.block_container .block .inspect_container .inspect li span{display:block;font-size:20px;font-weight:700;margin:5px 0;overflow-wrap:break-word}.block_container .block .inspect_container .inspect li pre{position:relative;-webkit-box-sizing:border-box;box-sizing:border-box;width:auto;background-color:#eee;border:1px solid #ccc;margin:5px 0 5px 21px;padding:2px 5px;word-wrap:break-word;word-break:break-all;border-radius:3px}.block_container .block.suggestion .suggestive_text{display:block;position:absolute;width:calc(100% - 40px);text-align:center;top:50%;-webkit-transform:translateY(-50%);transform:translateY(-50%);color:#d9d9d9;font-size:20px;font-weight:100} /*# sourceMappingURL=dashboard.css.map*/ \ No newline at end of file diff --git a/src/wagtail_personalisation/static/css/dashboard.css.map b/src/wagtail_personalisation/static/css/dashboard.css.map index b51f9209..6c3c9c8d 100644 --- a/src/wagtail_personalisation/static/css/dashboard.css.map +++ b/src/wagtail_personalisation/static/css/dashboard.css.map @@ -1 +1 @@ -{"version":3,"sources":["webpack:///./scss/dashboard.scss"],"names":[],"mappings":"AAAA,cACI,kBACA,kBAAmB,CAGvB,iBACI,cACA,eAAgB,CAGpB,wBACI,cACA,WACA,8BAAsB,sBACtB,kBACA,uBACA,iBACA,kBACA,mBACA,yBACA,kBACA,sBACA,+DAAkE,uDAClE,2GAA8F,2UAC9F,cAAe,CAGnB,0FAEI,UAAY,CAGZ,2BACI,qBACA,UAAW,CAGnB,uCACI,iBAAkB,CAGlB,uCACI,gBACA,SACA,SAAU,CAGV,0CACI,WACA,iBAAkB,CAGtB,qDACI,cAAe,CAG3B,mCACI,yBAA0B,CAG9B,8BACI,sBACA,uEAAkE,+DAGtE,yBACI,wBACI,WACA,cAAe,CAClB,CAGL,2CACI,oBAAa,iCACb,8BAAmB,uEACnB,qBAAiB,iBACjB,yBAA8B,oDAC9B,0BAAoB,2CACpB,kBAAmB,CAGvB,oDACI,cACA,WACA,uBACA,UACA,SACA,eAAgB,CAGhB,yCACI,kBACA,qBAAsB,CAG1B,uDACI,qBACA,iBAAkB,CAGtB,4DACI,cACA,eACA,gBACA,aACA,wBAAyB,CAG7B,2DACI,kBACA,8BAAsB,sBACtB,WACA,sBACA,sBACA,sBACA,gBACA,qBACA,qBACA,iBAAkB,CAG1B,oDACI,cACA,kBACA,wBACA,kBACA,QACA,mCAA2B,2BAC3B,cACA,eACA,eAAgB","file":"../css/dashboard.css","sourcesContent":[".nice-padding {\n padding-left: 50px;\n padding-right: 50px;\n}\n\n.block_container {\n display: block;\n margin-top: 30px;\n}\n\n.block_container .block {\n display: block;\n float: left;\n box-sizing: border-box;\n position: relative;\n width: calc(50% - 10px);\n min-height: 216px;\n padding: 10px 20px;\n margin-bottom: 20px;\n border: 1px solid #d9d9d9;\n border-radius: 3px;\n background-color: #fff;\n box-shadow: 0 1px 3px rgba(0,0,0,0.00), 0 1px 2px rgba(0,0,0,0.00);\n transition: box-shadow 0.3s cubic-bezier(.25,.8,.25,1), border 0.3s cubic-bezier(.25,.8,.25,1);\n cursor: pointer;\n}\n\n.block_container .block--disabled h2,\n.block_container .block--disabled .inspect_container {\n opacity: 0.5;\n}\n\n .block_container .block h2 {\n display: inline-block;\n width: auto;\n }\n\n.block_container .block:nth-child(odd) {\n margin-right: 20px;\n}\n\n .block_container .block .block_actions {\n list-style: none;\n margin: 0;\n padding: 0;\n }\n\n .block_container .block .block_actions li {\n float: left;\n margin-right: 10px;\n }\n\n .block_container .block .block_actions li:last-child {\n margin-right: 0;\n }\n\n.block_container .block.suggestion {\n border: 1px dashed #d9d9d9;\n}\n\n.block_container .block:hover {\n border: 1px solid #fff;\n box-shadow: 0 3px 6px rgba(0,0,0,0.16), 0 3px 6px rgba(0,0,0,0.23);\n}\n\n@media (max-width: 699px) {\n .block_container .block {\n width: 100%;\n margin-right: 0;\n }\n}\n\n.block_container .block .inspect_container {\n display: flex;\n flex-direction: row;\n flex-wrap: nowrap;\n justify-content: space-between;\n align-items: stretch;\n margin-bottom: 10px;\n}\n\n.block_container .block .inspect_container .inspect {\n display: block;\n float: left;\n width: calc(50% - 10px);\n padding: 0;\n margin: 0;\n list-style: none;\n}\n\n .block_container .block span.icon::before {\n margin-right: 0.3em;\n vertical-align: bottom;\n }\n\n .block_container .block .inspect_container .inspect li {\n display: inline-block;\n margin-bottom: 5px;\n }\n\n .block_container .block .inspect_container .inspect li span {\n display: block;\n font-size: 20px;\n font-weight: bold;\n margin: 5px 0;\n overflow-wrap: break-word;\n }\n\n .block_container .block .inspect_container .inspect li pre {\n position: relative;\n box-sizing: border-box;\n width: auto;\n background-color: #eee;\n border: 1px solid #ccc;\n margin: 5px 0 5px 21px;\n padding: 2px 5px;\n word-wrap: break-word;\n word-break: break-all;\n border-radius: 3px;\n }\n\n.block_container .block.suggestion .suggestive_text {\n display: block;\n position: absolute;\n width: calc(100% - 40px);\n text-align: center;\n top: 50%;\n transform: translateY(-50%);\n color: #d9d9d9;\n font-size: 20px;\n font-weight: 100;\n}\n\n\n\n// WEBPACK FOOTER //\n// ./scss/dashboard.scss"],"sourceRoot":""} \ No newline at end of file +{"version":3,"sources":["webpack:///./scss/dashboard.scss"],"names":[],"mappings":"AAAA,cACI,kBACA,kBAAmB,CAGvB,iBACI,cACA,eAAgB,CAGpB,wBACI,cACA,WACA,8BAAsB,sBACtB,kBACA,uBACA,iBACA,kBACA,mBACA,yBACA,kBACA,sBACA,+DAAkE,uDAClE,2GAA8F,2UAC9F,cAAe,CAGnB,0FAEI,UAAY,CAGZ,2BACI,qBACA,UAAW,CAGnB,uCACI,iBAAkB,CAGlB,uCACI,gBACA,SACA,SAAU,CAGV,0CACI,WACA,iBAAkB,CAGtB,qDACI,cAAe,CAG3B,mCACI,yBAA0B,CAG9B,8BACI,sBACA,uEAAkE,+DAGtE,yBACI,wBACI,WACA,cAAe,CAClB,CAGL,2CACI,oBAAa,iCACb,8BAAmB,uEACnB,qBAAiB,iBACjB,yBAA8B,oDAC9B,0BAAoB,2CACpB,kBAAmB,CAGvB,oDACI,cACA,WACA,uBACA,UACA,SACA,eAAgB,CAMnB,+DAJO,qBACA,kBACA,iBAAkB,CAItB,yCACI,kBACA,qBAAsB,CAG1B,4DACI,cACA,eACA,gBACA,aACA,wBAAyB,CAG7B,2DACI,kBACA,8BAAsB,sBACtB,WACA,sBACA,sBACA,sBACA,gBACA,qBACA,qBACA,iBAAkB,CAG1B,oDACI,cACA,kBACA,wBACA,kBACA,QACA,mCAA2B,2BAC3B,cACA,eACA,eAAgB","file":"../css/dashboard.css","sourcesContent":[".nice-padding {\n padding-left: 50px;\n padding-right: 50px;\n}\n\n.block_container {\n display: block;\n margin-top: 30px;\n}\n\n.block_container .block {\n display: block;\n float: left;\n box-sizing: border-box;\n position: relative;\n width: calc(50% - 10px);\n min-height: 216px;\n padding: 10px 20px;\n margin-bottom: 20px;\n border: 1px solid #d9d9d9;\n border-radius: 3px;\n background-color: #fff;\n box-shadow: 0 1px 3px rgba(0,0,0,0.00), 0 1px 2px rgba(0,0,0,0.00);\n transition: box-shadow 0.3s cubic-bezier(.25,.8,.25,1), border 0.3s cubic-bezier(.25,.8,.25,1);\n cursor: pointer;\n}\n\n.block_container .block--disabled h2,\n.block_container .block--disabled .inspect_container {\n opacity: 0.5;\n}\n\n .block_container .block h2 {\n display: inline-block;\n width: auto;\n }\n\n.block_container .block:nth-child(odd) {\n margin-right: 20px;\n}\n\n .block_container .block .block_actions {\n list-style: none;\n margin: 0;\n padding: 0;\n }\n\n .block_container .block .block_actions li {\n float: left;\n margin-right: 10px;\n }\n\n .block_container .block .block_actions li:last-child {\n margin-right: 0;\n }\n\n.block_container .block.suggestion {\n border: 1px dashed #d9d9d9;\n}\n\n.block_container .block:hover {\n border: 1px solid #fff;\n box-shadow: 0 3px 6px rgba(0,0,0,0.16), 0 3px 6px rgba(0,0,0,0.23);\n}\n\n@media (max-width: 699px) {\n .block_container .block {\n width: 100%;\n margin-right: 0;\n }\n}\n\n.block_container .block .inspect_container {\n display: flex;\n flex-direction: row;\n flex-wrap: nowrap;\n justify-content: space-between;\n align-items: stretch;\n margin-bottom: 10px;\n}\n\n.block_container .block .inspect_container .inspect {\n display: block;\n float: left;\n width: calc(50% - 10px);\n padding: 0;\n margin: 0;\n list-style: none;\n .stat_card {\n display: inline-block;\n margin-bottom: 5px;\n margin-right: 10px;\n }\n}\n\n .block_container .block span.icon::before {\n margin-right: 0.3em;\n vertical-align: bottom;\n }\n\n .block_container .block .inspect_container .inspect li span {\n display: block;\n font-size: 20px;\n font-weight: bold;\n margin: 5px 0;\n overflow-wrap: break-word;\n }\n\n .block_container .block .inspect_container .inspect li pre {\n position: relative;\n box-sizing: border-box;\n width: auto;\n background-color: #eee;\n border: 1px solid #ccc;\n margin: 5px 0 5px 21px;\n padding: 2px 5px;\n word-wrap: break-word;\n word-break: break-all;\n border-radius: 3px;\n }\n\n.block_container .block.suggestion .suggestive_text {\n display: block;\n position: absolute;\n width: calc(100% - 40px);\n text-align: center;\n top: 50%;\n transform: translateY(-50%);\n color: #d9d9d9;\n font-size: 20px;\n font-weight: 100;\n}\n\n\n\n// WEBPACK FOOTER //\n// ./scss/dashboard.scss"],"sourceRoot":""} \ No newline at end of file diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index 7b9fa47b..8828d7c2 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -26,20 +26,33 @@

{% trans 'Filter' %}

{{ segment }}

    -
  • +
  • {% trans "This segment has been visited" %} {{ segment.visit_count|localize }} {% trans "time" %}{{ segment.visit_count|pluralize }}
    • -
  • +
  • {% trans "This segment has been active for" %} {{ segment.enable_date|days_since:segment.disable_date }} {% trans "day" %}{{ segment.enable_date|days_since:segment.disable_date|pluralize }}
    • + {% if segment.is_static %} +
  • + {% trans "This segment is Static" %} + + {{ segment.sessions.count|localize }} + {% if segment.sessions.count < segment.count %} + / {{ segment.count }} {% trans "member" %}{{ segment.count|pluralize }} + {% else %} + {% trans "member" %}{{ segment.sessions.count|pluralize }} + {% endif %} + +
    • + {% endif %}
    -
  • +
  • {% trans "The visitor must match" %} {% if segment.match_any %} {% trans "Any rule" %} @@ -48,7 +61,7 @@

    {{ segment }}

    {% endif %}
    • -
  • +
  • {% trans "The persistence of this segment is" %} {% if segment.persistent %} {% trans "Persistent" %} @@ -58,7 +71,7 @@

    {{ segment }}

    • {% for rule in segment.get_rules %} -
  • +
  • {{ rule.description.title }} {% if rule.description.code %} 
    {{ rule.description.value }}
    From f339879907928e44a501223a0bfb5a62e69d99f8 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 20 Oct 2017 09:53:18 +0100 Subject: [PATCH 04/97] Ensure that mixed static and dynamic segments are not populated at runtime --- src/wagtail_personalisation/models.py | 7 +++- tests/unit/test_static_dynamic_segments.py | 37 +++++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 3df7731b..f1e1a1a6 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -169,6 +169,10 @@ def save(self, *args, **kwargs): if self.is_static: request = RequestFactory().get('/') + + rules = self.get_rules() + all_static = all(rule.static for rule in rules) + for session in Session.objects.filter( expire_date__gt=timezone.now(), ).iterator(): @@ -176,7 +180,8 @@ def save(self, *args, **kwargs): user = user_from_data(session_data.get('_auth_id')) request.user = user request.session = SessionStore(session_key=session.session_key) - if all(rule.test_user(request) for rule in self.get_rules() if rule.static): + all_pass = all(rule.test_user(request) for rule in rules if rule.static) + if rules and all_static and all_pass: self.sessions.add(session) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 7815d8e0..b70b8061 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -17,14 +17,34 @@ def test_session_added_to_static_segment_at_creation(rf, site, client): segment = SegmentFactory(type=Segment.TYPE_STATIC) VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + segment.save() assert session.session_key in segment.sessions.values_list('session_key', flat=True) +@pytest.mark.django_db +def test_mixed_static_dynamic_session_doesnt_generate_at_creation(rf, site, client): + session = client.session + session.save() + client.get(site.root_page.url) + + segment = SegmentFactory(type=Segment.TYPE_STATIC) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + TimeRule.objects.create( + start_time=datetime.time(0, 0, 0), + end_time=datetime.time(23, 59, 59), + segment=segment, + ) + segment.save() + + assert not segment.sessions.all() + + @pytest.mark.django_db def test_session_not_added_to_static_segment_after_creation(rf, site, client): segment = SegmentFactory(type=Segment.TYPE_STATIC) VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + segment.save() session = client.session session.save() @@ -37,6 +57,7 @@ def test_session_not_added_to_static_segment_after_creation(rf, site, client): def test_session_added_to_static_segment_after_creation(rf, site, client): segment = SegmentFactory(type=Segment.TYPE_STATIC, count=1) VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + segment.save() session = client.session session.save() @@ -49,6 +70,7 @@ def test_session_added_to_static_segment_after_creation(rf, site, client): def test_session_not_added_to_static_segment_after_full(rf, site, client): segment = SegmentFactory(type=Segment.TYPE_STATIC, count=1) VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + segment.save() session = client.session session.save() @@ -65,12 +87,18 @@ def test_session_not_added_to_static_segment_after_full(rf, site, client): @pytest.mark.django_db -def test_sessions_not_added_to_static_segment_if_rule_not_static(): +def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site): + session = client.session + session.save() + client.get(site.root_page.url) + segment = SegmentFactory(type=Segment.TYPE_STATIC) TimeRule.objects.create( - start_time=datetime.time(8, 0, 0), - end_time=datetime.time(23, 0, 0), - segment=segment) + start_time=datetime.time(0, 0, 0), + end_time=datetime.time(23, 59, 59), + segment=segment, + ) + segment.save() assert not segment.sessions.all() @@ -83,6 +111,7 @@ def test_does_not_calculate_the_segment_again(rf, site, client, mocker): segment = SegmentFactory(type=Segment.TYPE_STATIC, count=2) VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + segment.save() mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules') client.get(site.root_page.url) From cf41be4b76a3963749493b7da7d8be199052ecfc Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 20 Oct 2017 10:57:19 +0100 Subject: [PATCH 05/97] Add clean method to ensure mixed static segments are valid --- src/wagtail_personalisation/models.py | 20 +++++++++--- tests/unit/test_static_dynamic_segments.py | 37 ++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index f1e1a1a6..25485836 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser from django.contrib.sessions.models import Session +from django.core.exceptions import ValidationError from django.db import models, transaction from django.template.defaultfilters import slugify from django.test.client import RequestFactory @@ -120,10 +121,22 @@ def __init__(self, *args, **kwargs): def __str__(self): return self.name + def clean(self): + if self.is_static and not self.is_consistent and not self.count: + raise ValidationError({ + 'count': _('Static segments with non-static rules must include a count.'), + }) + @property def is_static(self): return self.type == self.TYPE_STATIC + @property + def is_consistent(self): + rules = self.get_rules() + all_static = all(rule.static for rule in rules) + return rules and all_static + @property def is_full(self): return self.sessions.count() >= self.count @@ -170,9 +183,6 @@ def save(self, *args, **kwargs): if self.is_static: request = RequestFactory().get('/') - rules = self.get_rules() - all_static = all(rule.static for rule in rules) - for session in Session.objects.filter( expire_date__gt=timezone.now(), ).iterator(): @@ -180,8 +190,8 @@ def save(self, *args, **kwargs): user = user_from_data(session_data.get('_auth_id')) request.user = user request.session = SessionStore(session_key=session.session_key) - all_pass = all(rule.test_user(request) for rule in rules if rule.static) - if rules and all_static and all_pass: + all_pass = all(rule.test_user(request) for rule in self.get_rules() if rule.static) + if not self.is_consistent and all_pass: self.sessions.add(session) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index b70b8061..03648141 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -4,6 +4,7 @@ import pytest +from django.core.exceptions import ValidationError from tests.factories.segment import SegmentFactory from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import TimeRule, VisitCountRule @@ -116,3 +117,39 @@ def test_does_not_calculate_the_segment_again(rf, site, client, mocker): mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules') client.get(site.root_page.url) assert mock_test_rule.call_count == 0 + + +@pytest.mark.django_db +def test_non_static_rules_have_a_count(): + segment = SegmentFactory(type=Segment.TYPE_STATIC, count=0) + TimeRule.objects.create( + start_time=datetime.time(0, 0, 0), + end_time=datetime.time(23, 59, 59), + segment=segment, + ) + with pytest.raises(ValidationError): + segment.clean() + + +@pytest.mark.django_db +def test_static_segment_with_static_rules_needs_no_count(site): + segment = SegmentFactory(type=Segment.TYPE_STATIC, count=0) + VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + try: + segment.clean() + except ValidationError: + pytest.fail('Should not raise ValidationError.') + + +@pytest.mark.django_db +def test_dynamic_segment_with_non_static_rules_have_a_count(): + segment = SegmentFactory(type=Segment.TYPE_DYNAMIC, count=0) + TimeRule.objects.create( + start_time=datetime.time(0, 0, 0), + end_time=datetime.time(23, 59, 59), + segment=segment, + ) + try: + segment.clean() + except ValidationError: + pytest.fail('Should not raise ValidationError.') From ef20580334c3f7e6c9084079e6807c5361e8ccef Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 20 Oct 2017 12:09:25 +0100 Subject: [PATCH 06/97] Notify users to static compatible rules and update docs --- src/wagtail_personalisation/models.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 25485836..4cd9f694 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -81,8 +81,10 @@ class Segment(ClusterableModel): default=TYPE_DYNAMIC, help_text=_( "The users in a dynamic segment will change as more or less users meet " - "the rules specified in the segment. Static segments will contain the " - "members that existed at creation." + "the rules specified in the segment. Static segments containing only " + "static compatible ruless will contain the members that existed at " + "creation. Mixed static segments or those containing entirely non " + "static compatible rules will be populated using the count variable." ) ) count = models.PositiveSmallIntegerField( @@ -111,7 +113,10 @@ def __init__(self, *args, **kwargs): MultiFieldPanel([ InlinePanel( "{}_related".format(rule_model._meta.db_table), - label=rule_model._meta.verbose_name, + label='{}{}'.format( + rule_model._meta.verbose_name, + ' ({})'.format(_('Static compatible')) if rule_model.static else '' + ), ) for rule_model in AbstractBaseRule.__subclasses__() ], heading=_("Rules")), ] @@ -124,7 +129,7 @@ def __str__(self): def clean(self): if self.is_static and not self.is_consistent and not self.count: raise ValidationError({ - 'count': _('Static segments with non-static rules must include a count.'), + 'count': _('Static segments with non-static compatible rules must include a count.'), }) @property From ff236a095d96544e9cc1a2d5e66dfbc51f281b4e Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 20 Oct 2017 12:18:29 +0100 Subject: [PATCH 07/97] Improve the clarity of the help text --- src/wagtail_personalisation/models.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 4cd9f694..5c77cbeb 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -15,6 +15,7 @@ from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property from django.utils.lru_cache import lru_cache +from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ from modelcluster.models import ClusterableModel from wagtail.wagtailadmin.edit_handlers import ( @@ -79,13 +80,16 @@ class Segment(ClusterableModel): max_length=20, choices=TYPE_CHOICES, default=TYPE_DYNAMIC, - help_text=_( - "The users in a dynamic segment will change as more or less users meet " - "the rules specified in the segment. Static segments containing only " - "static compatible ruless will contain the members that existed at " - "creation. Mixed static segments or those containing entirely non " - "static compatible rules will be populated using the count variable." - ) + help_text=mark_safe(_(""" +

    Dynamic: Users in this segment will change + as more or less meet the rules specified in the segment. +
    Static: If the segment contains only static + compatible rules the segment will contain the members that pass + those rules when the segment is created. Mixed static segments or + those containing entirely non static compatible rules will be + populated using the count variable. + """ + )) ) count = models.PositiveSmallIntegerField( default=0, From 0d2834a55f70a8e7528dbb75c9ad860ff40dda81 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 20 Oct 2017 17:17:31 +0100 Subject: [PATCH 08/97] Update the help text migration --- .../migrations/0013_add_dynamic_static_to_segment.py | 2 +- src/wagtail_personalisation/models.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py b/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py index 7ab4e114..6196fa84 100644 --- a/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py +++ b/src/wagtail_personalisation/migrations/0013_add_dynamic_static_to_segment.py @@ -26,6 +26,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='segment', name='type', - field=models.CharField(choices=[('dynamic', 'Dynamic'), ('static', 'Static')], default='dynamic', help_text='The users in a dynamic segment will change as more or less users meet the rules specified in the segment. Static segments will contain the members that existed at creation.', max_length=20), + field=models.CharField(choices=[('dynamic', 'Dynamic'), ('static', 'Static')], default='dynamic', help_text='\n

    Dynamic: Users in this segment will change\n as more or less meet the rules specified in the segment.\n
    Static: If the segment contains only static\n compatible rules the segment will contain the members that pass\n those rules when the segment is created. Mixed static segments or\n those containing entirely non static compatible rules will be\n populated using the count variable.\n ', max_length=20), ), ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 5c77cbeb..913c3631 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -88,8 +88,7 @@ class Segment(ClusterableModel): those rules when the segment is created. Mixed static segments or those containing entirely non static compatible rules will be populated using the count variable. - """ - )) + """)) ) count = models.PositiveSmallIntegerField( default=0, From c6ff2801c54adea354e1328c0fdf9ca614c14d80 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 20 Oct 2017 17:33:47 +0100 Subject: [PATCH 09/97] Update to use a post_init signal to populate the segment --- .../migrations/0014_add_frozen_to_segment.py | 20 ++++++++ src/wagtail_personalisation/models.py | 46 ++++++++++++------- tests/unit/test_static_dynamic_segments.py | 6 +++ 3 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py diff --git a/src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py b/src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py new file mode 100644 index 00000000..d594e032 --- /dev/null +++ b/src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2017-10-20 16:26 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtail_personalisation', '0013_add_dynamic_static_to_segment'), + ] + + operations = [ + migrations.AddField( + model_name='segment', + name='frozen', + field=models.BooleanField(default=False), + ), + ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 913c3631..6ebdfcc8 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -9,6 +9,7 @@ from django.contrib.sessions.models import Session from django.core.exceptions import ValidationError from django.db import models, transaction +from django.dispatch import receiver from django.template.defaultfilters import slugify from django.test.client import RequestFactory from django.utils import timezone @@ -98,6 +99,7 @@ class Segment(ClusterableModel): ) ) sessions = models.ManyToManyField(Session) + frozen = models.BooleanField(default=False) objects = SegmentQuerySet.as_manager() @@ -149,6 +151,12 @@ def is_consistent(self): def is_full(self): return self.sessions.count() >= self.count + @property + def can_populate(self): + return ( + self.id and self.is_static and not self.frozen and self.is_consistent + ) + def encoded_name(self): """Return a string with a slug for the segment.""" return slugify(self.name.lower()) @@ -185,22 +193,28 @@ def toggle(self, save=True): if save: self.save() - def save(self, *args, **kwargs): - super(Segment, self).save(*args, **kwargs) - - if self.is_static: - request = RequestFactory().get('/') - - for session in Session.objects.filter( - expire_date__gt=timezone.now(), - ).iterator(): - session_data = session.get_decoded() - user = user_from_data(session_data.get('_auth_id')) - request.user = user - request.session = SessionStore(session_key=session.session_key) - all_pass = all(rule.test_user(request) for rule in self.get_rules() if rule.static) - if not self.is_consistent and all_pass: - self.sessions.add(session) + +@receiver(models.signals.post_init, sender=Segment) +def populate_sessions_first_time(sender, **kwargs): + instance = kwargs.pop('instance', None) + if instance.can_populate: + request = RequestFactory().get('/') + + for session in Session.objects.filter( + expire_date__gt=timezone.now(), + ).iterator(): + session_data = session.get_decoded() + user = user_from_data(session_data.get('_auth_id')) + request.user = user + request.session = SessionStore(session_key=session.session_key) + all_pass = all(rule.test_user(request) for rule in instance.get_rules() if rule.static) + if all_pass: + instance.sessions.add(session.session_key) + + models.signals.post_init.disconnect(populate_sessions_first_time, sender=sender) + instance.frozen = True + instance.save() + models.signals.post_init.connect(populate_sessions_first_time, sender=sender) class PersonalisablePageMetadata(ClusterableModel): diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 03648141..a97cc0f4 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -20,6 +20,9 @@ def test_session_added_to_static_segment_at_creation(rf, site, client): VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) segment.save() + # We need to trigger the post init + segment = Segment.objects.get(id=segment.id) + assert session.session_key in segment.sessions.values_list('session_key', flat=True) @@ -38,6 +41,9 @@ def test_mixed_static_dynamic_session_doesnt_generate_at_creation(rf, site, clie ) segment.save() + # We need to trigger the post init + segment = Segment.objects.get(id=segment.id) + assert not segment.sessions.all() From 44cc95617ec2429836a048602d444c05e89cc164 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Mon, 23 Oct 2017 15:00:31 +0100 Subject: [PATCH 10/97] Use a form to clean the instance --- src/wagtail_personalisation/forms.py | 18 +++++++++ src/wagtail_personalisation/models.py | 10 ++--- tests/unit/test_static_dynamic_segments.py | 45 +++++++++++++++------- 3 files changed, 54 insertions(+), 19 deletions(-) create mode 100644 src/wagtail_personalisation/forms.py diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py new file mode 100644 index 00000000..74874989 --- /dev/null +++ b/src/wagtail_personalisation/forms.py @@ -0,0 +1,18 @@ +from django.core.exceptions import ValidationError +from wagtail.wagtailadmin.forms import WagtailAdminModelForm + + +class SegmentAdminForm(WagtailAdminModelForm): + def clean(self): + cleaned_data = super(SegmentAdminForm, self).clean() + + rules = [form for formset in self.formsets.values() for form in formset if form not in formset.deleted_forms] + consistent = rules and all(rule.instance.static for rule in rules) + + from .models import Segment + if cleaned_data.get('type') == Segment.TYPE_STATIC and not cleaned_data.get('count') and not consistent: + raise ValidationError({ + 'count': ('Static segments with non-static compatible rules must include a count.'), + }) + + return cleaned_data diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 6ebdfcc8..6185c0cc 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -26,6 +26,8 @@ from wagtail_personalisation.rules import AbstractBaseRule from wagtail_personalisation.utils import count_active_days +from .forms import SegmentAdminForm + SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -103,6 +105,8 @@ class Segment(ClusterableModel): objects = SegmentQuerySet.as_manager() + base_form_class = SegmentAdminForm + def __init__(self, *args, **kwargs): Segment.panels = [ MultiFieldPanel([ @@ -131,12 +135,6 @@ def __init__(self, *args, **kwargs): def __str__(self): return self.name - def clean(self): - if self.is_static and not self.is_consistent and not self.count: - raise ValidationError({ - 'count': _('Static segments with non-static compatible rules must include a count.'), - }) - @property def is_static(self): return self.type == self.TYPE_STATIC diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index a97cc0f4..9f167926 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -5,7 +5,9 @@ import pytest from django.core.exceptions import ValidationError +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 TimeRule, VisitCountRule @@ -125,37 +127,54 @@ def test_does_not_calculate_the_segment_again(rf, site, client, mocker): assert mock_test_rule.call_count == 0 +def form_with_data(segment, rule): + model_fields = ['type', 'status', 'count', 'name'] + + class TestSegmentAdminForm(SegmentAdminForm): + class Meta: + model = Segment + fields = model_fields + + data = model_to_dict(segment, model_fields) + for formset in TestSegmentAdminForm().formsets.values(): + rule_data = {} + if isinstance(rule, formset.model): + rule_data = model_to_dict(rule) + for key, value in rule_data.items(): + data['{}-0-{}'.format(formset.prefix, key)] = value + data['{}-INITIAL_FORMS'.format(formset.prefix)] = 0 + data['{}-TOTAL_FORMS'.format(formset.prefix)] = 1 if rule_data else 0 + return TestSegmentAdminForm(data) + + + @pytest.mark.django_db def test_non_static_rules_have_a_count(): segment = SegmentFactory(type=Segment.TYPE_STATIC, count=0) - TimeRule.objects.create( + rule = TimeRule.objects.create( start_time=datetime.time(0, 0, 0), end_time=datetime.time(23, 59, 59), segment=segment, ) - with pytest.raises(ValidationError): - segment.clean() + form = form_with_data(segment, rule) + assert not form.is_valid() @pytest.mark.django_db def test_static_segment_with_static_rules_needs_no_count(site): segment = SegmentFactory(type=Segment.TYPE_STATIC, count=0) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - try: - segment.clean() - except ValidationError: - pytest.fail('Should not raise ValidationError.') + rule = VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + form = form_with_data(segment, rule) + assert form.is_valid() @pytest.mark.django_db def test_dynamic_segment_with_non_static_rules_have_a_count(): segment = SegmentFactory(type=Segment.TYPE_DYNAMIC, count=0) - TimeRule.objects.create( + rule = TimeRule.objects.create( start_time=datetime.time(0, 0, 0), end_time=datetime.time(23, 59, 59), segment=segment, ) - try: - segment.clean() - except ValidationError: - pytest.fail('Should not raise ValidationError.') + form = form_with_data(segment, rule) + assert form.is_valid(), form.errors From a116b14d5766d98e0dd94b594f2d6714e41900ea Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Mon, 23 Oct 2017 15:37:08 +0100 Subject: [PATCH 11/97] Update to use the save method on the form to populate the segments --- src/wagtail_personalisation/forms.py | 41 ++++++ src/wagtail_personalisation/models.py | 51 +------ tests/unit/test_static_dynamic_segments.py | 158 +++++++++++---------- 3 files changed, 130 insertions(+), 120 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 74874989..a8339232 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -1,7 +1,29 @@ +from importlib import import_module + +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.core.exceptions import ValidationError +from django.test.client import RequestFactory +from django.utils import timezone +from django.utils.lru_cache import lru_cache from wagtail.wagtailadmin.forms import WagtailAdminModelForm +SessionStore = import_module(settings.SESSION_ENGINE).SessionStore + + +@lru_cache(maxsize=1000) +def user_from_data(user_id): + User = get_user_model() + try: + return User.objects.get(id=user_id) + except User.DoesNotExist: + return AnonymousUser + + + class SegmentAdminForm(WagtailAdminModelForm): def clean(self): cleaned_data = super(SegmentAdminForm, self).clean() @@ -16,3 +38,22 @@ def clean(self): }) return cleaned_data + + def save(self, *args, **kwargs): + instance = super(SegmentAdminForm, self).save(*args, **kwargs) + + if instance.can_populate: + request = RequestFactory().get('/') + + for session in Session.objects.filter(expire_date__gt=timezone.now()).iterator(): + session_data = session.get_decoded() + user = user_from_data(session_data.get('_auth_id')) + request.user = user + request.session = SessionStore(session_key=session.session_key) + all_pass = all(rule.test_user(request) for rule in instance.get_rules() if rule.static) + if all_pass: + instance.sessions.add(session) + + instance.frozen = True + instance.save() + return instance diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 6185c0cc..4fa5f013 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -1,26 +1,20 @@ from __future__ import absolute_import, unicode_literals -from importlib import import_module - from django import forms -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.core.exceptions import ValidationError from django.db import models, transaction -from django.dispatch import receiver from django.template.defaultfilters import slugify -from django.test.client import RequestFactory -from django.utils import timezone from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property -from django.utils.lru_cache import lru_cache from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ from modelcluster.models import ClusterableModel from wagtail.wagtailadmin.edit_handlers import ( - FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel) + FieldPanel, + FieldRowPanel, + InlinePanel, + MultiFieldPanel, +) from wagtail.wagtailcore.models import Page from wagtail_personalisation.rules import AbstractBaseRule @@ -29,23 +23,11 @@ from .forms import SegmentAdminForm -SessionStore = import_module(settings.SESSION_ENGINE).SessionStore - - class SegmentQuerySet(models.QuerySet): def enabled(self): return self.filter(status=self.model.STATUS_ENABLED) -@lru_cache(maxsize=1000) -def user_from_data(user_id): - User = get_user_model() - try: - return User.objects.get(id=user_id) - except User.DoesNotExist: - return AnonymousUser - - @python_2_unicode_compatible class Segment(ClusterableModel): """The segment model.""" @@ -192,29 +174,6 @@ def toggle(self, save=True): self.save() -@receiver(models.signals.post_init, sender=Segment) -def populate_sessions_first_time(sender, **kwargs): - instance = kwargs.pop('instance', None) - if instance.can_populate: - request = RequestFactory().get('/') - - for session in Session.objects.filter( - expire_date__gt=timezone.now(), - ).iterator(): - session_data = session.get_decoded() - user = user_from_data(session_data.get('_auth_id')) - request.user = user - request.session = SessionStore(session_key=session.session_key) - all_pass = all(rule.test_user(request) for rule in instance.get_rules() if rule.static) - if all_pass: - instance.sessions.add(session.session_key) - - models.signals.post_init.disconnect(populate_sessions_first_time, sender=sender) - instance.frozen = True - instance.save() - models.signals.post_init.connect(populate_sessions_first_time, sender=sender) - - class PersonalisablePageMetadata(ClusterableModel): """The personalisable page model. Allows creation of variants with linked segments. diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 9f167926..490c7ba5 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -2,97 +2,124 @@ import datetime -import pytest - -from django.core.exceptions import ValidationError from django.forms.models import model_to_dict from tests.factories.segment import SegmentFactory + +import pytest from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import TimeRule, VisitCountRule +def form_with_data(segment, *rules): + model_fields = ['type', 'status', 'count', 'name'] + + class TestSegmentAdminForm(SegmentAdminForm): + class Meta: + model = Segment + fields = model_fields + + data = model_to_dict(segment, model_fields) + for formset in TestSegmentAdminForm().formsets.values(): + rule_data = {} + for rule in rules: + if isinstance(rule, formset.model): + rule_data = model_to_dict(rule) + for key, value in rule_data.items(): + data['{}-0-{}'.format(formset.prefix, key)] = value + data['{}-INITIAL_FORMS'.format(formset.prefix)] = 0 + data['{}-TOTAL_FORMS'.format(formset.prefix)] = 1 if rule_data else 0 + return TestSegmentAdminForm(data) + + @pytest.mark.django_db -def test_session_added_to_static_segment_at_creation(rf, site, client): +def test_session_added_to_static_segment_at_creation(site, client): session = client.session session.save() client.get(site.root_page.url) - segment = SegmentFactory(type=Segment.TYPE_STATIC) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - segment.save() - - # We need to trigger the post init - segment = Segment.objects.get(id=segment.id) + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() - assert session.session_key in segment.sessions.values_list('session_key', flat=True) + assert instance.frozen + assert session.session_key in instance.sessions.values_list('session_key', flat=True) @pytest.mark.django_db -def test_mixed_static_dynamic_session_doesnt_generate_at_creation(rf, site, client): +def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, client): session = client.session session.save() client.get(site.root_page.url) - segment = SegmentFactory(type=Segment.TYPE_STATIC) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - TimeRule.objects.create( + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) + static_rule = VisitCountRule(counted_page=site.root_page) + non_static_rule = TimeRule( start_time=datetime.time(0, 0, 0), end_time=datetime.time(23, 59, 59), - segment=segment, ) - segment.save() - - # We need to trigger the post init - segment = Segment.objects.get(id=segment.id) + form = form_with_data(segment, static_rule, non_static_rule) + instance = form.save() - assert not segment.sessions.all() + assert instance.frozen + assert not instance.sessions.all() @pytest.mark.django_db -def test_session_not_added_to_static_segment_after_creation(rf, site, client): - segment = SegmentFactory(type=Segment.TYPE_STATIC) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - segment.save() +def test_session_not_added_to_static_segment_after_creation(site, client): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=0) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() session = client.session session.save() client.get(site.root_page.url) - assert not segment.sessions.all() + assert instance.frozen + assert not instance.sessions.all() @pytest.mark.django_db -def test_session_added_to_static_segment_after_creation(rf, site, client): - segment = SegmentFactory(type=Segment.TYPE_STATIC, count=1) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - segment.save() +def test_session_added_to_static_segment_after_creation(site, client): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() session = client.session session.save() client.get(site.root_page.url) - assert session.session_key in segment.sessions.values_list('session_key', flat=True) + assert instance.frozen + assert session.session_key in instance.sessions.values_list('session_key', flat=True) @pytest.mark.django_db -def test_session_not_added_to_static_segment_after_full(rf, site, client): - segment = SegmentFactory(type=Segment.TYPE_STATIC, count=1) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - segment.save() +def test_session_not_added_to_static_segment_after_full(site, client): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + assert instance.frozen + assert instance.sessions.count() == 0 session = client.session session.save() client.get(site.root_page.url) + assert instance.sessions.count() == 1 + second_session = client.session second_session.create() client.get(site.root_page.url) assert session.session_key != second_session.session_key - assert segment.sessions.count() == 1 - assert session.session_key in segment.sessions.values_list('session_key', flat=True) - assert second_session.session_key not in segment.sessions.values_list('session_key', flat=True) + assert instance.sessions.count() == 1 + assert session.session_key in instance.sessions.values_list('session_key', flat=True) + assert second_session.session_key not in instance.sessions.values_list('session_key', flat=True) @pytest.mark.django_db @@ -101,57 +128,41 @@ def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site): session.save() client.get(site.root_page.url) - segment = SegmentFactory(type=Segment.TYPE_STATIC) - TimeRule.objects.create( + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) + rule = TimeRule( start_time=datetime.time(0, 0, 0), end_time=datetime.time(23, 59, 59), segment=segment, ) - segment.save() + form = form_with_data(segment, rule) + instance = form.save() - assert not segment.sessions.all() + assert instance.frozen + assert not instance.sessions.all() @pytest.mark.django_db -def test_does_not_calculate_the_segment_again(rf, site, client, mocker): +def test_does_not_calculate_the_segment_again(site, client, mocker): session = client.session session.save() client.get(site.root_page.url) - segment = SegmentFactory(type=Segment.TYPE_STATIC, count=2) - VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) - segment.save() + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=2) + rule = VisitCountRule(counted_page=site.root_page, segment=segment) + form = form_with_data(segment, rule) + instance = form.save() + assert instance.frozen + assert session.session_key in instance.sessions.values_list('session_key', flat=True) mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules') client.get(site.root_page.url) assert mock_test_rule.call_count == 0 -def form_with_data(segment, rule): - model_fields = ['type', 'status', 'count', 'name'] - - class TestSegmentAdminForm(SegmentAdminForm): - class Meta: - model = Segment - fields = model_fields - - data = model_to_dict(segment, model_fields) - for formset in TestSegmentAdminForm().formsets.values(): - rule_data = {} - if isinstance(rule, formset.model): - rule_data = model_to_dict(rule) - for key, value in rule_data.items(): - data['{}-0-{}'.format(formset.prefix, key)] = value - data['{}-INITIAL_FORMS'.format(formset.prefix)] = 0 - data['{}-TOTAL_FORMS'.format(formset.prefix)] = 1 if rule_data else 0 - return TestSegmentAdminForm(data) - - - @pytest.mark.django_db def test_non_static_rules_have_a_count(): - segment = SegmentFactory(type=Segment.TYPE_STATIC, count=0) - rule = TimeRule.objects.create( + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=0) + rule = TimeRule( start_time=datetime.time(0, 0, 0), end_time=datetime.time(23, 59, 59), segment=segment, @@ -162,19 +173,18 @@ def test_non_static_rules_have_a_count(): @pytest.mark.django_db def test_static_segment_with_static_rules_needs_no_count(site): - segment = SegmentFactory(type=Segment.TYPE_STATIC, count=0) - rule = VisitCountRule.objects.create(counted_page=site.root_page, segment=segment) + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=0) + rule = VisitCountRule(counted_page=site.root_page, segment=segment) form = form_with_data(segment, rule) assert form.is_valid() @pytest.mark.django_db def test_dynamic_segment_with_non_static_rules_have_a_count(): - segment = SegmentFactory(type=Segment.TYPE_DYNAMIC, count=0) - rule = TimeRule.objects.create( + segment = SegmentFactory.build(type=Segment.TYPE_DYNAMIC, count=0) + rule = TimeRule( start_time=datetime.time(0, 0, 0), end_time=datetime.time(23, 59, 59), - segment=segment, ) form = form_with_data(segment, rule) assert form.is_valid(), form.errors From 9e0fc8e6fd6c45d4c591e3a669d85f82e446497f Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Tue, 24 Oct 2017 10:50:05 +0100 Subject: [PATCH 12/97] Make the static segments work with match_any and fix bug in visit count --- src/wagtail_personalisation/adapters.py | 2 +- src/wagtail_personalisation/forms.py | 13 ++++++-- src/wagtail_personalisation/rules.py | 2 +- tests/unit/test_static_dynamic_segments.py | 38 ++++++++++++++++++---- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 16260ec9..e46ed197 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -144,7 +144,7 @@ def add_page_visit(self, page): def get_visit_count(self, page=None): """Return the number of visits on the current request or given page""" - path = page.path if page else self.request.path + path = page.url_path if page else self.request.path visit_count = self.request.session.setdefault('visit_count', []) for visit in visit_count: if visit['path'] == path: diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index a8339232..47031169 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -1,3 +1,5 @@ +from __future__ import absolute_import, unicode_literals + from importlib import import_module from django.conf import settings @@ -11,6 +13,7 @@ from wagtail.wagtailadmin.forms import WagtailAdminModelForm + SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -43,15 +46,21 @@ def save(self, *args, **kwargs): instance = super(SegmentAdminForm, self).save(*args, **kwargs) if instance.can_populate: + from .adapters import get_segment_adapter + request = RequestFactory().get('/') + request.session = SessionStore() + adapter = get_segment_adapter(request) + + rules = [rule for rule in instance.get_rules() if rule.static] for session in Session.objects.filter(expire_date__gt=timezone.now()).iterator(): session_data = session.get_decoded() user = user_from_data(session_data.get('_auth_id')) request.user = user request.session = SessionStore(session_key=session.session_key) - all_pass = all(rule.test_user(request) for rule in instance.get_rules() if rule.static) - if all_pass: + passes = adapter._test_rules(rules, request, instance.match_any) + if passes: instance.sessions.add(session) instance.frozen = True diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index ee98e36b..66a6b87c 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -229,7 +229,7 @@ def test_user(self, request): adapter = get_segment_adapter(request) - visit_count = adapter.get_visit_count() + visit_count = adapter.get_visit_count(self.counted_page) if visit_count and operator == "more_than": if visit_count > segment_count: return True diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 490c7ba5..7752ad2d 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -3,16 +3,17 @@ import datetime from django.forms.models import model_to_dict -from tests.factories.segment import SegmentFactory - +from django.contrib.sessions.backends.db import SessionStore import pytest from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import TimeRule, VisitCountRule +from tests.factories.segment import SegmentFactory + def form_with_data(segment, *rules): - model_fields = ['type', 'status', 'count', 'name'] + model_fields = ['type', 'status', 'count', 'name', 'match_any'] class TestSegmentAdminForm(SegmentAdminForm): class Meta: @@ -22,13 +23,15 @@ class Meta: data = model_to_dict(segment, model_fields) for formset in TestSegmentAdminForm().formsets.values(): rule_data = {} + count = 0 for rule in rules: if isinstance(rule, formset.model): rule_data = model_to_dict(rule) for key, value in rule_data.items(): - data['{}-0-{}'.format(formset.prefix, key)] = value + data['{}-{}-{}'.format(formset.prefix, count, key)] = value + count += 1 data['{}-INITIAL_FORMS'.format(formset.prefix)] = 0 - data['{}-TOTAL_FORMS'.format(formset.prefix)] = 1 if rule_data else 0 + data['{}-TOTAL_FORMS'.format(formset.prefix)] = count return TestSegmentAdminForm(data) @@ -47,6 +50,28 @@ def test_session_added_to_static_segment_at_creation(site, client): assert session.session_key in instance.sessions.values_list('session_key', flat=True) +@pytest.mark.django_db +def test_match_any_correct_populates(site, client): + session = client.session + client.get(site.root_page.url) + + client.cookies.clear() + second_session = client.session + other_page = site.root_page.get_last_child() + 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) + instance = form.save() + + assert instance.frozen + assert session.session_key != second_session.session_key + assert session.session_key in instance.sessions.values_list('session_key', flat=True) + assert second_session.session_key in instance.sessions.values_list('session_key', flat=True) + + @pytest.mark.django_db def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, client): session = client.session @@ -107,13 +132,12 @@ def test_session_not_added_to_static_segment_after_full(site, client): assert instance.sessions.count() == 0 session = client.session - session.save() client.get(site.root_page.url) assert instance.sessions.count() == 1 + client.cookies.clear() second_session = client.session - second_session.create() client.get(site.root_page.url) assert session.session_key != second_session.session_key From 7cf22d05f65540b9ad9cf5b83a64c63fd0d5e89e Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Thu, 26 Oct 2017 10:55:13 +0100 Subject: [PATCH 13/97] Tidy up the logic checks and remove the frozen property --- src/wagtail_personalisation/forms.py | 25 ++++++++++--------- .../migrations/0014_add_frozen_to_segment.py | 20 --------------- src/wagtail_personalisation/models.py | 16 +++++------- tests/unit/test_static_dynamic_segments.py | 9 +------ 4 files changed, 20 insertions(+), 50 deletions(-) delete mode 100644 src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 47031169..02544ce0 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -10,6 +10,7 @@ from django.test.client import RequestFactory from django.utils import timezone from django.utils.lru_cache import lru_cache +from django.utils.translation import ugettext_lazy as _ from wagtail.wagtailadmin.forms import WagtailAdminModelForm @@ -30,39 +31,39 @@ def user_from_data(user_id): class SegmentAdminForm(WagtailAdminModelForm): def clean(self): cleaned_data = super(SegmentAdminForm, self).clean() + Segment = self._meta.model - rules = [form for formset in self.formsets.values() for form in formset if form not in formset.deleted_forms] - consistent = rules and all(rule.instance.static for rule in rules) + rules = [ + form.instance for formset in self.formsets.values() + for form in formset + if form not in formset.deleted_forms + ] + consistent = rules and Segment.all_static(rules) - from .models import Segment if cleaned_data.get('type') == Segment.TYPE_STATIC and not cleaned_data.get('count') and not consistent: - raise ValidationError({ - 'count': ('Static segments with non-static compatible rules must include a count.'), - }) + self.add_error('count', _('Static segments with non-static compatible rules must include a count.')) return cleaned_data def save(self, *args, **kwargs): + is_new = not self.instance.id + instance = super(SegmentAdminForm, self).save(*args, **kwargs) - if instance.can_populate: + if is_new and instance.is_static and instance.all_rules_static: from .adapters import get_segment_adapter request = RequestFactory().get('/') request.session = SessionStore() adapter = get_segment_adapter(request) - rules = [rule for rule in instance.get_rules() if rule.static] - for session in Session.objects.filter(expire_date__gt=timezone.now()).iterator(): session_data = session.get_decoded() user = user_from_data(session_data.get('_auth_id')) request.user = user request.session = SessionStore(session_key=session.session_key) - passes = adapter._test_rules(rules, request, instance.match_any) + passes = adapter._test_rules(instance.get_rules(), request, instance.match_any) if passes: instance.sessions.add(session) - instance.frozen = True - instance.save() return instance diff --git a/src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py b/src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py deleted file mode 100644 index d594e032..00000000 --- a/src/wagtail_personalisation/migrations/0014_add_frozen_to_segment.py +++ /dev/null @@ -1,20 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11.6 on 2017-10-20 16:26 -from __future__ import unicode_literals - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('wagtail_personalisation', '0013_add_dynamic_static_to_segment'), - ] - - operations = [ - migrations.AddField( - model_name='segment', - name='frozen', - field=models.BooleanField(default=False), - ), - ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 4fa5f013..79723267 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -83,7 +83,6 @@ class Segment(ClusterableModel): ) ) sessions = models.ManyToManyField(Session) - frozen = models.BooleanField(default=False) objects = SegmentQuerySet.as_manager() @@ -121,22 +120,19 @@ def __str__(self): def is_static(self): return self.type == self.TYPE_STATIC + @classmethod + def all_static(cls, rules): + return all(rule.static for rule in rules) + @property - def is_consistent(self): + def all_rules_static(self): rules = self.get_rules() - all_static = all(rule.static for rule in rules) - return rules and all_static + return rules and self.all_static(rules) @property def is_full(self): return self.sessions.count() >= self.count - @property - def can_populate(self): - return ( - self.id and self.is_static and not self.frozen and self.is_consistent - ) - def encoded_name(self): """Return a string with a slug for the segment.""" return slugify(self.name.lower()) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 7752ad2d..fe4b05c4 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -46,7 +46,6 @@ def test_session_added_to_static_segment_at_creation(site, client): form = form_with_data(segment, rule) instance = form.save() - assert instance.frozen assert session.session_key in instance.sessions.values_list('session_key', flat=True) @@ -66,7 +65,6 @@ def test_match_any_correct_populates(site, client): form = form_with_data(segment, rule_1, rule_2) instance = form.save() - assert instance.frozen assert session.session_key != second_session.session_key assert session.session_key in instance.sessions.values_list('session_key', flat=True) assert second_session.session_key in instance.sessions.values_list('session_key', flat=True) @@ -87,7 +85,6 @@ def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, client): form = form_with_data(segment, static_rule, non_static_rule) instance = form.save() - assert instance.frozen assert not instance.sessions.all() @@ -102,7 +99,6 @@ def test_session_not_added_to_static_segment_after_creation(site, client): session.save() client.get(site.root_page.url) - assert instance.frozen assert not instance.sessions.all() @@ -117,7 +113,6 @@ def test_session_added_to_static_segment_after_creation(site, client): session.save() client.get(site.root_page.url) - assert instance.frozen assert session.session_key in instance.sessions.values_list('session_key', flat=True) @@ -128,7 +123,6 @@ def test_session_not_added_to_static_segment_after_full(site, client): form = form_with_data(segment, rule) instance = form.save() - assert instance.frozen assert instance.sessions.count() == 0 session = client.session @@ -161,7 +155,6 @@ def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site): form = form_with_data(segment, rule) instance = form.save() - assert instance.frozen assert not instance.sessions.all() @@ -175,7 +168,7 @@ def test_does_not_calculate_the_segment_again(site, client, mocker): rule = VisitCountRule(counted_page=site.root_page, segment=segment) form = form_with_data(segment, rule) instance = form.save() - assert instance.frozen + assert session.session_key in instance.sessions.values_list('session_key', flat=True) mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules') From bc0b69fde585034f58acc15619117659a83cd55b Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Thu, 26 Oct 2017 11:47:28 +0100 Subject: [PATCH 14/97] Hide and show the count input as required --- src/wagtail_personalisation/forms.py | 9 +++++++++ src/wagtail_personalisation/models.py | 2 +- .../static/js/segment_form_control.js | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 src/wagtail_personalisation/static/js/segment_form_control.js diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 02544ce0..969daa26 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -6,6 +6,7 @@ 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.core.exceptions import ValidationError from django.test.client import RequestFactory from django.utils import timezone @@ -67,3 +68,11 @@ def save(self, *args, **kwargs): instance.sessions.add(session) return instance + + @property + def media(self): + media = super(SegmentAdminForm, self).media + media.add_js( + [static('js/segment_form_control.js')] + ) + return media diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 79723267..e7d5e5f1 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -98,7 +98,7 @@ def __init__(self, *args, **kwargs): ]), FieldPanel('match_any'), FieldPanel('type', widget=forms.RadioSelect), - FieldPanel('count'), + FieldPanel('count', classname='count_field'), ], heading="Segment"), MultiFieldPanel([ InlinePanel( diff --git a/src/wagtail_personalisation/static/js/segment_form_control.js b/src/wagtail_personalisation/static/js/segment_form_control.js new file mode 100644 index 00000000..26e43d2b --- /dev/null +++ b/src/wagtail_personalisation/static/js/segment_form_control.js @@ -0,0 +1,18 @@ +(function($) { + $(document).ready( () => { + var count = $('.count_field'); + count.hide(); + + var updateCountDispay = function(value) { + if (value == 'dynamic') { + count.slideUp(250); + } else { + count.slideDown(250); + } + }; + + $('input:radio[name="type"]').change( event => { + updateCountDispay(event.target.value); + }); + }); +})(jQuery); From 743d3f668e3651f2748aacfc18bb9b9711673d35 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Thu, 26 Oct 2017 12:47:59 +0100 Subject: [PATCH 15/97] Limit the segemnt to count for all static case --- src/wagtail_personalisation/forms.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 969daa26..c4a4c422 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals from importlib import import_module +from itertools import takewhile from django.conf import settings from django.contrib.auth import get_user_model @@ -49,6 +50,9 @@ def clean(self): def save(self, *args, **kwargs): is_new = not self.instance.id + if not self.instance.is_static: + self.instance.count = 0 + instance = super(SegmentAdminForm, self).save(*args, **kwargs) if is_new and instance.is_static and instance.all_rules_static: @@ -58,14 +62,22 @@ def save(self, *args, **kwargs): request.session = SessionStore() adapter = get_segment_adapter(request) - for session in Session.objects.filter(expire_date__gt=timezone.now()).iterator(): + sessions_to_add = [] + sessions = Session.objects.filter(expire_date__gt=timezone.now()).iterator() + take_session = takewhile( + lambda x: instance.count == 0 or len(sessions_to_add) <= instance.count, + sessions + ) + for session in take_session: session_data = session.get_decoded() user = user_from_data(session_data.get('_auth_id')) request.user = user request.session = SessionStore(session_key=session.session_key) passes = adapter._test_rules(instance.get_rules(), request, instance.match_any) if passes: - instance.sessions.add(session) + sessions_to_add.append(session) + + instance.sessions.add(*sessions_to_add) return instance From 71d7faba1f64e741bf9bd4195260a84c54f314c2 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Thu, 26 Oct 2017 13:11:16 +0100 Subject: [PATCH 16/97] Correctly initialise the form --- .../static/js/segment_form_control.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/wagtail_personalisation/static/js/segment_form_control.js b/src/wagtail_personalisation/static/js/segment_form_control.js index 26e43d2b..269cd4b7 100644 --- a/src/wagtail_personalisation/static/js/segment_form_control.js +++ b/src/wagtail_personalisation/static/js/segment_form_control.js @@ -1,7 +1,7 @@ (function($) { $(document).ready( () => { var count = $('.count_field'); - count.hide(); + var typeRadio = $('input:radio[name="type"]'); var updateCountDispay = function(value) { if (value == 'dynamic') { @@ -11,7 +11,9 @@ } }; - $('input:radio[name="type"]').change( event => { + updateCountDispay(typeRadio.filter(':checked').val()); + + typeRadio.change( event => { updateCountDispay(event.target.value); }); }); From d07e06b4f0621179089995fcdb7df34357f3b4e0 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Thu, 26 Oct 2017 14:34:16 +0100 Subject: [PATCH 17/97] lock down the static segment to prevent changes --- src/wagtail_personalisation/forms.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index c4a4c422..487fb342 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -45,8 +45,24 @@ def clean(self): if cleaned_data.get('type') == Segment.TYPE_STATIC and not cleaned_data.get('count') and not consistent: self.add_error('count', _('Static segments with non-static compatible rules must include a count.')) + if self.instance.id and self.instance.is_static: + if self.has_changed(): + self.add_error_to_fields(self, 'name') + + for formset in self.formsets.values(): + if formset.has_changed(): + for form in formset: + if form not in formset.deleted_forms: + self.add_error_to_fields(form) + return cleaned_data + def add_error_to_fields(self, form, *exclude): + for field in form.changed_data: + if field not in exclude: + form.add_error(field, _('Cannot update a static segment')) + + def save(self, *args, **kwargs): is_new = not self.instance.id From b8bf27fb99639f0109170aef19b65cdda4cd55fe Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Fri, 27 Oct 2017 07:29:41 +0100 Subject: [PATCH 18/97] add enabled to the excluded segments checked --- src/wagtail_personalisation/forms.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 487fb342..e8e5debc 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -47,7 +47,7 @@ def clean(self): if self.instance.id and self.instance.is_static: if self.has_changed(): - self.add_error_to_fields(self, 'name') + self.add_error_to_fields(self, excluded=['name', 'enabled']) for formset in self.formsets.values(): if formset.has_changed(): @@ -57,9 +57,9 @@ def clean(self): return cleaned_data - def add_error_to_fields(self, form, *exclude): + def add_error_to_fields(self, form, excluded=list()): for field in form.changed_data: - if field not in exclude: + if field not in excluded: form.add_error(field, _('Cannot update a static segment')) From 1f4a4536ab55bab13e58b1062c79fd7b4bd3fb40 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Wed, 1 Nov 2017 16:43:22 +0000 Subject: [PATCH 19/97] Make the static elements tracked users only We cannot track anonymous users as the session expires after 10 minutes of inactivity. This also avoids an issue where there is an error when the user's session has expired and they navigate a page --- src/wagtail_personalisation/adapters.py | 9 +-- src/wagtail_personalisation/forms.py | 25 +++++---- .../migrations/0015_static_users.py | 26 +++++++++ src/wagtail_personalisation/models.py | 7 ++- tests/fixtures.py | 5 ++ tests/unit/test_static_dynamic_segments.py | 56 ++++++++++++------- 6 files changed, 87 insertions(+), 41 deletions(-) create mode 100644 src/wagtail_personalisation/migrations/0015_static_users.py diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index e46ed197..e145cd9f 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -175,7 +175,7 @@ def refresh(self): # Run tests on all remaining enabled segments to verify applicability. additional_segments = [] for segment in enabled_segments: - if segment.is_static and self.request.session.session_key in segment.sessions.values_list('session_key', flat=True): + if segment.is_static and segment.static_users.filter(id=self.request.user.id).exists(): additional_segments.append(segment) elif not segment.is_static or not segment.is_full: segment_rules = [] @@ -186,11 +186,8 @@ def refresh(self): match_any=segment.match_any) if result and segment.is_static and not segment.is_full: - session = self.request.session.model.objects.get( - session_key=self.request.session.session_key, - expire_date__gt=timezone.now(), - ) - segment.sessions.add(session) + if self.request.user.is_authenticated(): + segment.static_users.add(self.request.user) if result: additional_segments.append(segment) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index e8e5debc..a8d659a1 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -26,7 +26,7 @@ def user_from_data(user_id): try: return User.objects.get(id=user_id) except User.DoesNotExist: - return AnonymousUser + return AnonymousUser() @@ -78,22 +78,23 @@ def save(self, *args, **kwargs): request.session = SessionStore() adapter = get_segment_adapter(request) - sessions_to_add = [] - sessions = Session.objects.filter(expire_date__gt=timezone.now()).iterator() + users_to_add = [] + sessions = Session.objects.iterator() take_session = takewhile( - lambda x: instance.count == 0 or len(sessions_to_add) <= instance.count, + 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_id')) - request.user = user - request.session = SessionStore(session_key=session.session_key) - passes = adapter._test_rules(instance.get_rules(), request, instance.match_any) - if passes: - sessions_to_add.append(session) - - instance.sessions.add(*sessions_to_add) + 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: + users_to_add.append(user) + + instance.static_users.add(*users_to_add) return instance diff --git a/src/wagtail_personalisation/migrations/0015_static_users.py b/src/wagtail_personalisation/migrations/0015_static_users.py new file mode 100644 index 00000000..ea76aa8b --- /dev/null +++ b/src/wagtail_personalisation/migrations/0015_static_users.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2017-11-01 15:58 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('wagtail_personalisation', '0013_add_dynamic_static_to_segment'), + ] + + operations = [ + migrations.RemoveField( + model_name='segment', + name='sessions', + ), + migrations.AddField( + model_name='segment', + name='static_users', + field=models.ManyToManyField(to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index e7d5e5f1..cc1a6b2a 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals from django import forms +from django.conf import settings from django.contrib.sessions.models import Session from django.db import models, transaction from django.template.defaultfilters import slugify @@ -82,7 +83,9 @@ class Segment(ClusterableModel): "set until the number is reached. After this no more users will be added." ) ) - sessions = models.ManyToManyField(Session) + static_users = models.ManyToManyField( + settings.AUTH_USER_MODEL, + ) objects = SegmentQuerySet.as_manager() @@ -131,7 +134,7 @@ def all_rules_static(self): @property def is_full(self): - return self.sessions.count() >= self.count + return self.static_users.count() >= self.count def encoded_name(self): """Return a string with a slug for the segment.""" diff --git a/tests/fixtures.py b/tests/fixtures.py index 03efbd9c..9f5adfc5 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -44,3 +44,8 @@ def request(self, user=None, **request): request.session = SessionStore() request._messages = FallbackStorage(request) return request + + +@pytest.fixture +def user(django_user_model): + return django_user_model.objects.create(username='user') diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index fe4b05c4..778c0b5f 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -36,9 +36,10 @@ class Meta: @pytest.mark.django_db -def test_session_added_to_static_segment_at_creation(site, client): +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) segment = SegmentFactory.build(type=Segment.TYPE_STATIC) @@ -46,17 +47,21 @@ def test_session_added_to_static_segment_at_creation(site, client): form = form_with_data(segment, rule) instance = form.save() - assert session.session_key in instance.sessions.values_list('session_key', flat=True) + assert user in instance.static_users.all() @pytest.mark.django_db -def test_match_any_correct_populates(site, client): +def test_match_any_correct_populates(site, client, django_user_model): + 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) @@ -66,14 +71,15 @@ def test_match_any_correct_populates(site, client): instance = form.save() assert session.session_key != second_session.session_key - assert session.session_key in instance.sessions.values_list('session_key', flat=True) - assert second_session.session_key in instance.sessions.values_list('session_key', flat=True) + 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): +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) segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) @@ -85,11 +91,11 @@ def test_mixed_static_dynamic_session_doesnt_generate_at_creation(site, client): form = form_with_data(segment, static_rule, non_static_rule) instance = form.save() - assert not instance.sessions.all() + assert not instance.static_users.all() @pytest.mark.django_db -def test_session_not_added_to_static_segment_after_creation(site, client): +def test_session_not_added_to_static_segment_after_creation(site, client, user): segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=0) rule = VisitCountRule(counted_page=site.root_page) form = form_with_data(segment, rule) @@ -97,13 +103,14 @@ def test_session_not_added_to_static_segment_after_creation(site, client): session = client.session session.save() + client.force_login(user) client.get(site.root_page.url) - assert not instance.sessions.all() + assert not instance.static_users.all() @pytest.mark.django_db -def test_session_added_to_static_segment_after_creation(site, client): +def test_session_added_to_static_segment_after_creation(site, client, user): segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) rule = VisitCountRule(counted_page=site.root_page) form = form_with_data(segment, rule) @@ -111,39 +118,45 @@ def test_session_added_to_static_segment_after_creation(site, client): session = client.session session.save() + client.force_login(user) client.get(site.root_page.url) - assert session.session_key in instance.sessions.values_list('session_key', flat=True) + assert user in instance.static_users.all() @pytest.mark.django_db -def test_session_not_added_to_static_segment_after_full(site, client): +def test_session_not_added_to_static_segment_after_full(site, client, django_user_model): + user = django_user_model.objects.create(username='first') + other_user = django_user_model.objects.create(username='second') segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) rule = VisitCountRule(counted_page=site.root_page) form = form_with_data(segment, rule) instance = form.save() - assert instance.sessions.count() == 0 + assert not instance.static_users.all() session = client.session + client.force_login(user) client.get(site.root_page.url) - assert instance.sessions.count() == 1 + assert instance.static_users.count() == 1 client.cookies.clear() second_session = client.session + client.force_login(other_user) client.get(site.root_page.url) assert session.session_key != second_session.session_key - assert instance.sessions.count() == 1 - assert session.session_key in instance.sessions.values_list('session_key', flat=True) - assert second_session.session_key not in instance.sessions.values_list('session_key', flat=True) + assert instance.static_users.count() == 1 + assert user in instance.static_users.all() + assert other_user not in instance.static_users.all() @pytest.mark.django_db -def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site): +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) segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) @@ -155,13 +168,14 @@ def test_sessions_not_added_to_static_segment_if_rule_not_static(client, site): form = form_with_data(segment, rule) instance = form.save() - assert not instance.sessions.all() + assert not instance.static_users.all() @pytest.mark.django_db -def test_does_not_calculate_the_segment_again(site, client, mocker): +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) @@ -169,7 +183,7 @@ def test_does_not_calculate_the_segment_again(site, client, mocker): form = form_with_data(segment, rule) instance = form.save() - assert session.session_key in instance.sessions.values_list('session_key', flat=True) + assert user in instance.static_users.all() mock_test_rule = mocker.patch('wagtail_personalisation.adapters.SessionSegmentsAdapter._test_rules') client.get(site.root_page.url) From 23b145643899aef2e03aeeac1279f1b6b6ad361b Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Wed, 1 Nov 2017 17:10:03 +0000 Subject: [PATCH 20/97] Add tests which cover anonymous users --- tests/unit/test_static_dynamic_segments.py | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 778c0b5f..e8401648 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -50,6 +50,19 @@ def test_session_added_to_static_segment_at_creation(site, client, user): assert user in instance.static_users.all() +@pytest.mark.django_db +def test_anonymous_user_not_added_to_static_segment_at_creation(site, client): + session = client.session + session.save() + client.get(site.root_page.url) + + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + assert not instance.static_users.all() + @pytest.mark.django_db def test_match_any_correct_populates(site, client, django_user_model): user = django_user_model.objects.create(username='first') @@ -124,6 +137,20 @@ def test_session_added_to_static_segment_after_creation(site, client, user): assert user in instance.static_users.all() +@pytest.mark.django_db +def test_anonymou_user_not_added_to_static_segment_after_creation(site, client): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + session = client.session + session.save() + client.get(site.root_page.url) + + assert not instance.static_users.all() + + @pytest.mark.django_db def test_session_not_added_to_static_segment_after_full(site, client, django_user_model): user = django_user_model.objects.create(username='first') From 35fd4836b0d823dbceea8694c9f7a23c4dc40f20 Mon Sep 17 00:00:00 2001 From: Todd Dembrey Date: Wed, 8 Nov 2017 13:58:50 +0000 Subject: [PATCH 21/97] update the realease --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index f9f7c770..a3646905 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ setup( name='wagtail-personalisation', - version='0.9.1', + version='0.10.0beta1', description='A Wagtail add-on for showing personalized content', author='Lab Digital BV', author_email='opensource@labdigital.nl', From 232609fb4e38626271532488268774f524c31824 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 9 Nov 2017 11:52:55 +0200 Subject: [PATCH 22/97] Update setup file for new package name --- setup.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index a3646905..f54b566a 100644 --- a/setup.py +++ b/setup.py @@ -32,12 +32,12 @@ '^.. start-no-pypi.*^.. end-no-pypi', '', fh.read(), flags=re.M | re.S) setup( - name='wagtail-personalisation', + name='wagtail-personalisation-molo', version='0.10.0beta1', - description='A Wagtail add-on for showing personalized content', - author='Lab Digital BV', - author_email='opensource@labdigital.nl', - url='http://labdigital.nl', + description='A forked version of Wagtail add-on for showing personalized content', + author='Praekelt.org', + author_email='dev@praekeltfoundation.org', + url='https://github.com/praekeltfoundation/wagtail-personalisation/', install_requires=install_requires, tests_require=tests_require, extras_require={ From 3bfd5b8e8f424cab34f90893e25aea981e98a752 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 9 Nov 2017 11:54:13 +0200 Subject: [PATCH 23/97] Add deploy instructions to travis file --- .travis.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.travis.yml b/.travis.yml index a28ac857..370d7873 100644 --- a/.travis.yml +++ b/.travis.yml @@ -41,3 +41,13 @@ script: after_success: - tox -e coverage-report - codecov + +deploy: + provider: pypi + distributions: sdist bdist_wheel + user: praekelt.org + password: + secure: KEY_GOES_HERE + on: + tags: true + all_branches: true From 9d1f3074c08e202f8ab258f654bad014d96055f9 Mon Sep 17 00:00:00 2001 From: Milton Madanda Date: Thu, 9 Nov 2017 13:18:22 +0200 Subject: [PATCH 24/97] add pypi password --- .travis.yml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 370d7873..37e13f3c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,7 +47,7 @@ deploy: distributions: sdist bdist_wheel user: praekelt.org password: - secure: KEY_GOES_HERE + secure: IxPnc95OFNQsl7kFfLlLc38KfHh/W79VXnhEkdb2x1GZznOsG167QlhpAnyXIJysCQxgMMwLMuPOOdk1WIxOpGNM1/M80hNzPAfxMYWPuwposDdwoIc8SyREPJt16BXWAY+rAH8SHxld9p1YdRbOEPSSglODe4dCmQWsCbKjV3aKv+gZxBvf6pMxUglp2fBIlCwrE77MyMYh9iW0AGzD6atC2xN9NgAm4f+2WFlKCUL/MztLhNWdvWEiibQav6Tts7p8tWrsBVzywDW2IOy3O0ihPgRdISZ7QrxhiJTjlHYPAy4eRGOnYSQXvp6Dy8ONE5a6Uv5g3Xw2UmQo85sSMUs2VxR0G7d+PQgL0A7ENBQ5i7eSAFHYs8EswiGilW2A7mM4qRXwg9URLelYSdkM+aNXvR+25dCsXakiO4NjCz/BPgNzxPeQLlBdxR5vHugeM/XYuhy6CHlZrR/uueaO0X8RyhJcqeDjBy58IrwYS3Mpj7QCfBpQ9PqsqXEWV9BKwKiBXM2+3hkhawFDWa0GW2PDbORKtSLy/ORfGLx5Y9qxQYKEGvFQA3iqkTjrLj+KeUziKtuvEAcvsdBIJVIxeoHwdl+qqxEm8A7YuRBnWVxWc3jE6wBXboeFP92gVe/ueoXmY22riK9Ja0pli3TyNga8by9WM8Qp4D2ZqkVXHwg= on: tags: true all_branches: true diff --git a/setup.py b/setup.py index f54b566a..ee1c2b83 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.0beta1', + version='0.10.0', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From a5705fd53c933e19700746c7f8bc3b7f0011f437 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 9 Nov 2017 13:27:31 +0200 Subject: [PATCH 25/97] Removed extra newline --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index ee1c2b83..0d1db271 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,6 @@ import re from setuptools import find_packages, setup - install_requires = [ 'wagtail>=1.9,<1.11', 'user-agents>=1.0.1', From 51e9aa9724be552c02585b347c6a9a0387bb817c Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 9 Nov 2017 13:36:43 +0200 Subject: [PATCH 26/97] remove all the extra testing environments --- .travis.yml | 24 ------------------------ tox.ini | 8 +++----- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/.travis.yml b/.travis.yml index 37e13f3c..019a7543 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,30 +7,6 @@ matrix: # Django 1.9, Wagtail 1.9 - python: 2.7 env: TOXENV=py27-django19-wagtail19 - - python: 3.5 - env: TOXENV=py35-django19-wagtail19 - - python: 3.6 - env: TOXENV=py36-django19-wagtail19 - - # Django 1.10, Wagtail 1.10 - - python: 2.7 - env: TOXENV=py27-django110-wagtail110 - - python: 3.5 - env: TOXENV=py35-django110-wagtail110 - - python: 3.6 - env: TOXENV=py36-django110-wagtail110 - - # Django 1.11, Wagtail 1.10 - - python: 2.7 - env: TOXENV=py27-django111-wagtail110 - - python: 3.5 - env: TOXENV=py35-django111-wagtail110 - - python: 3.6 - env: TOXENV=py36-django111-wagtail110 - - allow_failures: - - python: 3.5 - env: TOXENV=lint install: - pip install tox codecov diff --git a/tox.ini b/tox.ini index 634c7d45..c4ca7bf9 100644 --- a/tox.ini +++ b/tox.ini @@ -1,18 +1,16 @@ [tox] -envlist = py{27,35,36}-django{19,110,111}-wagtail{19,110},lint +envlist = py{27}-django{19}-wagtail{19},lint [testenv] commands = coverage run --parallel -m pytest {posargs} extras = test deps = django19: django>=1.9,<1.10 - django110: django>=1.10<1.11 - django111: django>=1.11,<1.12 wagtail19: wagtail>=1.9,<1.10 wagtail110: wagtail>=1.10,<1.11 [testenv:coverage-report] -basepython = python3.5 +basepython = python2.7 deps = coverage pip_pre = true skip_install = true @@ -22,7 +20,7 @@ commands = [testenv:lint] -basepython = python3.5 +basepython = python2.7 deps = flake8 commands = flake8 src tests setup.py From 9c9a9d3acd8394b4dd78012ee8c830adc1a32903 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 9 Nov 2017 16:37:52 +0200 Subject: [PATCH 27/97] add missing function call --- src/wagtail_personalisation/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index a8d659a1..a6ea9e71 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -87,7 +87,7 @@ def save(self, *args, **kwargs): for session in take_session: session_data = session.get_decoded() user = user_from_data(session_data.get('_auth_user_id')) - if user.is_authenticated: + 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) From 4918c99b5f1370d0c631111eac6b5f02a30de5d9 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 9 Nov 2017 16:53:22 +0200 Subject: [PATCH 28/97] Version Bump 0.10.0 --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 1c774923..1f9a9f50 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +0.10.0 +================== + - Adds static and dynamic segments + 0.9.1 (tbd) ================== From c76d6d1617e3f89ccea10fd4bf159581f18e58ad Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Mon, 13 Nov 2017 14:58:20 +0200 Subject: [PATCH 29/97] Update manifest to include missing js files --- MANIFEST.in | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/MANIFEST.in b/MANIFEST.in index d0a6bf15..bb487c99 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,6 @@ include README.rst -recursive-include src \ No newline at end of file +recursive-include src * + +recursive-exclude src __pycache__ +recursive-exclude src *.py[co] From a8d3aeab6800a3e375c89b4329172f67900287b2 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Mon, 13 Nov 2017 15:02:56 +0200 Subject: [PATCH 30/97] Version Bump 0.10.1 --- docs/conf.py | 4 ++-- setup.cfg | 6 +++--- setup.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 965ca755..b2f3752b 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.9.1' +version = '0.10.1' # The full version, including alpha/beta/rc tags. -release = '0.9.1' +release = '0.10.1' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 8bb9e92a..1fb79c2d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.9.1 +current_version = 0.10.1 commit = true tag = true tag_name = {new_version} @@ -15,14 +15,14 @@ python_paths = . [flake8] ignore = E731 max-line-length = 120 -exclude = +exclude = src/**/migrations/*.py [wheel] universal = 1 [coverage:run] -omit = +omit = src/**/migrations/*.py [bumpversion:file:setup.py] diff --git a/setup.py b/setup.py index 0d1db271..2ab4f236 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.0', + version='0.10.1', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 0a42ce3eeb15d859a0f28fcd60fcfc63f9479023 Mon Sep 17 00:00:00 2001 From: Tomasz Knapik Date: Thu, 23 Nov 2017 14:10:16 +0000 Subject: [PATCH 31/97] Fix not updating visitor count rule properly --- src/wagtail_personalisation/adapters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index e145cd9f..a52e090c 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -133,12 +133,13 @@ def add_page_visit(self, page): if page_visits: for page_visit in page_visits: page_visit['count'] += 1 + page_visit['path'] = page.url_path if page else self.request.path self.request.session.modified = True else: visit_count.append({ 'slug': page.slug, 'id': page.pk, - 'path': self.request.path, + 'path': page.url_path if page else self.request.path, 'count': 1, }) From 06471248d3d9717f11350f62cfff93547af13523 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Thu, 23 Nov 2017 16:32:20 +0200 Subject: [PATCH 32/97] Version Bump 0.10.2 --- docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index b2f3752b..2c116d62 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.10.1' +version = '0.10.2' # The full version, including alpha/beta/rc tags. -release = '0.10.1' +release = '0.10.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 1fb79c2d..e7073a1a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.10.1 +current_version = 0.10.2 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index 2ab4f236..cd9aa096 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.1', + version='0.10.2', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From f19de241b0a784526cbc6804c2a51cfc1c7d3e12 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Fri, 5 Jan 2018 18:28:40 +0200 Subject: [PATCH 33/97] Update dependencies for wagtail and django Only run tests for wagtail v1.13 and django v1.11 --- .travis.yml | 3 +-- setup.py | 2 +- tox.ini | 7 +++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 019a7543..3aedbb13 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,9 +4,8 @@ language: python matrix: include: - # Django 1.9, Wagtail 1.9 - python: 2.7 - env: TOXENV=py27-django19-wagtail19 + env: TOXENV=py27-django111-wagtail113 install: - pip install tox codecov diff --git a/setup.py b/setup.py index cd9aa096..2de0fdd9 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import find_packages, setup install_requires = [ - 'wagtail>=1.9,<1.11', + 'wagtail>=1.10,<1.14', 'user-agents>=1.0.1', 'wagtailfontawesome>=1.0.6', ] diff --git a/tox.ini b/tox.ini index c4ca7bf9..8a5d1ba6 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,12 @@ [tox] -envlist = py{27}-django{19}-wagtail{19},lint +envlist = py{27}-django{111}-wagtail{113},lint [testenv] commands = coverage run --parallel -m pytest {posargs} extras = test deps = - django19: django>=1.9,<1.10 - wagtail19: wagtail>=1.9,<1.10 - wagtail110: wagtail>=1.10,<1.11 + django111: django>=1.11,<1.12 + wagtail19: wagtail>=1.13,<1.14 [testenv:coverage-report] basepython = python2.7 From a4a283e4f3641e981fe42e73f09ebd2212e8c694 Mon Sep 17 00:00:00 2001 From: Alex Muller Date: Wed, 20 Dec 2017 09:40:24 +0000 Subject: [PATCH 34/97] Fix segment edit link This is hardcoded at the moment which doesn't work unless you redirect URLs to have a trailing slash. Using the reverse URL lookup works in all cases. --- .../modeladmin/wagtail_personalisation/segment/dashboard.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index 8828d7c2..a11d3fd5 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -22,7 +22,7 @@

    {% trans 'Filter' %}

    {% if all_count %} {% for segment in object_list %} -
    +

    {{ segment }}

    {% endif %}
    From b3f0ac2d58a0368c1b52edc6b0c1745574aae20e Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Fri, 5 Jan 2018 19:16:45 +0200 Subject: [PATCH 35/97] Version Bump 0.10.3 --- docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 2c116d62..398a21ff 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.10.2' +version = '0.10.3' # The full version, including alpha/beta/rc tags. -release = '0.10.2' +release = '0.10.3' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index e7073a1a..9c907f70 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.10.2 +current_version = 0.10.3 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index 2de0fdd9..f83e8084 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.2', + version='0.10.3', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 808aa6d20218e9d7395ae7661ea76b66ad9b0ac9 Mon Sep 17 00:00:00 2001 From: Alex Muller Date: Mon, 8 Jan 2018 09:07:15 +0000 Subject: [PATCH 36/97] Add tests for exclude_variants --- tests/unit/test_utils.py | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index daab476d..f3d60713 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,4 +1,4 @@ -from wagtail_personalisation.utils import impersonate_other_page +from wagtail_personalisation.utils import impersonate_other_page, exclude_variants class Page(object): @@ -19,3 +19,40 @@ def test_impersonate_other_page(): impersonate_other_page(page, other_page) assert page == other_page + + +class Metadata(object): + def __init__(self, is_canonical=True): + self.is_canonical = is_canonical + + +class PersonalisationMetadataPage(object): + def __init__(self): + self.personalisation_metadata = Metadata() + + +def test_exclude_variants_includes_pages_with_no_metadata_property(): + page = PersonalisationMetadataPage() + del page.personalisation_metadata + result = exclude_variants([page]) + assert result == [page] + + +def test_exclude_variants_includes_pages_with_metadata_none(): + page = PersonalisationMetadataPage() + page.personalisation_metadata = None + result = exclude_variants([page]) + assert result == [page] + + +def test_exclude_variants_includes_pages_with_metadata_canonical(): + page = PersonalisationMetadataPage() + result = exclude_variants([page]) + assert result == [page] + + +def test_exclude_variants_excludes_pages_with_metadata_not_canonical(): + page = PersonalisationMetadataPage() + page.personalisation_metadata.is_canonical = False + result = exclude_variants([page]) + assert result == [] From e3488e87ad6b46961c3449b6ec17f25ccef4479d Mon Sep 17 00:00:00 2001 From: Alex Muller Date: Mon, 8 Jan 2018 09:08:11 +0000 Subject: [PATCH 37/97] Enable and fix lint --- .travis.yml | 3 +++ src/wagtail_personalisation/adapters.py | 1 - src/wagtail_personalisation/forms.py | 6 ------ .../0011_personalisablepagemetadata.py | 2 +- src/wagtail_personalisation/models.py | 9 ++------- src/wagtail_personalisation/utils.py | 20 +++++++++++++------ src/wagtail_personalisation/wagtail_hooks.py | 2 +- tests/settings.py | 5 +++-- tests/site/pages/migrations/0001_initial.py | 2 +- .../site/pages/migrations/0002_regularpage.py | 4 ++-- tests/unit/test_factories.py | 8 ++------ tests/unit/test_rules_time.py | 2 ++ tests/unit/test_static_dynamic_segments.py | 8 ++++---- tests/unit/test_utils.py | 3 ++- tox.ini | 2 +- 15 files changed, 38 insertions(+), 39 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3aedbb13..8c4237d5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,9 @@ language: python matrix: include: + - python: 2.7 + env: lint + - python: 2.7 env: TOXENV=py27-django111-wagtail113 diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index a52e090c..f8f24669 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -3,7 +3,6 @@ from django.conf import settings from django.db.models import F from django.utils.module_loading import import_string -from django.utils import timezone from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import AbstractBaseRule diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index a6ea9e71..0c9cd4f5 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -8,15 +8,11 @@ from django.contrib.auth.models import AnonymousUser from django.contrib.sessions.models import Session from django.contrib.staticfiles.templatetags.staticfiles import static -from django.core.exceptions import ValidationError from django.test.client import RequestFactory -from django.utils import timezone from django.utils.lru_cache import lru_cache from django.utils.translation import ugettext_lazy as _ from wagtail.wagtailadmin.forms import WagtailAdminModelForm - - SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -29,7 +25,6 @@ def user_from_data(user_id): return AnonymousUser() - class SegmentAdminForm(WagtailAdminModelForm): def clean(self): cleaned_data = super(SegmentAdminForm, self).clean() @@ -62,7 +57,6 @@ def add_error_to_fields(self, form, excluded=list()): if field not in excluded: form.add_error(field, _('Cannot update a static segment')) - def save(self, *args, **kwargs): is_new = not self.instance.id diff --git a/src/wagtail_personalisation/migrations/0011_personalisablepagemetadata.py b/src/wagtail_personalisation/migrations/0011_personalisablepagemetadata.py index e8eccad4..a4834f5c 100644 --- a/src/wagtail_personalisation/migrations/0011_personalisablepagemetadata.py +++ b/src/wagtail_personalisation/migrations/0011_personalisablepagemetadata.py @@ -2,8 +2,8 @@ # Generated by Django 1.11.1 on 2017-05-31 14:28 from __future__ import unicode_literals -from django.db import migrations, models import django.db.models.deletion +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index cc1a6b2a..f2fcce12 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -2,7 +2,6 @@ from django import forms from django.conf import settings -from django.contrib.sessions.models import Session from django.db import models, transaction from django.template.defaultfilters import slugify from django.utils.encoding import python_2_unicode_compatible @@ -11,11 +10,7 @@ from django.utils.translation import ugettext_lazy as _ from modelcluster.models import ClusterableModel from wagtail.wagtailadmin.edit_handlers import ( - FieldPanel, - FieldRowPanel, - InlinePanel, - MultiFieldPanel, -) + FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel) from wagtail.wagtailcore.models import Page from wagtail_personalisation.rules import AbstractBaseRule @@ -125,7 +120,7 @@ def is_static(self): @classmethod def all_static(cls, rules): - return all(rule.static for rule in rules) + return all(rule.static for rule in rules) @property def all_rules_static(self): diff --git a/src/wagtail_personalisation/utils.py b/src/wagtail_personalisation/utils.py index b261791b..49bea407 100644 --- a/src/wagtail_personalisation/utils.py +++ b/src/wagtail_personalisation/utils.py @@ -103,9 +103,17 @@ def exclude_variants(pages): :return: List of pages that aren't variants :rtype: list """ - return [page for page in pages - if (hasattr(page, 'personalisation_metadata') is False) - or (hasattr(page, 'personalisation_metadata') - and page.personalisation_metadata is None) - or (hasattr(page, 'personalisation_metadata') - and page.personalisation_metadata.is_canonical)] + return [ + page for page in pages + if ( + ( + hasattr(page, 'personalisation_metadata') is False + ) or + ( + hasattr(page, 'personalisation_metadata') and page.personalisation_metadata is None + ) or + ( + hasattr(page, 'personalisation_metadata') and page.personalisation_metadata.is_canonical + ) + ) + ] diff --git a/src/wagtail_personalisation/wagtail_hooks.py b/src/wagtail_personalisation/wagtail_hooks.py index 55ddd423..4f5a31d7 100644 --- a/src/wagtail_personalisation/wagtail_hooks.py +++ b/src/wagtail_personalisation/wagtail_hooks.py @@ -7,7 +7,7 @@ from django.template.defaultfilters import pluralize from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ -from wagtail.wagtailadmin.site_summary import SummaryItem, PagesSummaryItem +from wagtail.wagtailadmin.site_summary import PagesSummaryItem, SummaryItem from wagtail.wagtailadmin.widgets import Button, ButtonWithDropdownFromHook from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import Page diff --git a/tests/settings.py b/tests/settings.py index 15e185f9..ac0f1c0d 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,10 +1,9 @@ from __future__ import absolute_import, unicode_literals import os -from pkg_resources import parse_version as V import django - +from pkg_resources import parse_version as V DATABASES = { 'default': { @@ -56,6 +55,7 @@ }, ] + def get_middleware_settings(): return ( 'django.middleware.common.CommonMiddleware', @@ -69,6 +69,7 @@ def get_middleware_settings(): 'wagtail.wagtailcore.middleware.SiteMiddleware', ) + # Django 1.10 started to use "MIDDLEWARE" instead of "MIDDLEWARE_CLASSES". if V(django.get_version()) < V('1.10'): MIDDLEWARE_CLASSES = get_middleware_settings() diff --git a/tests/site/pages/migrations/0001_initial.py b/tests/site/pages/migrations/0001_initial.py index d911ea45..b24911e6 100644 --- a/tests/site/pages/migrations/0001_initial.py +++ b/tests/site/pages/migrations/0001_initial.py @@ -21,7 +21,7 @@ class Migration(migrations.Migration): migrations.CreateModel( name='ContentPage', fields=[ - ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), + ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), # noqa: E501 ('subtitle', models.CharField(blank=True, default='', max_length=255)), ('body', wagtail.wagtailcore.fields.RichTextField(blank=True, default='')), ], diff --git a/tests/site/pages/migrations/0002_regularpage.py b/tests/site/pages/migrations/0002_regularpage.py index 7b001aeb..09be0f88 100644 --- a/tests/site/pages/migrations/0002_regularpage.py +++ b/tests/site/pages/migrations/0002_regularpage.py @@ -2,9 +2,9 @@ # Generated by Django 1.11.1 on 2017-06-02 04:26 from __future__ import unicode_literals -from django.db import migrations, models import django.db.models.deletion import wagtail.wagtailcore.fields +from django.db import migrations, models class Migration(migrations.Migration): @@ -18,7 +18,7 @@ class Migration(migrations.Migration): migrations.CreateModel( name='RegularPage', fields=[ - ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), + ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), # noqa: E501 ('subtitle', models.CharField(blank=True, default='', max_length=255)), ('body', wagtail.wagtailcore.fields.RichTextField(blank=True, default='')), ], diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index a29892fd..ab79a36e 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -4,16 +4,14 @@ import pytest -from tests.factories.page import ContentPageFactory -from tests.factories.rule import ( - DayRuleFactory, DeviceRuleFactory, ReferralRuleFactory, TimeRuleFactory) +from tests.factories.rule import ReferralRuleFactory from tests.factories.segment import SegmentFactory -from tests.factories.site import SiteFactory from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import TimeRule # Factory tests + @pytest.mark.django_db def test_segment_create(): factoried_segment = SegmentFactory() @@ -27,8 +25,6 @@ def test_segment_create(): assert factoried_segment.status == segment.status - - @pytest.mark.django_db def test_referral_rule_create(): segment = SegmentFactory(name='Referral') diff --git a/tests/unit/test_rules_time.py b/tests/unit/test_rules_time.py index 33dec181..f1fa0053 100644 --- a/tests/unit/test_rules_time.py +++ b/tests/unit/test_rules_time.py @@ -16,6 +16,8 @@ def test_time_rule_create(): segment=segment) assert time_rule.start_time == datetime.time(8, 0, 0) + + @pytest.mark.django_db @freeze_time("10:00:00") def test_requesttime_segment(client, site): diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index e8401648..0796699e 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -2,15 +2,14 @@ import datetime -from django.forms.models import model_to_dict -from django.contrib.sessions.backends.db import SessionStore import pytest +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 TimeRule, VisitCountRule -from tests.factories.segment import SegmentFactory - def form_with_data(segment, *rules): model_fields = ['type', 'status', 'count', 'name', 'match_any'] @@ -63,6 +62,7 @@ def test_anonymous_user_not_added_to_static_segment_at_creation(site, client): assert not instance.static_users.all() + @pytest.mark.django_db def test_match_any_correct_populates(site, client, django_user_model): user = django_user_model.objects.create(username='first') diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index f3d60713..f5cee8a2 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,4 +1,5 @@ -from wagtail_personalisation.utils import impersonate_other_page, exclude_variants +from wagtail_personalisation.utils import ( + exclude_variants, impersonate_other_page) class Page(object): diff --git a/tox.ini b/tox.ini index 8a5d1ba6..34be95a0 100644 --- a/tox.ini +++ b/tox.ini @@ -20,7 +20,7 @@ commands = [testenv:lint] basepython = python2.7 -deps = flake8 +deps = flake8==3.5.0 commands = flake8 src tests setup.py isort -q --recursive --diff src/ tests/ From a00929846e64615f673b68cd14d6318e5a466525 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 18 Jan 2018 16:17:30 +0200 Subject: [PATCH 38/97] Set query rule to be static --- src/wagtail_personalisation/rules.py | 1 + tests/unit/test_factories.py | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index 66a6b87c..d94fc50b 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -261,6 +261,7 @@ class QueryRule(AbstractBaseRule): """ icon = 'fa-link' + static = True parameter = models.SlugField(_("The query parameter to search for"), max_length=20) diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index ab79a36e..dc2402cd 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -4,7 +4,7 @@ import pytest -from tests.factories.rule import ReferralRuleFactory +from tests.factories.rule import ReferralRuleFactory, QueryRuleFactory from tests.factories.segment import SegmentFactory from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import TimeRule @@ -33,3 +33,16 @@ def test_referral_rule_create(): segment=segment) assert referral_rule.regex_string == 'test.test' + + +@pytest.mark.django_db +def test_query_rule_create(): + segment = SegmentFactory(name='Query') + query_rule = QueryRuleFactory( + parameter="query", + value="value", + segment=segment) + + assert query_rule.parameter == 'query' + assert query_rule.value == 'value' + assert query_rule.static From c6ce67c9c97bc7344f1ba08678be570b73ada144 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 22 Jan 2018 12:58:12 +0200 Subject: [PATCH 39/97] Version bump 0.10.4 --- docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 398a21ff..a78f47ad 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.10.3' +version = '0.10.4' # The full version, including alpha/beta/rc tags. -release = '0.10.3' +release = '0.10.4' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 9c907f70..0752f14e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.10.3 +current_version = 0.10.4 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index f83e8084..6c27e716 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.3', + version='0.10.4', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 33f96af4a3c6cc171b69bf1f1e3f7a610a1017d5 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Wed, 24 Jan 2018 15:14:24 +0200 Subject: [PATCH 40/97] Allow test_user() for static rules to accept a user --- src/wagtail_personalisation/rules.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index d94fc50b..f4e6fa71 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -220,7 +220,12 @@ class VisitCountRule(AbstractBaseRule): class Meta: verbose_name = _('Visit count Rule') - def test_user(self, request): + 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 when the rule uses + # historical data + return False operator = self.operator segment_count = self.count @@ -276,7 +281,13 @@ class QueryRule(AbstractBaseRule): class Meta: verbose_name = _('Query Rule') - def test_user(self, request): + 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 + return request.GET.get(self.parameter, '') == self.value def description(self): From 4021d2c91501c9b296ebe1f8270ba9ef24ca74a4 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Wed, 24 Jan 2018 21:57:31 +0200 Subject: [PATCH 41/97] Add method to calculate the number of users that match a segment --- src/wagtail_personalisation/forms.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 0c9cd4f5..5eb1bf58 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -26,6 +26,29 @@ def user_from_data(user_id): class SegmentAdminForm(WagtailAdminModelForm): + + def count_matching_users(self, rules, match_any): + """ Calculates how many users match the given static rules + """ + count = 0 + + static_rules = [rule for rule in rules if rule.static] + + if not static_rules: + return count + + User = get_user_model() + users = User.objects.all() + + for user in users.iterator(): + if match_any: + if any(rule.test_user(None, user) for rule in static_rules): + count += 1 + elif all(rule.test_user(None, user) for rule in static_rules): + count += 1 + + return count + def clean(self): cleaned_data = super(SegmentAdminForm, self).clean() Segment = self._meta.model From caf73aa43c571fa13be15deb1ab8b31fe3fe9f51 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 25 Jan 2018 11:12:46 +0200 Subject: [PATCH 42/97] Add matched_users_count field to segments --- src/wagtail_personalisation/models.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index f2fcce12..2f9bfcf4 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -82,6 +82,9 @@ class Segment(ClusterableModel): settings.AUTH_USER_MODEL, ) + matched_users_count = models.PositiveIntegerField(default=0, editable=False) + matched_count_updated_at = models.DateTimeField(null=True, editable=False) + objects = SegmentQuerySet.as_manager() base_form_class = SegmentAdminForm From 786a8801b1dab68772f5c2a9b9b56bf15b96fbc4 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 25 Jan 2018 11:26:57 +0200 Subject: [PATCH 43/97] Migrations for Segment.matched_user_count --- .../migrations/0016_auto_20180125_0918.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/wagtail_personalisation/migrations/0016_auto_20180125_0918.py diff --git a/src/wagtail_personalisation/migrations/0016_auto_20180125_0918.py b/src/wagtail_personalisation/migrations/0016_auto_20180125_0918.py new file mode 100644 index 00000000..ae7bdd1d --- /dev/null +++ b/src/wagtail_personalisation/migrations/0016_auto_20180125_0918.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.9 on 2018-01-25 09:18 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtail_personalisation', '0015_static_users'), + ] + + operations = [ + migrations.AddField( + model_name='segment', + name='matched_count_updated_at', + field=models.DateTimeField(editable=False, null=True), + ), + migrations.AddField( + model_name='segment', + name='matched_users_count', + field=models.PositiveIntegerField(default=0, editable=False), + ), + ] From ef271587ec9efd84b9646185ce62809e4b4d7148 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 25 Jan 2018 13:26:05 +0200 Subject: [PATCH 44/97] Test count_matching_users method --- tests/unit/test_static_dynamic_segments.py | 113 ++++++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 0796699e..7718b47b 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -8,7 +8,8 @@ from tests.factories.segment import SegmentFactory from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment -from wagtail_personalisation.rules import TimeRule, VisitCountRule +from wagtail_personalisation.rules import (AbstractBaseRule, TimeRule, + VisitCountRule) def form_with_data(segment, *rules): @@ -246,3 +247,113 @@ def test_dynamic_segment_with_non_static_rules_have_a_count(): ) form = form_with_data(segment, rule) assert form.is_valid(), form.errors + + +@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 + + django_user_model.objects.create(username='first') + django_user_model.objects.create(username='second') + + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + rule = TestStaticRule() + form = form_with_data(segment, rule) + + assert form.count_matching_users([rule], True) is 2 + + +@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 + + django_user_model.objects.create(username='first') + django_user_model.objects.create(username='second') + + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + rule = TestStaticRule() + form = form_with_data(segment, rule) + + assert form.count_matching_users([rule], True) is 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 + + 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() + form = form_with_data(segment, first_rule, second_rule) + + assert form.count_matching_users([first_rule, second_rule], True) is 2 + + +@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 + + 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) + + assert form.count_matching_users([first_rule, s_rule], False) is 1 From fbcebb43a4cbc6ba0de15595317ac2b3aedf8739 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 25 Jan 2018 15:14:19 +0200 Subject: [PATCH 45/97] Store record count on a segment when it is created --- src/wagtail_personalisation/forms.py | 11 +++++++++++ tests/unit/test_static_dynamic_segments.py | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 5eb1bf58..9d342d90 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +from datetime import datetime from importlib import import_module from itertools import takewhile @@ -86,6 +87,16 @@ def save(self, *args, **kwargs): if not self.instance.is_static: self.instance.count = 0 + if is_new: + rules = [ + form.instance for formset in self.formsets.values() + for form in formset + if form not in formset.deleted_forms + ] + self.instance.matched_users_count = self.count_matching_users( + rules, self.instance.match_any) + self.instance.matched_count_updated_at = datetime.now() + instance = super(SegmentAdminForm, self).save(*args, **kwargs) if is_new and instance.is_static and instance.all_rules_static: diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 7718b47b..8dcabb67 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -357,3 +357,25 @@ def test_user(self, request, user): form = form_with_data(segment, first_rule, s_rule) assert form.count_matching_users([first_rule, s_rule], False) is 1 + + +@pytest.mark.django_db +def test_matched_user_count_added_to_segment_at_creation(site, client, 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() + form = form_with_data(segment, rule) + instance = form.save() + + assert instance.matched_users_count is 2 From 5b39e82f803004d79b92de69ea06e975f6f93654 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 25 Jan 2018 18:42:38 +0200 Subject: [PATCH 46/97] Fixed test for adding user counter to segment --- tests/unit/test_static_dynamic_segments.py | 40 ++++++++++------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 8dcabb67..6ef91a9a 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -4,8 +4,10 @@ import pytest from django.forms.models import model_to_dict +# from unittest.mock import patch from tests.factories.segment import SegmentFactory +# from wagtail import wagtailadmin from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import (AbstractBaseRule, TimeRule, @@ -249,6 +251,22 @@ def test_dynamic_segment_with_non_static_rules_have_a_count(): assert form.is_valid(), form.errors +@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') + django_user_model.objects.create(username='second') + + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + rule = VisitCountRule() + + form = form_with_data(segment, rule) + mock_test_user = mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True) + instance = form.save() + + assert mock_test_user.call_count == 2 + instance.matched_users_count = 2 + + @pytest.mark.django_db def test_count_users_matching_static_rules(site, client, django_user_model): class TestStaticRule(AbstractBaseRule): @@ -357,25 +375,3 @@ def test_user(self, request, user): form = form_with_data(segment, first_rule, s_rule) assert form.count_matching_users([first_rule, s_rule], False) is 1 - - -@pytest.mark.django_db -def test_matched_user_count_added_to_segment_at_creation(site, client, 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() - form = form_with_data(segment, rule) - instance = form.save() - - assert instance.matched_users_count is 2 From d5e89d374bd177634f08e4fa2591e0fff794f9bd Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 25 Jan 2018 19:51:50 +0200 Subject: [PATCH 47/97] Remove unnecessary imports --- tests/unit/test_static_dynamic_segments.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 6ef91a9a..c43e7768 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -4,10 +4,8 @@ import pytest from django.forms.models import model_to_dict -# from unittest.mock import patch from tests.factories.segment import SegmentFactory -# from wagtail import wagtailadmin from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import (AbstractBaseRule, TimeRule, From 5ad70d68f6d2a1af1b67badba7573d8b2f882a9c Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 26 Jan 2018 15:38:26 +0200 Subject: [PATCH 48/97] Don't include staff and inactive users when counting matched users --- src/wagtail_personalisation/forms.py | 2 +- tests/unit/test_static_dynamic_segments.py | 42 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 9d342d90..a946158d 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -39,7 +39,7 @@ def count_matching_users(self, rules, match_any): return count User = get_user_model() - users = User.objects.all() + users = User.objects.filter(is_active=True, is_staff=False) for user in users.iterator(): if match_any: diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index c43e7768..686e89fa 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -286,6 +286,48 @@ def test_user(self, request, user): 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 + + 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() + form = form_with_data(segment, rule) + + assert form.count_matching_users([rule], True) is 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 + + 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() + form = form_with_data(segment, rule) + + assert form.count_matching_users([rule], True) is 1 + + @pytest.mark.django_db def test_count_matching_users_only_counts_static_rules(site, client, django_user_model): class TestStaticRule(AbstractBaseRule): From 99f9700ed030a4687ce27deb11a74f13d7e87133 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 26 Jan 2018 16:21:15 +0200 Subject: [PATCH 49/97] Display record counter for active segments --- .../wagtail_personalisation/segment/dashboard.html | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index a11d3fd5..454a82e4 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -80,6 +80,11 @@

    {{ segment }}

    {% endif %}
    • {% endfor %} + {% if segment.matched_users_count > 0 %} +
  • + {{ segment.matched_users_count }} {% trans "user" %}{{ segment.matched_users_count|pluralize }} {% trans "were possible matches for this segment at creation" %} +
    • + {% endif %}
From 51774b939e219750ef901ff7a69d1d88abe5dcde Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 26 Jan 2018 17:57:43 +0200 Subject: [PATCH 50/97] Version 0.10.5 --- CHANGES | 6 ++++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 1f9a9f50..aa39a294 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +0.10.5 +================== + - Count how many users match a segments rules before saving the segment + - Stores count on the segment and displays in the dashboard + - Enables testing users against rules if there isn't an active request + 0.10.0 ================== - Adds static and dynamic segments diff --git a/docs/conf.py b/docs/conf.py index a78f47ad..dbcd3e03 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.10.4' +version = '0.10.5' # The full version, including alpha/beta/rc tags. -release = '0.10.4' +release = '0.10.5' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 0752f14e..009384cb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.10.4 +current_version = 0.10.5 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index 6c27e716..026a5899 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.4', + version='0.10.5', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From ae97118c3fc1c277b69a91c5640cc35b1abd7b75 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 2 Feb 2018 10:13:18 +0200 Subject: [PATCH 51/97] Store randomisation percentage on segment model --- src/wagtail_personalisation/models.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 2f9bfcf4..9d27eb56 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -2,6 +2,7 @@ from django import forms from django.conf import settings +from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models, transaction from django.template.defaultfilters import slugify from django.utils.encoding import python_2_unicode_compatible @@ -85,6 +86,16 @@ class Segment(ClusterableModel): matched_users_count = models.PositiveIntegerField(default=0, editable=False) matched_count_updated_at = models.DateTimeField(null=True, editable=False) + randomisation_percent = models.PositiveSmallIntegerField( + null=True, blank=True, default=None, + help_text=_( + "If this number is set each user matching the rules will " + "have this percentage chance of being placed in the segment." + ), validators=[ + MaxValueValidator(100), + MinValueValidator(0) + ]) + objects = SegmentQuerySet.as_manager() base_form_class = SegmentAdminForm @@ -100,6 +111,7 @@ def __init__(self, *args, **kwargs): FieldPanel('match_any'), FieldPanel('type', widget=forms.RadioSelect), FieldPanel('count', classname='count_field'), + FieldPanel('randomisation_percent', classname='percent_field'), ], heading="Segment"), MultiFieldPanel([ InlinePanel( From 602919d2d490d79f94ecd51127224d3e35e950f4 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 2 Feb 2018 10:14:18 +0200 Subject: [PATCH 52/97] Test randomisation percentage added to segments --- tests/unit/test_static_dynamic_segments.py | 34 +++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 686e89fa..f5fce7c4 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -13,7 +13,7 @@ def form_with_data(segment, *rules): - model_fields = ['type', 'status', 'count', 'name', 'match_any'] + model_fields = ['type', 'status', 'count', 'name', 'match_any', 'randomisation_percent'] class TestSegmentAdminForm(SegmentAdminForm): class Meta: @@ -249,6 +249,38 @@ def test_dynamic_segment_with_non_static_rules_have_a_count(): assert form.is_valid(), form.errors +@pytest.mark.django_db +def test_randomisation_percentage_added_to_segment_at_creation(site, client, mocker, django_user_model): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + segment.randomisation_percent = 80 + rule = VisitCountRule() + + form = form_with_data(segment, rule) + instance = form.save() + + assert instance.randomisation_percent == 80 + + +@pytest.mark.django_db +def test_randomisation_percentage_min_zero(site, client, mocker, django_user_model): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + segment.randomisation_percent = -1 + rule = VisitCountRule() + + form = form_with_data(segment, rule) + assert not form.is_valid() + + +@pytest.mark.django_db +def test_randomisation_percentage_max_100(site, client, mocker, django_user_model): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC) + segment.randomisation_percent = 101 + rule = VisitCountRule() + + form = form_with_data(segment, rule) + assert not form.is_valid() + + @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') From 5c3acc66611f7b673f0875f18896d76df2d5e465 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 2 Feb 2018 10:15:04 +0200 Subject: [PATCH 53/97] Display randomisation percentages on segment dashboard --- .../wagtail_personalisation/segment/dashboard.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index 454a82e4..d6858dd5 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -70,6 +70,13 @@

{{ segment }}

{% endif %} + {% if segment.randomisation_percent is not None %} +
  • + {{ segment.randomisation_percent }} % + {% trans "Chance that visitors matching the rules are added to the segment" %} +
    • + {% endif %} + {% for rule in segment.get_rules %}
  • {{ rule.description.title }} From 29aa91477e834ac14ffb2dda2464181d1527744c Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 2 Feb 2018 10:15:20 +0200 Subject: [PATCH 54/97] Migrations --- .../0017_segment_randomisation_percent.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 src/wagtail_personalisation/migrations/0017_segment_randomisation_percent.py diff --git a/src/wagtail_personalisation/migrations/0017_segment_randomisation_percent.py b/src/wagtail_personalisation/migrations/0017_segment_randomisation_percent.py new file mode 100644 index 00000000..bd68335a --- /dev/null +++ b/src/wagtail_personalisation/migrations/0017_segment_randomisation_percent.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.8 on 2018-01-31 16:12 +from __future__ import unicode_literals + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtail_personalisation', '0016_auto_20180125_0918'), + ] + + operations = [ + migrations.AddField( + model_name='segment', + name='randomisation_percent', + field=models.PositiveSmallIntegerField(blank=True, default=None, help_text='If this number is set each user matching the rules will have this percentage chance of being placed in the segment.', null=True, validators=[django.core.validators.MaxValueValidator(100), django.core.validators.MinValueValidator(0)]), + ), + ] From 6f97c7695846b0036a5da0c9283ae8611017b058 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 5 Feb 2018 12:18:22 +0200 Subject: [PATCH 55/97] Add method to randomise matching sessions into the segment --- src/wagtail_personalisation/models.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 9d27eb56..48d176b0 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -1,4 +1,5 @@ from __future__ import absolute_import, unicode_literals +import random from django import forms from django.conf import settings @@ -182,6 +183,19 @@ def toggle(self, save=True): if save: self.save() + def randomise_into_segment(self): + """ Returns True if randomisation_percent is not set or it generates + a random number less than the randomisation_percent + This is so there is some randomisation in which users are added to the + segment + """ + if self.randomisation_percent is None: + return True + + if random.randint(1, 100) <= self.randomisation_percent: + return True + return False + class PersonalisablePageMetadata(ClusterableModel): """The personalisable page model. Allows creation of variants with linked From 7200b5b4c49f9bfba5a2d7f5b73c1814535da4da Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 5 Feb 2018 12:19:36 +0200 Subject: [PATCH 56/97] If a session passes segment rules randomise them into the segement --- src/wagtail_personalisation/adapters.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index f8f24669..8d1ada8b 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -184,13 +184,12 @@ def refresh(self): result = self._test_rules(segment_rules, self.request, match_any=segment.match_any) - - if result and segment.is_static and not segment.is_full: - if self.request.user.is_authenticated(): - segment.static_users.add(self.request.user) - - if result: - additional_segments.append(segment) + if result and segment.randomise_into_segment(): + if segment.is_static and not segment.is_full: + if self.request.user.is_authenticated(): + segment.static_users.add(self.request.user) + else: + additional_segments.append(segment) self.set_segments(current_segments + additional_segments) self.update_visit_count() From d073c7d268076945b3affe7e84c4f9bd24f4fc1f Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 5 Feb 2018 12:20:10 +0200 Subject: [PATCH 57/97] Randomise into static segments when they are created --- src/wagtail_personalisation/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index a946158d..6183f4c5 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -119,7 +119,7 @@ def save(self, *args, **kwargs): request.user = user request.session = SessionStore(session_key=session.session_key) passes = adapter._test_rules(instance.get_rules(), request, instance.match_any) - if passes: + if passes and instance.randomise_into_segment(): users_to_add.append(user) instance.static_users.add(*users_to_add) From 881090f2f9f79cfabf44187a253140e220dfd26e Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 5 Feb 2018 12:21:09 +0200 Subject: [PATCH 58/97] Test randomisation for dynamic segments --- tests/unit/test_static_dynamic_segments.py | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index f5fce7c4..4abd6ed7 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -281,6 +281,72 @@ def test_randomisation_percentage_max_100(site, client, mocker, django_user_mode assert not form.is_valid() +@pytest.mark.django_db +def test_in_segment_if_random_is_below_percentage(site, client, mocker, user): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, + randomisation_percent=40) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + mocker.patch('random.randint', return_value=39) + session = client.session + session.save() + client.force_login(user) + client.get(site.root_page.url) + + assert user in instance.static_users.all() + + +@pytest.mark.django_db +def test_not_in_segment_if_random_is_above_percentage(site, client, mocker, user): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, + randomisation_percent=40) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + mocker.patch('random.randint', return_value=41) + session = client.session + session.save() + client.force_login(user) + client.get(site.root_page.url) + + assert user not in instance.static_users.all() + + +@pytest.mark.django_db +def test_not_in_segment_if_percentage_is_0(site, client, mocker, user): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, + randomisation_percent=0) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + session = client.session + session.save() + client.force_login(user) + client.get(site.root_page.url) + + assert user not in instance.static_users.all() + + +@pytest.mark.django_db +def test_always_in_segment_if_percentage_is_100(site, client, mocker, user): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, + randomisation_percent=100) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + session = client.session + session.save() + client.force_login(user) + client.get(site.root_page.url) + + assert user in instance.static_users.all() + + @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') From 086168954d4f4dc79f226ebbcedf7b634cbe7b66 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 5 Feb 2018 12:30:12 +0200 Subject: [PATCH 59/97] Test randomisation of static segments at creation --- tests/unit/test_static_dynamic_segments.py | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 4abd6ed7..b43a2961 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -347,6 +347,38 @@ def test_always_in_segment_if_percentage_is_100(site, client, mocker, user): assert user in instance.static_users.all() +@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) + + 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) + instance = form.save() + + assert user not in instance.static_users.all() + + +@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) + + 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) + instance = form.save() + + assert user in instance.static_users.all() + + @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') From 4c60bcbe6be6fb15bf9fbccd0448c37fc3405b15 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Tue, 6 Feb 2018 15:25:51 +0200 Subject: [PATCH 60/97] Version 0.10.6 --- CHANGES | 5 +++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index aa39a294..74234b8b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +0.10.6 +================== + - Accepts and stores randomisation percentage for segment + - Adds users to segment based on random number relative to percentage + 0.10.5 ================== - Count how many users match a segments rules before saving the segment diff --git a/docs/conf.py b/docs/conf.py index dbcd3e03..f282bd7a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.10.5' +version = '0.10.6' # The full version, including alpha/beta/rc tags. -release = '0.10.5' +release = '0.10.6' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 009384cb..34a404cd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.10.5 +current_version = 0.10.6 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index 026a5899..c1802483 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.5', + version='0.10.6', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 6b1a7cf1f278f30aca844fb36ce3ceb8fc10badc Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Tue, 6 Feb 2018 16:11:20 +0200 Subject: [PATCH 61/97] Only deploy for one environment per travis build --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8c4237d5..305772fd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,4 +28,4 @@ deploy: secure: IxPnc95OFNQsl7kFfLlLc38KfHh/W79VXnhEkdb2x1GZznOsG167QlhpAnyXIJysCQxgMMwLMuPOOdk1WIxOpGNM1/M80hNzPAfxMYWPuwposDdwoIc8SyREPJt16BXWAY+rAH8SHxld9p1YdRbOEPSSglODe4dCmQWsCbKjV3aKv+gZxBvf6pMxUglp2fBIlCwrE77MyMYh9iW0AGzD6atC2xN9NgAm4f+2WFlKCUL/MztLhNWdvWEiibQav6Tts7p8tWrsBVzywDW2IOy3O0ihPgRdISZ7QrxhiJTjlHYPAy4eRGOnYSQXvp6Dy8ONE5a6Uv5g3Xw2UmQo85sSMUs2VxR0G7d+PQgL0A7ENBQ5i7eSAFHYs8EswiGilW2A7mM4qRXwg9URLelYSdkM+aNXvR+25dCsXakiO4NjCz/BPgNzxPeQLlBdxR5vHugeM/XYuhy6CHlZrR/uueaO0X8RyhJcqeDjBy58IrwYS3Mpj7QCfBpQ9PqsqXEWV9BKwKiBXM2+3hkhawFDWa0GW2PDbORKtSLy/ORfGLx5Y9qxQYKEGvFQA3iqkTjrLj+KeUziKtuvEAcvsdBIJVIxeoHwdl+qqxEm8A7YuRBnWVxWc3jE6wBXboeFP92gVe/ueoXmY22riK9Ja0pli3TyNga8by9WM8Qp4D2ZqkVXHwg= on: tags: true - all_branches: true + condition: $TOXENV=py27-django111-wagtail113 From 3017f32b6b45a2dc3c8caa8eb1e66cdef860bf1c Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Tue, 6 Feb 2018 16:18:16 +0200 Subject: [PATCH 62/97] Add spaces around = in bash deploy condition --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 305772fd..3a780866 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,4 +28,4 @@ deploy: secure: IxPnc95OFNQsl7kFfLlLc38KfHh/W79VXnhEkdb2x1GZznOsG167QlhpAnyXIJysCQxgMMwLMuPOOdk1WIxOpGNM1/M80hNzPAfxMYWPuwposDdwoIc8SyREPJt16BXWAY+rAH8SHxld9p1YdRbOEPSSglODe4dCmQWsCbKjV3aKv+gZxBvf6pMxUglp2fBIlCwrE77MyMYh9iW0AGzD6atC2xN9NgAm4f+2WFlKCUL/MztLhNWdvWEiibQav6Tts7p8tWrsBVzywDW2IOy3O0ihPgRdISZ7QrxhiJTjlHYPAy4eRGOnYSQXvp6Dy8ONE5a6Uv5g3Xw2UmQo85sSMUs2VxR0G7d+PQgL0A7ENBQ5i7eSAFHYs8EswiGilW2A7mM4qRXwg9URLelYSdkM+aNXvR+25dCsXakiO4NjCz/BPgNzxPeQLlBdxR5vHugeM/XYuhy6CHlZrR/uueaO0X8RyhJcqeDjBy58IrwYS3Mpj7QCfBpQ9PqsqXEWV9BKwKiBXM2+3hkhawFDWa0GW2PDbORKtSLy/ORfGLx5Y9qxQYKEGvFQA3iqkTjrLj+KeUziKtuvEAcvsdBIJVIxeoHwdl+qqxEm8A7YuRBnWVxWc3jE6wBXboeFP92gVe/ueoXmY22riK9Ja0pli3TyNga8by9WM8Qp4D2ZqkVXHwg= on: tags: true - condition: $TOXENV=py27-django111-wagtail113 + condition: $TOXENV = py27-django111-wagtail113 From d114bb2570311450ebd71c4fdf4e86aa3d651e2d Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 8 Feb 2018 19:47:35 +0200 Subject: [PATCH 63/97] Always add the segment to the session if they pass --- src/wagtail_personalisation/adapters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 8d1ada8b..1012e7d0 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -184,12 +184,12 @@ def refresh(self): result = self._test_rules(segment_rules, self.request, match_any=segment.match_any) + if result and segment.randomise_into_segment(): if segment.is_static and not segment.is_full: if self.request.user.is_authenticated(): segment.static_users.add(self.request.user) - else: - additional_segments.append(segment) + additional_segments.append(segment) self.set_segments(current_segments + additional_segments) self.update_visit_count() From 824e42174fce6e5c9ad98328dfcd980a57d3ae37 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 8 Feb 2018 19:48:31 +0200 Subject: [PATCH 64/97] Tests --- tests/unit/test_static_dynamic_segments.py | 40 ++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index b43a2961..b7aae7c2 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -282,7 +282,7 @@ def test_randomisation_percentage_max_100(site, client, mocker, django_user_mode @pytest.mark.django_db -def test_in_segment_if_random_is_below_percentage(site, client, mocker, user): +def test_in_static_segment_if_random_is_below_percentage(site, client, mocker, user): segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, randomisation_percent=40) rule = VisitCountRule(counted_page=site.root_page) @@ -295,11 +295,12 @@ def test_in_segment_if_random_is_below_percentage(site, client, mocker, user): client.force_login(user) client.get(site.root_page.url) + assert instance.id == client.session['segments'][0]['id'] assert user in instance.static_users.all() @pytest.mark.django_db -def test_not_in_segment_if_random_is_above_percentage(site, client, mocker, user): +def test_not_in_static_segment_if_random_is_above_percentage(site, client, mocker, user): segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, randomisation_percent=40) rule = VisitCountRule(counted_page=site.root_page) @@ -312,9 +313,42 @@ def test_not_in_segment_if_random_is_above_percentage(site, client, mocker, user client.force_login(user) client.get(site.root_page.url) + assert len(client.session['segments']) == 0 assert user not in instance.static_users.all() +@pytest.mark.django_db +def test_offered_dynamic_segment_if_random_is_below_percentage(site, client, mocker): + segment = SegmentFactory.build(type=Segment.TYPE_DYNAMIC, + randomisation_percent=40) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + + mocker.patch('random.randint', return_value=39) + session = client.session + session.save() + client.get(site.root_page.url) + + assert instance.id == client.session['segments'][0]['id'] + + +@pytest.mark.django_db +def test_not_offered_dynamic_segment_if_random_is_above_percentage(site, client, mocker): + segment = SegmentFactory.build(type=Segment.TYPE_DYNAMIC, + randomisation_percent=40) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + form.save() + + mocker.patch('random.randint', return_value=41) + session = client.session + session.save() + client.get(site.root_page.url) + + assert len(client.session['segments']) == 0 + + @pytest.mark.django_db def test_not_in_segment_if_percentage_is_0(site, client, mocker, user): segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, @@ -328,6 +362,7 @@ def test_not_in_segment_if_percentage_is_0(site, client, mocker, user): client.force_login(user) client.get(site.root_page.url) + assert len(client.session['segments']) == 0 assert user not in instance.static_users.all() @@ -344,6 +379,7 @@ def test_always_in_segment_if_percentage_is_100(site, client, mocker, user): client.force_login(user) client.get(site.root_page.url) + assert instance.id == client.session['segments'][0]['id'] assert user in instance.static_users.all() From 3162191a16950f1ebf76e5f32caeb6d9052dba49 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Feb 2018 12:32:42 +0200 Subject: [PATCH 65/97] Add field to segment to store excluded users --- .../migrations/0018_segment_excluded_users.py | 22 +++++++++++++++++++ src/wagtail_personalisation/models.py | 6 +++++ 2 files changed, 28 insertions(+) create mode 100644 src/wagtail_personalisation/migrations/0018_segment_excluded_users.py diff --git a/src/wagtail_personalisation/migrations/0018_segment_excluded_users.py b/src/wagtail_personalisation/migrations/0018_segment_excluded_users.py new file mode 100644 index 00000000..bafa4776 --- /dev/null +++ b/src/wagtail_personalisation/migrations/0018_segment_excluded_users.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.9 on 2018-02-09 08:28 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('wagtail_personalisation', '0017_segment_randomisation_percent'), + ] + + operations = [ + migrations.AddField( + model_name='segment', + name='excluded_users', + field=models.ManyToManyField(help_text='Users that matched the rules but were excluded from the segment for some reason e.g. randomisation', related_name='excluded_segments', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 48d176b0..7a4063eb 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -83,6 +83,12 @@ class Segment(ClusterableModel): static_users = models.ManyToManyField( settings.AUTH_USER_MODEL, ) + excluded_users = models.ManyToManyField( + settings.AUTH_USER_MODEL, + help_text=_("Users that matched the rules but were excluded from the " + "segment for some reason e.g. randomisation"), + related_name="excluded_segments" + ) matched_users_count = models.PositiveIntegerField(default=0, editable=False) matched_count_updated_at = models.DateTimeField(null=True, editable=False) From 56a8e106d8ce75596c4264ad99f5ecc7edda55c1 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Feb 2018 12:35:09 +0200 Subject: [PATCH 66/97] Add users excluded by randomisation to excluded_users list --- src/wagtail_personalisation/adapters.py | 3 +++ src/wagtail_personalisation/forms.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 1012e7d0..a4222f7b 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -190,6 +190,9 @@ def refresh(self): if self.request.user.is_authenticated(): segment.static_users.add(self.request.user) additional_segments.append(segment) + elif result: + if self.request.user.is_authenticated(): + segment.excluded_users.add(self.request.user) self.set_segments(current_segments + additional_segments) self.update_visit_count() diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 6183f4c5..92f5f8e8 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -107,6 +107,7 @@ def save(self, *args, **kwargs): adapter = get_segment_adapter(request) 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, @@ -121,8 +122,11 @@ def save(self, *args, **kwargs): 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) instance.static_users.add(*users_to_add) + instance.excluded_users.add(*users_to_exclude) return instance From 521222f748d547dfed30b7f48f7a4f5b1b110099 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Feb 2018 12:35:53 +0200 Subject: [PATCH 67/97] Don't check if excluded users match segment rules --- src/wagtail_personalisation/adapters.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index a4222f7b..5001face 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -177,6 +177,8 @@ def refresh(self): 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(): + continue elif not segment.is_static or not segment.is_full: segment_rules = [] for rule_model in rule_models: From 0f18024ebc7fa6262cb4605844562dd1d7722187 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Feb 2018 12:36:34 +0200 Subject: [PATCH 68/97] Tests --- tests/unit/test_static_dynamic_segments.py | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index b7aae7c2..8b7181dd 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -297,6 +297,7 @@ def test_in_static_segment_if_random_is_below_percentage(site, client, mocker, u assert instance.id == client.session['segments'][0]['id'] assert user in instance.static_users.all() + assert user not in instance.excluded_users.all() @pytest.mark.django_db @@ -315,6 +316,7 @@ def test_not_in_static_segment_if_random_is_above_percentage(site, client, mocke assert len(client.session['segments']) == 0 assert user not in instance.static_users.all() + assert user in instance.excluded_users.all() @pytest.mark.django_db @@ -364,6 +366,7 @@ def test_not_in_segment_if_percentage_is_0(site, client, mocker, user): assert len(client.session['segments']) == 0 assert user not in instance.static_users.all() + assert user in instance.excluded_users.all() @pytest.mark.django_db @@ -381,6 +384,7 @@ def test_always_in_segment_if_percentage_is_100(site, client, mocker, user): assert instance.id == client.session['segments'][0]['id'] assert user in instance.static_users.all() + assert user not in instance.excluded_users.all() @pytest.mark.django_db @@ -397,6 +401,7 @@ def test_not_added_to_static_segment_at_creation_if_random_above_percent(site, c instance = form.save() assert user not in instance.static_users.all() + assert user in instance.excluded_users.all() @pytest.mark.django_db @@ -413,6 +418,31 @@ def test_added_to_static_segment_at_creation_if_random_below_percent(site, clien instance = form.save() assert user in instance.static_users.all() + assert user not in instance.excluded_users.all() + + +@pytest.mark.django_db +def test_rules_check_skipped_if_user_in_excluded(site, client, mocker, user): + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1, + randomisation_percent=100) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + instance = form.save() + instance.excluded_users.add(user) + instance.save + + 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 + assert len(client.session['segments']) == 0 + assert user not in instance.static_users.all() + assert user in instance.excluded_users.all() @pytest.mark.django_db From 33277a0b2045edefbce6e24b9bb6b2a4184203f5 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Feb 2018 16:59:26 +0200 Subject: [PATCH 69/97] Version 0.10.7 --- CHANGES | 6 ++++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 74234b8b..e3543bf8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +0.10.7 +================== + - Bug Fix: Ensure static segment members are show the survey immediately + - Records users excluded by randomisation on the segment + - Don't re-check excluded users + 0.10.6 ================== - Accepts and stores randomisation percentage for segment diff --git a/docs/conf.py b/docs/conf.py index f282bd7a..567a8115 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.10.6' +version = '0.10.7' # The full version, including alpha/beta/rc tags. -release = '0.10.6' +release = '0.10.7' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 34a404cd..a6a243b6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.10.6 +current_version = 0.10.7 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index c1802483..b783cfef 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.6', + version='0.10.7', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From c11960f921b40686a6857724c9475eb4f469a35b Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 12 Feb 2018 16:58:20 +0200 Subject: [PATCH 70/97] Only store excluded users for static segments --- src/wagtail_personalisation/adapters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 5001face..80518eb9 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -193,7 +193,7 @@ def refresh(self): 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) self.set_segments(current_segments + additional_segments) From 0f0aecf67359f501deca43ced366160183491096 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 12 Feb 2018 17:57:36 +0200 Subject: [PATCH 71/97] Store excluded segments in the session object --- src/wagtail_personalisation/adapters.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 80518eb9..176a4985 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -89,11 +89,13 @@ def get_segments(self): 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 +110,9 @@ def set_segments(self, segments): 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,6 +174,7 @@ def refresh(self): rule_models = AbstractBaseRule.get_descendant_models() current_segments = self.get_segments() + excluded_segments = [] # Run tests on all remaining enabled segments to verify applicability. additional_segments = [] @@ -195,8 +199,11 @@ def refresh(self): elif result: 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() From ea1ecc2a9894abf192a6928691bf05ed615287b3 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 12 Feb 2018 18:00:38 +0200 Subject: [PATCH 72/97] Get excluded segments from session and don't check them again --- src/wagtail_personalisation/adapters.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 176a4985..7c84f423 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -66,17 +66,21 @@ def __init__(self, request): 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,7 +90,8 @@ def get_segments(self): .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, key="segments"): @@ -174,14 +179,15 @@ def refresh(self): rule_models = AbstractBaseRule.get_descendant_models() current_segments = self.get_segments() - excluded_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 = [] From c909852b08197dac33e4bd846738a0da591b13bf Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 12 Feb 2018 18:01:01 +0200 Subject: [PATCH 73/97] Add tests --- tests/unit/test_static_dynamic_segments.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 8b7181dd..dbd3a453 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/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 From 4fd0b30c661acc406159ebe6d740da78421c4fee Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 12 Feb 2018 18:56:13 +0200 Subject: [PATCH 74/97] Check rules test skipped if segment excluded by session --- tests/unit/test_static_dynamic_segments.py | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index dbd3a453..f1834588 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -447,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') From 6e83366df625199ba51af36a0ac7517da3943541 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Tue, 13 Feb 2018 10:12:35 +0200 Subject: [PATCH 75/97] Bump version to 0.10.8 --- CHANGES | 5 +++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index e3543bf8..d8dc0779 100644 --- a/CHANGES +++ b/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 diff --git a/docs/conf.py b/docs/conf.py index 567a8115..b7b2aa87 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # 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. diff --git a/setup.cfg b/setup.cfg index a6a243b6..3d944ccc 100644 --- a/setup.cfg +++ b/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} diff --git a/setup.py b/setup.py index b783cfef..f455ba20 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ 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', From 2ff29cc375d7f6d052520d38cb7634ca25b17600 Mon Sep 17 00:00:00 2001 From: Saeed Marzban Date: Tue, 13 Feb 2018 13:47:45 +0200 Subject: [PATCH 76/97] get the number of users in a static segment from static_users variable --- .../wagtail_personalisation/segment/dashboard.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index d6858dd5..1fe37567 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -38,11 +38,11 @@

    {{ segment }}

  • {% trans "This segment is Static" %} - {{ segment.sessions.count|localize }} - {% if segment.sessions.count < segment.count %} + {{ segment.static_users.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 %}
    • From 59f4877e0476aef001679a536a068dc89e694ed1 Mon Sep 17 00:00:00 2001 From: Saeed Marzban Date: Tue, 13 Feb 2018 13:57:00 +0200 Subject: [PATCH 77/97] add localize filter --- .../modeladmin/wagtail_personalisation/segment/dashboard.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index 1fe37567..b0fcf2e5 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -38,7 +38,7 @@

    {{ segment }}

  • {% trans "This segment is Static" %} - {{ segment.static_users.count }} + {{ segment.static_users.count|localize }} {% if segment.static_users.count < segment.count %} / {{ segment.count }} {% trans "member" %}{{ segment.count|pluralize }} {% else %} From 362f15e5ffdce5112837eaf9a058eb17094d2f61 Mon Sep 17 00:00:00 2001 From: Saeed Marzban Date: Tue, 13 Feb 2018 14:17:29 +0200 Subject: [PATCH 78/97] version bump 0.10.9 --- CHANGES | 4 ++++ setup.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index d8dc0779..26680bb1 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +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 diff --git a/setup.py b/setup.py index f455ba20..5f56c9b0 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.8', + version='0.10.9', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 8f789b3e17b8927d1e0c288aeea505f43bc59e21 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 15 Feb 2018 13:20:48 +0200 Subject: [PATCH 79/97] Fill static segments with users from the database at creation --- src/wagtail_personalisation/forms.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 92f5f8e8..d7c2fee0 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -108,22 +108,22 @@ def save(self, *args, **kwargs): users_to_add = [] users_to_exclude = [] - sessions = Session.objects.iterator() - take_session = takewhile( + # sessions = Session.objects.iterator() + + User = get_user_model() + users = User.objects.filter(is_active=True, is_staff=False) + + take_user = takewhile( lambda x: instance.count == 0 or len(users_to_add) <= instance.count, - sessions + users ) - 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) + for user in take_user: + request.user = user + 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) instance.static_users.add(*users_to_add) instance.excluded_users.add(*users_to_exclude) From 364cb1a7e605c302b71ce914c7a068bd6c2da7b4 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Tue, 20 Feb 2018 15:08:12 +0200 Subject: [PATCH 80/97] Query rule should not be static --- src/wagtail_personalisation/rules.py | 9 +-------- tests/unit/test_factories.py | 1 - 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index f4e6fa71..5316002a 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -266,7 +266,6 @@ class QueryRule(AbstractBaseRule): """ icon = 'fa-link' - static = True parameter = models.SlugField(_("The query parameter to search for"), max_length=20) @@ -281,13 +280,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): diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index dc2402cd..0ce5b7cb 100644 --- a/tests/unit/test_factories.py +++ b/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 From 330557be8df1766819ee69a087fcd6afaecbf275 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Wed, 21 Feb 2018 18:48:44 +0200 Subject: [PATCH 81/97] Make VisitCountRule.test_user actually test with only a user --- src/wagtail_personalisation/rules.py | 34 +++++++++++++++++++++++----- tests/unit/test_rules_visitcount.py | 29 ++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index 5316002a..6461ea79 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -2,17 +2,23 @@ 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): @@ -221,17 +227,33 @@ class Meta: verbose_name = _('Visit count Rule') 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.session = SessionStore() + + # If we're using the session adapter check for an active session + if SEGMENT_ADAPTER_CLASS == SessionSegmentsAdapter: + sessions = Session.objects.iterator() + for session in sessions: + session_data = session.get_decoded() + if session_data.get('_auth_user_id') == str(user.id): + request.session = SessionStore( + session_key=session.session_key) + break + + request.user = user + 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) diff --git a/tests/unit/test_rules_visitcount.py b/tests/unit/test_rules_visitcount.py index a4d7d60f..f153e3c2 100644 --- a/tests/unit/test_rules_visitcount.py +++ b/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,29 @@ 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) From 12b0cd92315aa1d6fc3ffee7e052b5a9cd0de6ed Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Wed, 21 Feb 2018 19:07:35 +0200 Subject: [PATCH 82/97] Make visit count session retrieval seperate method --- src/wagtail_personalisation/rules.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index 6461ea79..b1bdb28e 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -226,6 +226,14 @@ 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 ( @@ -234,19 +242,14 @@ def test_user(self, request, user=None): if user: # Create a fake request so we can use the adapter request = RequestFactory().get('/') - request.session = SessionStore() + request.user = user # If we're using the session adapter check for an active session if SEGMENT_ADAPTER_CLASS == SessionSegmentsAdapter: - sessions = Session.objects.iterator() - for session in sessions: - session_data = session.get_decoded() - if session_data.get('_auth_user_id') == str(user.id): - request.session = SessionStore( - session_key=session.session_key) - break + request.session = self._get_user_session(user) + else: + request.session = SessionStore() - request.user = user elif not request: # Return false if we don't have a user or a request return False From fdc0a7f2e1208d5a60cf96260c84e2cb8cf099d1 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Wed, 21 Feb 2018 19:08:29 +0200 Subject: [PATCH 83/97] Get user info for Visit count rule --- src/wagtail_personalisation/rules.py | 22 ++++++++++++++++++++++ tests/unit/test_rules_visitcount.py | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index b1bdb28e..721e8672 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -282,6 +282,28 @@ def description(self): ), } + 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. diff --git a/tests/unit/test_rules_visitcount.py b/tests/unit/test_rules_visitcount.py index f153e3c2..17dab9ce 100644 --- a/tests/unit/test_rules_visitcount.py +++ b/tests/unit/test_rules_visitcount.py @@ -49,3 +49,24 @@ def test_visit_count_call_test_user_with_user_or_request_fails(site, client, use 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' From ba6056e3f815262f1bfa4c02c8778adb6aa89563 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 22 Feb 2018 11:09:49 +0200 Subject: [PATCH 84/97] Link to download CSV of users in segment --- src/wagtail_personalisation/admin_urls.py | 2 ++ .../segment/dashboard.html | 3 ++ src/wagtail_personalisation/views.py | 32 ++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/admin_urls.py b/src/wagtail_personalisation/admin_urls.py index 87848c07..e5c978cb 100644 --- a/src/wagtail_personalisation/admin_urls.py +++ b/src/wagtail_personalisation/admin_urls.py @@ -13,4 +13,6 @@ 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[0-9]+)$', + views.segment_user_data, name='segment_user_data'), ] diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index b0fcf2e5..fc314c6d 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -103,6 +103,9 @@

    {{ segment }}

  • disable
    • {% endif %}
  • configure this
    • + {% if segment.is_static %} +
  • download users csv
    • + {% endif %} {% endif %} diff --git a/src/wagtail_personalisation/views.py b/src/wagtail_personalisation/views.py index 30ee9139..89e91198 100644 --- a/src/wagtail_personalisation/views.py +++ b/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() From 9408f90789315522cc9b5661e7c9b9d8138c684f Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 22 Feb 2018 14:23:14 +0200 Subject: [PATCH 85/97] Use mock for testing matching user count The fake class was causing other tests to fail because it inherits from AbstractBaseRule but isn't in the database. I removed it and replaced it with mocked calls --- tests/unit/test_static_dynamic_segments.py | 133 ++++++--------------- 1 file changed, 39 insertions(+), 94 deletions(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index f1834588..babd93b2 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -488,152 +488,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) + + mock_test_user = mocker.patch( + 'wagtail_personalisation.rules.VisitCountRule.test_user', + side_effect=[True, True, False, True]) - assert form.count_matching_users([first_rule, s_rule], False) is 1 + assert form.count_matching_users([first_rule, second_rule], False) is 1 + mock_test_user.call_count == 4 From 9a86b0c8cc741dc8b8f9893af026772a9a3d922a Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 22 Feb 2018 15:14:34 +0200 Subject: [PATCH 86/97] Add tests for segment users view --- src/wagtail_personalisation/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/views.py b/src/wagtail_personalisation/views.py index 89e91198..8227367f 100644 --- a/src/wagtail_personalisation/views.py +++ b/src/wagtail_personalisation/views.py @@ -149,7 +149,7 @@ def segment_user_data(request, segment_id): response = HttpResponse(content_type='text/csv; charset=utf-8') response['Content-Disposition'] = \ - 'attachment;filename=segment-%s-users.csv' % str(segment_id) + 'attachment;filename=segment-%s-users.csv' % str(segment_id) headers = ['Username'] for rule in segment.get_rules(): From 8ced5bd81c51e7b912d4362e88a1418013f45b82 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 22 Feb 2018 15:15:01 +0200 Subject: [PATCH 87/97] Fix flake8 --- tests/unit/test_static_dynamic_segments.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index babd93b2..79118cb3 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -8,8 +8,7 @@ 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): From f898dfe0172a9569f21a11ea74c41ad4a90bde5d Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 22 Feb 2018 16:12:24 +0200 Subject: [PATCH 88/97] Actually add tests for segment users view --- tests/unit/test_views.py | 55 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 tests/unit/test_views.py diff --git a/tests/unit/test_views.py b/tests/unit/test_views.py new file mode 100644 index 00000000..b2f3cc26 --- /dev/null +++ b/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' From a677846ff7788d4e33887d3263247a0089f24cb4 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 23 Feb 2018 17:02:17 +0200 Subject: [PATCH 89/97] Bump version to 0.11.0 --- CHANGES | 5 +++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 26680bb1..97ee6833 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +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 diff --git a/docs/conf.py b/docs/conf.py index b7b2aa87..794592f3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # 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. diff --git a/setup.cfg b/setup.cfg index 3d944ccc..9fb34845 100644 --- a/setup.cfg +++ b/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} diff --git a/setup.py b/setup.py index 5f56c9b0..cab0db07 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.10.9', + version='0.11.0', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From db2f82967e59580c5248ab5fad4921d049c31511 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Sun, 25 Feb 2018 15:33:25 +0200 Subject: [PATCH 90/97] Only loop through users once when saving static segments When saving a new static segment we count the matching users and populate the segment from the database. Each of these requires us to loop through all of the users, so it's better to do them at the same time. --- src/wagtail_personalisation/forms.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index d7c2fee0..6046ec01 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -87,7 +87,7 @@ def save(self, *args, **kwargs): 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 @@ -113,18 +113,20 @@ def save(self, *args, **kwargs): User = get_user_model() users = User.objects.filter(is_active=True, is_staff=False) - take_user = takewhile( - lambda x: instance.count == 0 or len(users_to_add) <= instance.count, - users - ) - for user in take_user: + matched_count = 0 + for user in users.iterator(): request.user = user 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) - + if passes: + matched_count += 1 + if 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) From d335e4fd7b2ac16b94a599af2d0799238a8b29aa Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 26 Feb 2018 14:31:56 +0200 Subject: [PATCH 91/97] Populate static segments even if the count is 0 --- src/wagtail_personalisation/forms.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 6046ec01..b6ab7da1 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -2,12 +2,10 @@ 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 @@ -108,7 +106,6 @@ def save(self, *args, **kwargs): users_to_add = [] users_to_exclude = [] - # sessions = Session.objects.iterator() User = get_user_model() users = User.objects.filter(is_active=True, is_staff=False) @@ -119,7 +116,7 @@ def save(self, *args, **kwargs): passes = adapter._test_rules(instance.get_rules(), request, instance.match_any) if passes: matched_count += 1 - if len(users_to_add) <= instance.count: + if instance.count == 0 or len(users_to_add) <= instance.count: if instance.randomise_into_segment(): users_to_add.append(user) else: From 0efd3ae937821ee176d89590d85ebf9d7f9dd1c5 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Mon, 26 Feb 2018 14:34:02 +0200 Subject: [PATCH 92/97] Update tests for new static segment population --- tests/unit/test_static_dynamic_segments.py | 71 ++++++++-------------- 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index 79118cb3..a965d518 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -35,22 +35,18 @@ class Meta: @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) @@ -58,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( @@ -102,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 @@ -180,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), @@ -193,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 @@ -389,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() @@ -406,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() @@ -471,7 +447,7 @@ def test_rules_check_skipped_if_dynamic_segment_in_excluded(site, client, mocker @pytest.mark.django_db -def test_matched_user_count_added_to_segment_at_creation(site, client, mocker, django_user_model): +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') @@ -479,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() From c7ad3251cfaf4f6f07fbf6ac0c18e2dc206f4e30 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 1 Mar 2018 16:26:20 +0200 Subject: [PATCH 93/97] Version 0.11.1 --- CHANGES | 4 ++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 97ee6833..4039bc2f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +0.11.1 +================== + - Populate entirely static segments from registered Users not active Sessions + 0.11.0 ================== - Bug Fix: Query rule should not be static diff --git a/docs/conf.py b/docs/conf.py index 794592f3..94e28039 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.11.0' +version = '0.11.1' # The full version, including alpha/beta/rc tags. -release = '0.11.0' +release = '0.11.1' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 9fb34845..0f28eb42 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.11.0 +current_version = 0.11.1 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index cab0db07..353f65e7 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.11.0', + version='0.11.1', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 74d31230846146f418c8a0f595aeedb7a15f8eb3 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 8 Mar 2018 13:14:29 +0200 Subject: [PATCH 94/97] Ensure static segments don't have one extra user --- src/wagtail_personalisation/forms.py | 2 +- tests/unit/test_static_dynamic_segments.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index b6ab7da1..02555eb4 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -116,7 +116,7 @@ def save(self, *args, **kwargs): 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.count == 0 or len(users_to_add) < instance.count: if instance.randomise_into_segment(): users_to_add.append(user) else: diff --git a/tests/unit/test_static_dynamic_segments.py b/tests/unit/test_static_dynamic_segments.py index a965d518..d4dead86 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -45,6 +45,20 @@ def test_user_added_to_static_segment_at_creation(site, user, mocker): assert user in instance.static_users.all() +@pytest.mark.django_db +def test_user_not_added_to_full_static_segment_at_creation(site, django_user_model, mocker): + django_user_model.objects.create(username='first') + django_user_model.objects.create(username='second') + segment = SegmentFactory.build(type=Segment.TYPE_STATIC, count=1) + rule = VisitCountRule(counted_page=site.root_page) + form = form_with_data(segment, rule) + mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', + side_effect=[True, True]) + instance = form.save() + + assert len(instance.static_users.all()) == 1 + + @pytest.mark.django_db def test_anonymous_user_not_added_to_static_segment_at_creation(site, client, mocker): session = client.session From 865efd0792a85c0d992873d2bb7bea2800df5359 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Thu, 8 Mar 2018 13:58:01 +0200 Subject: [PATCH 95/97] Version 0.11.2 --- CHANGES | 4 ++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 4039bc2f..5af2af88 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +0.11.2 +================== + - Bugfix: Stop populating static segments when the count is reached + 0.11.1 ================== - Populate entirely static segments from registered Users not active Sessions diff --git a/docs/conf.py b/docs/conf.py index 94e28039..c9b27322 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.11.1' +version = '0.11.2' # The full version, including alpha/beta/rc tags. -release = '0.11.1' +release = '0.11.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 0f28eb42..b79349eb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.11.1 +current_version = 0.11.2 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index 353f65e7..2feb9b94 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.11.1', + version='0.11.2', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', From 7f5e958ee38b599f29472bb37016dff2d6587838 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Mar 2018 19:20:30 +0200 Subject: [PATCH 96/97] Catch the exception if the visit count rule doesn't have a page --- src/wagtail_personalisation/rules.py | 7 +++++++ tests/unit/test_rules_visitcount.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index 721e8672..4fa82f79 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -6,6 +6,7 @@ from django.apps import apps from django.conf import settings +from django.core.exceptions import ObjectDoesNotExist from django.contrib.sessions.models import Session from django.db import models from django.template.defaultfilters import slugify @@ -239,6 +240,12 @@ def test_user(self, request, user=None): from wagtail_personalisation.adapters import ( get_segment_adapter, SessionSegmentsAdapter, SEGMENT_ADAPTER_CLASS) + # Django formsets don't honour 'required' fields so check rule is valid + try: + self.counted_page + except ObjectDoesNotExist: + return False + if user: # Create a fake request so we can use the adapter request = RequestFactory().get('/') diff --git a/tests/unit/test_rules_visitcount.py b/tests/unit/test_rules_visitcount.py index 17dab9ce..321f0439 100644 --- a/tests/unit/test_rules_visitcount.py +++ b/tests/unit/test_rules_visitcount.py @@ -2,6 +2,7 @@ from tests.factories.rule import VisitCountRuleFactory from tests.factories.segment import SegmentFactory +from wagtail_personalisation.rules import VisitCountRule @pytest.mark.django_db @@ -25,6 +26,12 @@ def test_visit_count(site, client): assert visit_count[1]['count'] == 1 +@pytest.mark.django_db +def test_call_test_user_on_invalid_rule_fails(site, user, mocker): + rule = VisitCountRule() + assert not (rule.test_user(None, user)) + + @pytest.mark.django_db def test_visit_count_call_test_user_with_user(site, client, user): segment = SegmentFactory(name='VisitCount') From 03eb812e45e5d304c5f80a7171310265f1890007 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Fri, 9 Mar 2018 20:35:08 +0200 Subject: [PATCH 97/97] Version 0.11.3 --- CHANGES | 4 ++++ docs/conf.py | 4 ++-- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 5af2af88..a5f1c2b7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +0.11.3 +================== + - Bugfix: Handle errors when testing an invalid visit count rule + 0.11.2 ================== - Bugfix: Stop populating static segments when the count is reached diff --git a/docs/conf.py b/docs/conf.py index c9b27322..1a7483ac 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ # built documents. # # The short X.Y version. -version = '0.11.2' +version = '0.11.3' # The full version, including alpha/beta/rc tags. -release = '0.11.2' +release = '0.11.3' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index b79349eb..5faf6ce6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.11.2 +current_version = 0.11.3 commit = true tag = true tag_name = {new_version} diff --git a/setup.py b/setup.py index 2feb9b94..15b216bb 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ setup( name='wagtail-personalisation-molo', - version='0.11.2', + version='0.11.3', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org',