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 933f744..5bde235 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals 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 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): # 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 0000000..7ab4e11 --- /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 82719d7..3df7731 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.rules import AbstractBaseRule 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 @@ class Segment(ClusterableModel): FieldPanel('persistent'), ]), FieldPanel('match_any'), + FieldPanel('type', widget=forms.RadioSelect), + FieldPanel('count'), ], heading="Segment"), MultiFieldPanel([ InlinePanel( @@ -70,6 +120,14 @@ class Segment(ClusterableModel): 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 @@ class Segment(ClusterableModel): 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 5d5b94e..ee98e36 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -18,6 +18,7 @@ from wagtail.wagtailadmin.edit_handlers import ( 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 0000000..53e30b1 --- /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 52d12f7..f9f7c77 100644 --- a/setup.py +++ b/setup.py @@ -18,6 +18,7 @@ tests_require = [ '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 5bde235..16260ec 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -175,7 +175,7 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): # 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 53e30b1..7815d8e 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 6f5147c..bb49e69 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 b1dc0ed..3731509 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 b51f920..6c3c9c8 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 7b9fa47..8828d7c 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 @@

{{ 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 @@ {% endif %}
  • -
  • +
  • {% trans "The persistence of this segment is" %} {% if segment.persistent %} {% trans "Persistent" %} @@ -58,7 +71,7 @@
  • {% 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 3df7731..f1e1a1a 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -169,6 +169,10 @@ class Segment(ClusterableModel): 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 @@ class Segment(ClusterableModel): 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 7815d8e..b70b806 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 f1e1a1a..2548583 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -7,6 +7,7 @@ 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.template.defaultfilters import slugify from django.test.client import RequestFactory @@ -120,10 +121,22 @@ class Segment(ClusterableModel): 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 @@ class Segment(ClusterableModel): 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 @@ class Segment(ClusterableModel): 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 b70b806..0364814 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -4,6 +4,7 @@ import datetime 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 2548583..4cd9f69 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 @@ class Segment(ClusterableModel): 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 @@ class Segment(ClusterableModel): 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 4cd9f69..5c77cbe 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -15,6 +15,7 @@ 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 ( @@ -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 7ab4e11..6196fa8 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 5c77cbe..913c363 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 | 42 ++++++++++++------- tests/unit/test_static_dynamic_segments.py | 6 +++ 3 files changed, 54 insertions(+), 14 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 0000000..d594e03 --- /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 913c363..6ebdfcc 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -9,6 +9,7 @@ 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 @@ -98,6 +99,7 @@ class Segment(ClusterableModel): ) ) sessions = models.ManyToManyField(Session) + frozen = models.BooleanField(default=False) objects = SegmentQuerySet.as_manager() @@ -149,6 +151,12 @@ class Segment(ClusterableModel): 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 @@ class Segment(ClusterableModel): if save: self.save() - def save(self, *args, **kwargs): - super(Segment, self).save(*args, **kwargs) - if self.is_static: - request = RequestFactory().get('/') +@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 self.get_rules() if rule.static) - if not self.is_consistent and all_pass: - self.sessions.add(session) + 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 0364814..a97cc0f 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 0000000..7487498 --- /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 6ebdfcc..6185c0c 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -26,6 +26,8 @@ from wagtail.wagtailcore.models import Page 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 @@ class Segment(ClusterableModel): 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 a97cc0f..9f16792 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -5,7 +5,9 @@ import datetime 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 | 270 +++++++++++---------- 3 files changed, 186 insertions(+), 176 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index 7487498..a833923 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 @@ class SegmentAdminForm(WagtailAdminModelForm): }) 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 6185c0c..4fa5f01 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 wagtail_personalisation.utils import count_active_days 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 @@ class Segment(ClusterableModel): 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 9f16792..490c7ba 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -2,132 +2,16 @@ from __future__ import absolute_import, unicode_literals 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 -@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) - 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) - - -@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() - - # We need to trigger the post init - segment = Segment.objects.get(id=segment.id) - - 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() - 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) - segment.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) - - -@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() - - 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(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(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_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) - segment.save() - - 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): +def form_with_data(segment, *rules): model_fields = ['type', 'status', 'count', 'name'] class TestSegmentAdminForm(SegmentAdminForm): @@ -138,20 +22,147 @@ def form_with_data(segment, rule): 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 + 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(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 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(site, client): + session = client.session + session.save() + client.get(site.root_page.url) + + 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), + ) + form = form_with_data(segment, static_rule, non_static_rule) + instance = form.save() + + assert instance.frozen + assert not instance.sessions.all() + + +@pytest.mark.django_db +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 instance.frozen + assert not instance.sessions.all() + + +@pytest.mark.django_db +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 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(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 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 +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.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, + ) + form = form_with_data(segment, rule) + instance = form.save() + + assert instance.frozen + assert not instance.sessions.all() + + +@pytest.mark.django_db +def test_does_not_calculate_the_segment_again(site, client, mocker): + session = client.session + session.save() + 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) + 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 + @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 16260ec..e46ed19 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -144,7 +144,7 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 a833923..4703116 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 django.utils.lru_cache import lru_cache from wagtail.wagtailadmin.forms import WagtailAdminModelForm + SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -43,15 +46,21 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 ee98e36..66a6b87 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -229,7 +229,7 @@ class VisitCountRule(AbstractBaseRule): 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 490c7ba..7752ad2 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -3,16 +3,17 @@ from __future__ import absolute_import, unicode_literals 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 @@ def form_with_data(segment, *rules): 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 4703116..02544ce 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -10,6 +10,7 @@ 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 @@ -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 d594e03..0000000 --- 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 4fa5f01..7972326 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 @@ class Segment(ClusterableModel): 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 7752ad2..fe4b05c 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 02544ce..969daa2 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -6,6 +6,7 @@ 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.core.exceptions import ValidationError from django.test.client import RequestFactory from django.utils import timezone @@ -67,3 +68,11 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 7972326..e7d5e5f 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -98,7 +98,7 @@ class Segment(ClusterableModel): ]), 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 0000000..26e43d2 --- /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 969daa2..c4a4c42 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 26e43d2..269cd4b 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 c4a4c42..487fb34 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -45,8 +45,24 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 487fb34..e8e5deb 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -47,7 +47,7 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 | 23 ++++---- .../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, 86 insertions(+), 40 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 e46ed19..e145cd9 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -175,7 +175,7 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): # 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 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 e8e5deb..a8d659a 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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) + 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.sessions.add(*sessions_to_add) + 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 0000000..ea76aa8 --- /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 e7d5e5f..cc1a6b2 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 @@ class Segment(ClusterableModel): @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 03efbd9..9f5adfc 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -44,3 +44,8 @@ class RequestFactory(BaseRequestFactory): 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 fe4b05c..778c0b5 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -36,9 +36,10 @@ def form_with_data(segment, *rules): @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 778c0b5..e840164 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 f9f7c77..a364690 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ with open('README.rst') as fh: 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 a364690..f54b566 100644 --- a/setup.py +++ b/setup.py @@ -32,12 +32,12 @@ with open('README.rst') as fh: '^.. 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 a28ac85..370d787 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 370d787..37e13f3 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 f54b566..ee1c2b8 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ with open('README.rst') as fh: 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 ee1c2b8..0d1db27 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 37e13f3..019a754 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 634c7d4..c4ca7bf 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 a8d659a..a6ea9e7 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -87,7 +87,7 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 1c77492..1f9a9f5 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 d0a6bf1..bb487c9 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 965ca75..b2f3752 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 8bb9e92..1fb79c2 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 0d1db27..2ab4f23 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 e145cd9..a52e090 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -133,12 +133,13 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 b2f3752..2c116d6 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 1fb79c2..e7073a1 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 2ab4f23..cd9aa09 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 019a754..3aedbb1 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 cd9aa09..2de0fdd 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ import re 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 c4ca7bf..8a5d1ba 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 8828d7c..a11d3fd 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 @@
    {% 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 2c116d6..398a21f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 e7073a1..9c907f7 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 2de0fdd..f83e808 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 daab476..f3d6071 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 3aedbb1..8c4237d 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 a52e090..f8f2466 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -3,7 +3,6 @@ from __future__ import absolute_import, unicode_literals 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 a6ea9e7..0c9cd4f 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -8,15 +8,11 @@ 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 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 e8eccad..a4834f5 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 cc1a6b2..f2fcce1 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -2,7 +2,6 @@ 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 from django.utils.encoding import python_2_unicode_compatible @@ -11,11 +10,7 @@ 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 @@ -125,7 +120,7 @@ class Segment(ClusterableModel): @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 b261791..49bea40 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 55ddd42..4f5a31d 100644 --- a/src/wagtail_personalisation/wagtail_hooks.py +++ b/src/wagtail_personalisation/wagtail_hooks.py @@ -7,7 +7,7 @@ from django.core.urlresolvers import reverse 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 15e185f..ac0f1c0 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 @@ TEMPLATES = [ }, ] + 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 d911ea4..b24911e 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 7b001ae..09be0f8 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 a29892f..ab79a36 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -4,16 +4,14 @@ import datetime 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 33dec18..f1fa005 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 e840164..0796699 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -2,15 +2,14 @@ from __future__ import absolute_import, unicode_literals 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 f3d6071..f5cee8a 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 8a5d1ba..34be95a 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 66a6b87..d94fc50 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 ab79a36..dc2402c 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -4,7 +4,7 @@ import datetime 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 398a21f..a78f47a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 9c907f7..0752f14 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 f83e808..6c27e71 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 d94fc50..f4e6fa7 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 0c9cd4f..5eb1bf5 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 f2fcce1..2f9bfcf 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 0000000..ae7bdd1 --- /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 0796699..7718b47 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -8,7 +8,8 @@ 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 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 5eb1bf5..9d342d9 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 7718b47..8dcabb6 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -357,3 +357,25 @@ def test_count_matching_users_handles_match_all(site, client, django_user_model) 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 8dcabb6..6ef91a9 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -4,8 +4,10 @@ import datetime 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_count_matching_users_handles_match_all(site, client, django_user_model) 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 6ef91a9..c43e776 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -4,10 +4,8 @@ import datetime 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 9d342d9..a946158 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -39,7 +39,7 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 c43e776..686e89f 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -286,6 +286,48 @@ def test_count_users_matching_static_rules(site, client, django_user_model): 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 a11d3fd..454a82e 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 @@ {% 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 1f9a9f5..aa39a29 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 a78f47a..dbcd3e0 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 0752f14..009384c 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 6c27e71..026a589 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 2f9bfcf..9d27eb5 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals 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 @@ class Segment(ClusterableModel): 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 686e89f..f5fce7c 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -13,7 +13,7 @@ from wagtail_personalisation.rules import (AbstractBaseRule, TimeRule, 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 454a82e..d6858dd 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 @@ {% 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 0000000..bd68335 --- /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 9d27eb5..48d176b 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 @@ class Segment(ClusterableModel): 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 f8f2466..8d1ada8 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -184,13 +184,12 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 a946158..6183f4c 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -119,7 +119,7 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 f5fce7c..4abd6ed 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 4abd6ed..b43a296 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 aa39a29..74234b8 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 dbcd3e0..f282bd7 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 009384c..34a404c 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 026a589..c180248 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 8c4237d..305772f 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 305772f..3a78086 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 8d1ada8..1012e7d 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -184,12 +184,12 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 b43a296..b7aae7c 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 0000000..bafa477 --- /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 48d176b..7a4063e 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 1012e7d..a4222f7 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -190,6 +190,9 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 6183f4c..92f5f8e 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -107,6 +107,7 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 a4222f7..5001fac 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -177,6 +177,8 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 b7aae7c..8b7181d 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 74234b8..e3543bf 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 f282bd7..567a811 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 34a404c..a6a243b 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 c180248..b783cfe 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 5001fac..80518eb 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -193,7 +193,7 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): segment.static_users.add(self.request.user) additional_segments.append(segment) elif result: - if self.request.user.is_authenticated(): + if segment.is_static and self.request.user.is_authenticated(): segment.excluded_users.add(self.request.user) 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 80518eb..176a498 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -89,11 +89,13 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): serialized_segments.append(serialized) segment_ids.add(segment.pk) - self.request.session['segments'] = serialized_segments - self._segment_cache = cache_segments + self.request.session[key] = serialized_segments + if key == "segments": + self._segment_cache = cache_segments def get_segment_by_id(self, segment_id): """Find and return a single segment from the request session. @@ -171,6 +174,7 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 176a498..7c84f42 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -66,17 +66,21 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): self.request.session.setdefault('segments', []) self._segment_cache = None - def get_segments(self): + def get_segments(self, key="segments"): """Return the persistent segments stored in the request session. + :param key: The key under which the segments are stored + :type key: String :returns: The segments in the request session :rtype: list of wagtail_personalisation.models.Segment or empty list """ - if self._segment_cache is not None: + if key == "segments" and self._segment_cache is not None: return self._segment_cache - raw_segments = self.request.session['segments'] + if key not in self.request.session: + return [] + raw_segments = self.request.session[key] segment_ids = [segment['id'] for segment in raw_segments] segments = ( @@ -86,7 +90,8 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): .in_bulk(segment_ids)) retval = [segments[pk] for pk in segment_ids if pk in segments] - self._segment_cache = retval + if key == "segments": + self._segment_cache = retval return retval def set_segments(self, segments, key="segments"): @@ -174,14 +179,15 @@ class SessionSegmentsAdapter(BaseSegmentsAdapter): 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 8b7181d..dbd3a45 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 dbd3a45..f183458 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 e3543bf..d8dc077 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 567a811..b7b2aa8 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # built documents. # # The short X.Y version. -version = '0.10.7' +version = '0.10.8' # The full version, including alpha/beta/rc tags. -release = '0.10.7' +release = '0.10.8' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index a6a243b..3d944cc 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 b783cfe..f455ba2 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: setup( name='wagtail-personalisation-molo', - version='0.10.7', + version='0.10.8', description='A forked version of Wagtail add-on for showing personalized content', author='Praekelt.org', author_email='dev@praekeltfoundation.org', 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 d6858dd..1fe3756 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 @@
  • {% 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 1fe3756..b0fcf2e 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 @@
  • {% 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 d8dc077..26680bb 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 f455ba2..5f56c9b 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 92f5f8e..d7c2fee 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -108,22 +108,22 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 f4e6fa7..5316002 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 dc2402c..0ce5b7c 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 5316002..6461ea7 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -2,17 +2,23 @@ from __future__ import absolute_import, unicode_literals import re from datetime import datetime +from importlib import import_module from django.apps import apps +from django.conf import settings +from django.contrib.sessions.models import Session from django.db import models from django.template.defaultfilters import slugify from django.utils.encoding import force_text, python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ +from django.test.client import RequestFactory from modelcluster.fields import ParentalKey from user_agents import parse from wagtail.wagtailadmin.edit_handlers import ( FieldPanel, FieldRowPanel, PageChooserPanel) +SessionStore = import_module(settings.SESSION_ENGINE).SessionStore + @python_2_unicode_compatible class AbstractBaseRule(models.Model): @@ -221,17 +227,33 @@ class VisitCountRule(AbstractBaseRule): 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 a4d7d60..f153e3c 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 6461ea7..b1bdb28 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 @@ class VisitCountRule(AbstractBaseRule): 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 b1bdb28..721e867 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -282,6 +282,28 @@ class VisitCountRule(AbstractBaseRule): ), } + def get_column_header(self): + return "Visit count - %s" % self.counted_page + + def get_user_info_string(self, user): + # Local import for cyclic import + from wagtail_personalisation.adapters import ( + get_segment_adapter, SessionSegmentsAdapter, SEGMENT_ADAPTER_CLASS) + + # Create a fake request so we can use the adapter + request = RequestFactory().get('/') + request.user = user + + # If we're using the session adapter check for an active session + if SEGMENT_ADAPTER_CLASS == SessionSegmentsAdapter: + request.session = self._get_user_session(user) + else: + request.session = SessionStore() + + adapter = get_segment_adapter(request) + visit_count = adapter.get_visit_count(self.counted_page) + return str(visit_count) + class QueryRule(AbstractBaseRule): """Query rule to segment users based on matching queries. diff --git a/tests/unit/test_rules_visitcount.py b/tests/unit/test_rules_visitcount.py index f153e3c..17dab9c 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 87848c0..e5c978c 100644 --- a/src/wagtail_personalisation/admin_urls.py +++ b/src/wagtail_personalisation/admin_urls.py @@ -13,4 +13,6 @@ urlpatterns = [ views.copy_page_view, name='copy_page'), url(r'^segment/toggle_segment_view/$', views.toggle_segment_view, name='toggle_segment_view'), + url(r'^segment/users/(?P[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 b0fcf2e..fc314c6 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 @@
  • 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 30ee913..89e9119 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 f183458..babd93b 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) - assert form.count_matching_users([first_rule, s_rule], False) is 1 + mock_test_user = mocker.patch( + 'wagtail_personalisation.rules.VisitCountRule.test_user', + side_effect=[True, True, False, True]) + + assert form.count_matching_users([first_rule, second_rule], False) is 1 + mock_test_user.call_count == 4 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 89e9119..8227367 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 babd93b..79118cb 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -8,8 +8,7 @@ from django.forms.models import model_to_dict from tests.factories.segment import SegmentFactory from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment -from wagtail_personalisation.rules import (AbstractBaseRule, TimeRule, - VisitCountRule) +from wagtail_personalisation.rules import TimeRule, VisitCountRule def form_with_data(segment, *rules): 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 0000000..b2f3cc2 --- /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 26680bb..97ee683 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 b7b2aa8..794592f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # built documents. # # The short X.Y version. -version = '0.10.8' +version = '0.11.0' # The full version, including alpha/beta/rc tags. -release = '0.10.8' +release = '0.11.0' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.cfg b/setup.cfg index 3d944cc..9fb3484 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 5f56c9b..cab0db0 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/wagtail_personalisation/forms.py b/src/wagtail_personalisation/forms.py index d7c2fee..6046ec0 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -87,7 +87,7 @@ class SegmentAdminForm(WagtailAdminModelForm): if not self.instance.is_static: self.instance.count = 0 - if is_new: + if is_new and self.instance.is_static and not self.instance.all_rules_static: rules = [ form.instance for formset in self.formsets.values() for form in formset @@ -113,18 +113,20 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 6046ec0..b6ab7da 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -2,12 +2,10 @@ from __future__ import absolute_import, unicode_literals from datetime import datetime from importlib import import_module -from itertools import takewhile from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser -from django.contrib.sessions.models import Session from django.contrib.staticfiles.templatetags.staticfiles import static from django.test.client import RequestFactory from django.utils.lru_cache import lru_cache @@ -108,7 +106,6 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 79118cb..a965d51 100644 --- a/tests/unit/test_static_dynamic_segments.py +++ b/tests/unit/test_static_dynamic_segments.py @@ -35,22 +35,18 @@ def form_with_data(segment, *rules): @pytest.mark.django_db -def test_session_added_to_static_segment_at_creation(site, client, user): - session = client.session - session.save() - client.force_login(user) - client.get(site.root_page.url) - +def test_user_added_to_static_segment_at_creation(site, user, mocker): segment = SegmentFactory.build(type=Segment.TYPE_STATIC) rule = VisitCountRule(counted_page=site.root_page) form = form_with_data(segment, rule) + mocker.patch('wagtail_personalisation.rules.VisitCountRule.test_user', return_value=True) instance = form.save() assert user in instance.static_users.all() @pytest.mark.django_db -def test_anonymous_user_not_added_to_static_segment_at_creation(site, client): +def test_anonymous_user_not_added_to_static_segment_at_creation(site, client, mocker): session = client.session session.save() client.get(site.root_page.url) @@ -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 97ee683..4039bc2 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 794592f..94e2803 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 9fb3484..0f28eb4 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 cab0db0..353f65e 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 b6ab7da..02555eb 100644 --- a/src/wagtail_personalisation/forms.py +++ b/src/wagtail_personalisation/forms.py @@ -116,7 +116,7 @@ class SegmentAdminForm(WagtailAdminModelForm): 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 a965d51..d4dead8 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 4039bc2..5af2af8 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 94e2803..c9b2732 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 0f28eb4..b79349e 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 353f65e..2feb9b9 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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 721e867..4fa82f7 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -6,6 +6,7 @@ from importlib import import_module 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 @@ class VisitCountRule(AbstractBaseRule): 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 17dab9c..321f043 100644 --- a/tests/unit/test_rules_visitcount.py +++ b/tests/unit/test_rules_visitcount.py @@ -2,6 +2,7 @@ import pytest 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 5af2af8..a5f1c2b 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 c9b2732..1a7483a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,10 +55,10 @@ author = 'Lab Digital BV' # 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 b79349e..5faf6ce 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 2feb9b9..15b216b 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ with open('README.rst') as fh: 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',