From 33f96af4a3c6cc171b69bf1f1e3f7a610a1017d5 Mon Sep 17 00:00:00 2001 From: Kaitlyn Crawford Date: Wed, 24 Jan 2018 15:14:24 +0200 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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,