diff --git a/README.rst b/README.rst index 1c2ba048..5825b420 100644 --- a/README.rst +++ b/README.rst @@ -41,14 +41,16 @@ To install the package with pip:: pip install wagtail-personalisation -Next, include the ``wagtail_personalisation``, ``wagtail.contrib.modeladmin`` -and ``wagtailfontawesome`` apps in your project's ``INSTALLED_APPS``: +Next, include the ``wagtail_personalisation``, ``wagtail.contrib.modeladmin``, +``'wagtail.contrib.settings'`` and ``wagtailfontawesome`` apps in your project's +``INSTALLED_APPS``: .. code-block:: python INSTALLED_APPS = [ # ... 'wagtail.contrib.modeladmin', + 'wagtail.contrib.settings', 'wagtail_personalisation', 'wagtailfontawesome', # ... diff --git a/sandbox/exampledata/personalisation.json b/sandbox/exampledata/personalisation.json index 93205e53..a4baeb47 100644 --- a/sandbox/exampledata/personalisation.json +++ b/sandbox/exampledata/personalisation.json @@ -24,7 +24,6 @@ "edit_date": "2017-06-02T10:58:39.399Z", "enable_date": "2017-06-02T10:58:39.389Z", "disable_date": "2017-06-02T10:34:51.722Z", - "visit_count": 0, "status": "enabled", "persistent": false, "match_any": false @@ -38,7 +37,6 @@ "edit_date": "2017-06-02T10:57:44.504Z", "enable_date": "2017-06-02T10:57:44.497Z", "disable_date": "2017-06-02T10:57:39.984Z", - "visit_count": 1, "status": "enabled", "persistent": false, "match_any": false diff --git a/sandbox/sandbox/settings.py b/sandbox/sandbox/settings.py index ac251455..22c82d20 100644 --- a/sandbox/sandbox/settings.py +++ b/sandbox/sandbox/settings.py @@ -45,6 +45,7 @@ 'wagtail.wagtailadmin', 'wagtail.wagtailcore', 'wagtail.contrib.modeladmin', + 'wagtail.contrib.settings', 'wagtailfontawesome', 'modelcluster', diff --git a/src/wagtail_personalisation/adapters.py b/src/wagtail_personalisation/adapters.py index 933f744a..6c9b94fb 100644 --- a/src/wagtail_personalisation/adapters.py +++ b/src/wagtail_personalisation/adapters.py @@ -1,7 +1,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 wagtail_personalisation.models import Segment @@ -36,7 +35,7 @@ def add(self): def refresh(self): """Refresh the segments stored in the adapter storage.""" - def _test_rules(self, rules, request, match_any=False): + def _test_rules(self, rules, match_any=False): """Tests the provided rules to see if the request still belongs to a segment. :param rules: The rules to test for @@ -50,9 +49,20 @@ def _test_rules(self, rules, request, match_any=False): """ if not rules: return False + + if not hasattr(self.request, 'matched_rules'): + self.request.matched_rules = [] + + results = [] + for rule in rules: + validation = rule.test_user(self.request) + if validation: + self.request.matched_rules.append(rule.unique_encoded_name) + results.append(validation) + if match_any: - return any(rule.test_user(request) for rule in rules) - return all(rule.test_user(request) for rule in rules) + return any(results) + return all(results) class Meta: abstract = True @@ -150,17 +160,6 @@ def get_visit_count(self, page=None): return visit['count'] return 0 - def update_visit_count(self): - """Update the visit count for all segments in the request session.""" - segments = self.request.session['segments'] - segment_pks = [s['id'] for s in segments] - - # Update counts - (Segment.objects - .enabled() - .filter(pk__in=segment_pks) - .update(visit_count=F('visit_count') + 1)) - def refresh(self): """Retrieve the request session segments and verify whether or not they still apply to the requesting visitor. @@ -178,14 +177,13 @@ def refresh(self): 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) + result = self._test_rules( + segment_rules, match_any=segment.match_any) if result: additional_segments.append(segment) self.set_segments(current_segments + additional_segments) - self.update_visit_count() SEGMENT_ADAPTER_CLASS = import_string(getattr( diff --git a/src/wagtail_personalisation/migrations/0013_detailed-segment-visits.py b/src/wagtail_personalisation/migrations/0013_detailed-segment-visits.py new file mode 100644 index 00000000..27be080d --- /dev/null +++ b/src/wagtail_personalisation/migrations/0013_detailed-segment-visits.py @@ -0,0 +1,80 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.4 on 2017-08-25 07:02 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('wagtail_personalisation', '0012_remove_personalisablepagemetadata_is_segmented'), + ] + + operations = [ + migrations.CreateModel( + name='PersonalisationSettings', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('detailed_visits', models.BooleanField(default=False, help_text='Enable to gather more detailed metadata about the visits to your segments and the rules that matched. Please note that this will create additional load on your database. Usage of caching is recommended.')), + ('reverse_match', models.BooleanField(default=False, help_text='Enable to reverse match past visits with users as soon as a user logs in. This will ensure your data is as complete as possible.')), + ('site', models.OneToOneField(editable=False, on_delete=django.db.models.deletion.CASCADE, to='wagtailcore.Site')), + ], + options={ + 'abstract': False, + }, + ), + migrations.CreateModel( + name='SegmentVisit', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('session', models.CharField(db_index=True, editable=False, max_length=64, null=True)), + ('visit_date', models.DateTimeField(auto_now_add=True)), + ('page', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='wagtailcore.Page')), + ], + ), + migrations.CreateModel( + name='SegmentVisitMetadata', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('matched_rules', models.CharField(max_length=2048)), + ], + ), + migrations.RemoveField( + model_name='segment', + name='visit_count', + ), + migrations.AddField( + model_name='segmentvisitmetadata', + name='segment', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='wagtail_personalisation.Segment'), + ), + migrations.AddField( + model_name='segmentvisitmetadata', + name='visit', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='wagtail_personalisation.SegmentVisit'), + ), + migrations.AddField( + model_name='segmentvisit', + name='segments', + field=models.ManyToManyField(through='wagtail_personalisation.SegmentVisitMetadata', to='wagtail_personalisation.Segment'), + ), + migrations.AddField( + model_name='segmentvisit', + name='served_segment', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='served_segment', to='wagtail_personalisation.Segment'), + ), + migrations.AddField( + model_name='segmentvisit', + name='served_variant', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='served_variant', to='wagtailcore.Page'), + ), + migrations.AddField( + model_name='segmentvisit', + name='user', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/src/wagtail_personalisation/models.py b/src/wagtail_personalisation/models.py index 82719d77..0cee1a4c 100644 --- a/src/wagtail_personalisation/models.py +++ b/src/wagtail_personalisation/models.py @@ -1,11 +1,14 @@ from __future__ import absolute_import, unicode_literals +from django.conf import settings +from django.contrib.auth.signals import user_logged_in from django.db import models, transaction from django.template.defaultfilters import slugify from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property from django.utils.translation import ugettext_lazy as _ from modelcluster.models import ClusterableModel +from wagtail.contrib.settings.models import BaseSetting, register_setting from wagtail.wagtailadmin.edit_handlers import ( FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel) from wagtail.wagtailcore.models import Page @@ -14,6 +17,29 @@ from wagtail_personalisation.utils import count_active_days +@register_setting(icon='fa-magic') +class PersonalisationSettings(BaseSetting): + detailed_visits = models.BooleanField( + default=False, + help_text=_('Enable to gather more detailed metadata about the visits ' + 'to your segments and the rules that matched. ' + 'Please note that this will create additional load on your ' + 'database. Usage of caching is recommended.')) + reverse_match = models.BooleanField( + default=False, + help_text=_('Enable to reverse match past visits with users as soon as ' + 'a user logs in. This will ensure your data is as complete ' + 'as possible.')) + + panels = [ + MultiFieldPanel([ + FieldPanel('detailed_visits'), + FieldPanel('reverse_match'), + ], heading='Analytics' + ) + ] + + class SegmentQuerySet(models.QuerySet): def enabled(self): return self.filter(status=self.model.STATUS_ENABLED) @@ -35,7 +61,6 @@ class Segment(ClusterableModel): edit_date = models.DateTimeField(auto_now=True) enable_date = models.DateTimeField(null=True, editable=False) disable_date = models.DateTimeField(null=True, editable=False) - visit_count = models.PositiveIntegerField(default=0, editable=False) status = models.CharField( max_length=20, choices=STATUS_CHOICES, default=STATUS_ENABLED) persistent = models.BooleanField( @@ -74,10 +99,27 @@ def encoded_name(self): """Return a string with a slug for the segment.""" return slugify(self.name.lower()) - def get_active_days(self): + @property + def active_days(self): """Return the amount of days the segment has been active.""" return count_active_days(self.enable_date, self.disable_date) + def get_visits(self): + """Return the segment visits.""" + return SegmentVisit.objects.filter(segments=self) + + @property + def visit_count(self): + """Returns the total amount of segment visits.""" + return self.get_visits().count() + + def get_serves(self): + return SegmentVisit.objects.filter(served_segment=self) + + @property + def serve_count(self): + return self.get_serves().count() + def get_used_pages(self): """Return the pages that have variants using this segment.""" pages = list(PersonalisablePageMetadata.objects.filter(segment=self)) @@ -107,6 +149,98 @@ def toggle(self, save=True): self.save() +class SegmentVisitMetadata(models.Model): + visit = models.ForeignKey( + 'wagtail_personalisation.SegmentVisit', on_delete=models.CASCADE) + segment = models.ForeignKey( + 'wagtail_personalisation.Segment', on_delete=models.SET_NULL, null=True) + matched_rules = models.CharField(max_length=2048) + + +class SegmentVisit(models.Model): + user = models.ForeignKey( + settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True) + page = models.ForeignKey(Page, on_delete=models.SET_NULL, null=True) + segments = models.ManyToManyField(Segment, through=SegmentVisitMetadata) + served_segment = models.ForeignKey( + Segment, on_delete=models.CASCADE, + related_name='served_segment', null=True) + served_variant = models.ForeignKey( + Page, on_delete=models.SET_NULL, + related_name='served_variant', null=True) + session = models.CharField( + max_length=64, editable=False, null=True, db_index=True) + visit_date = models.DateTimeField(auto_now_add=True) + + class Meta: + ordering = ['-visit_date'] + + @classmethod + def create_segment_visit(cls, page, request, metadata=None): + """Create a segment visit object. + :param page: The page being visited + :type page: wagtail.wagtailcore.models.Page + :param request: The http request + :type request: django.http.HttpRequest + :param metadata: A list of personalisable page metadata + :type page: wagtail_personalisation.models.PersonalisablePageMetadata + :returns: A committed Segment Visit object + :rtype: wagtail_personalisation.models.SegmentVisit + """ + from wagtail_personalisation.adapters import get_segment_adapter + wxp_settings = PersonalisationSettings.for_site(request.site) + + if wxp_settings.detailed_visits: + adapter = get_segment_adapter(request) + user_segments = adapter.get_segments() + + if not metadata: + metadata = page.personalisation_metadata + metadata = metadata.metadata_for_segments(user_segments) + + user = request.user if request.user.is_authenticated else None + visit = cls.objects.create( + user=user, + page=page, + served_segment=metadata.first().segment, + served_variant=metadata.first().variant, + session=request.session.session_key + ) + + for segment in user_segments: + rules = [ + rule for rule in segment.get_rules() if rule.unique_encoded_name + in request.matched_rules + ] + + SegmentVisitMetadata.objects.create( + visit=visit, + segment=segment, + matched_rules=','.join( + rule.unique_encoded_name for rule in rules) + ) + + return visit + + @classmethod + def reverse_match(cls, user): + user_visits = cls.objects.filter(user=user) + + for visit in user_visits: + cls.objects.filter( + session=visit.session, + user__isnull=True + ).update(user=user) + + +def reverse_match(sender, request, user, **kwargs): + wxp_settings = PersonalisationSettings.for_site(request.site) + if wxp_settings.detailed_visits and wxp_settings.reverse_match: + SegmentVisit.reverse_match(user) + +user_logged_in.connect(reverse_match) + + class PersonalisablePageMetadata(ClusterableModel): """The personalisable page model. Allows creation of variants with linked segments. diff --git a/src/wagtail_personalisation/receivers.py b/src/wagtail_personalisation/receivers.py index 722f09d7..12c5254b 100644 --- a/src/wagtail_personalisation/receivers.py +++ b/src/wagtail_personalisation/receivers.py @@ -14,7 +14,6 @@ def check_status_change(sender, instance, *args, **kwargs): if original_status != instance.status: if instance.status == instance.STATUS_ENABLED: instance.enable_date = timezone.now() - instance.visit_count = 0 return instance if instance.status == instance.STATUS_DISABLED: instance.disable_date = timezone.now() diff --git a/src/wagtail_personalisation/rules.py b/src/wagtail_personalisation/rules.py index 5d5b94e9..ddd56567 100644 --- a/src/wagtail_personalisation/rules.py +++ b/src/wagtail_personalisation/rules.py @@ -2,11 +2,14 @@ import re from datetime import datetime +from decimal import * from django.apps import apps from django.db import models +from django.db.models import Q from django.template.defaultfilters import slugify from django.utils.encoding import force_text, python_2_unicode_compatible +from django.utils.functional import cached_property from django.utils.translation import ugettext_lazy as _ from modelcluster.fields import ParentalKey from user_agents import parse @@ -40,6 +43,10 @@ def encoded_name(self): """Return a string with a slug for the rule.""" return slugify(force_text(self).lower()) + @cached_property + def unique_encoded_name(self): + return '%s-%d' % (self.encoded_name(), self.pk) + def description(self): """Return a description explaining the functionality of the rule. Used in the segmentation dashboard. @@ -55,6 +62,26 @@ def description(self): return description + @property + def hit_percentage(self): + percentage = round(Decimal( + (self.hit_count / self.visit_count) * 100 + ), 2) if self.hit_count > 0 else 0 + return '%d' % percentage + + @property + def hit_count(self): + from wagtail_personalisation.models import SegmentVisitMetadata + + hits = SegmentVisitMetadata.objects.filter( + Q(matched_rules__contains=self.unique_encoded_name), + segment=self.segment) + return hits.count() + + @property + def visit_count(self): + return self.segment.visit_count + @classmethod def get_descendant_models(cls): return [model for model in apps.get_models() diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html index 7b9fa47b..afd72c17 100644 --- a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html @@ -22,13 +22,13 @@

{% trans 'Filter' %}

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

{{ segment }}

  • {% trans "This segment has been visited" %} - {{ segment.visit_count|localize }} {% trans "time" %}{{ segment.visit_count|pluralize }} + {{ segment.visit_count }} {% trans "time" %}{{ segment.visit_count|pluralize }}
  • {% trans "This segment has been active for" %} @@ -77,7 +77,7 @@

    {{ segment }}

    {% elif segment.status == segment.STATUS_ENABLED %}
  • disable
  • {% endif %} -
  • configure this
  • +
  • configure
{% endif %}
diff --git a/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/inspect.html b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/inspect.html new file mode 100644 index 00000000..c7d0d592 --- /dev/null +++ b/src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/inspect.html @@ -0,0 +1,39 @@ +{% extends "modeladmin/inspect.html" %} + +{% load i18n %} +{% load wagtailadmin_tags %} + +{% block content_main %} + +
+ {% block fields_output %} +
+
+ {{ block.super }} +
+
+ {% endblock %} + + {% block rule_data %} +
+
+
+ {% for rule in instance.get_rules %} +
{{ rule }}
+
+ {{ rule.description.title }} {{ rule.description.value|lower }}
+ {{ rule.hit_percentage }}% ({{ rule.hit_count }} of {{ rule.visit_count }} visit{{ rule.visit_count|pluralize }}) +
+ {% endfor %} +
+
+
+ {% endblock %} +
+{% endblock %} diff --git a/src/wagtail_personalisation/views.py b/src/wagtail_personalisation/views.py index 30ee9139..7e93c175 100644 --- a/src/wagtail_personalisation/views.py +++ b/src/wagtail_personalisation/views.py @@ -5,11 +5,12 @@ from django.http import 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 +from wagtail.contrib.modeladmin.options import ( + ModelAdmin, ModelAdminGroup, modeladmin_register) from wagtail.contrib.modeladmin.views import IndexView from wagtail.wagtailcore.models import Page -from wagtail_personalisation.models import Segment +from wagtail_personalisation.models import Segment, SegmentVisit class SegmentModelIndexView(IndexView): @@ -32,16 +33,18 @@ def get_template_names(self): ] -@modeladmin_register class SegmentModelAdmin(ModelAdmin): """The model admin for the Segments administration interface.""" model = Segment index_view_class = SegmentModelIndexView dashboard_view_class = SegmentModelDashboardView + inspect_view_enabled = True menu_icon = 'fa-snowflake-o' add_to_settings_menu = False - list_display = ('name', 'persistent', 'match_any', 'status', - 'page_count', 'variant_count', 'statistics') + # TODO: Only show additional fields when 'detailed visits' is enabled. + list_display = ( + 'name', 'persistent', 'match_any', 'status', 'page_count', + 'variant_count', 'active_days', 'visit_count', 'serve_count') index_view_extra_js = ['js/commons.js', 'js/index.js'] index_view_extra_css = ['css/index.css'] form_view_extra_js = ['js/commons.js', 'js/form.js'] @@ -63,9 +66,28 @@ def page_count(self, obj): def variant_count(self, obj): return len(obj.get_created_variants()) - def statistics(self, obj): - return _("{visits} visits in {days} days").format( - visits=obj.visit_count, days=obj.get_active_days()) + +class SegmentVisitModelAdmin(ModelAdmin): + model = SegmentVisit + menu_icon = 'fa-rocket' + list_display = ( + 'page', 'segments_display', 'served_segment', 'served_variant', 'user', + 'session', 'visit_date') + list_filter = ('page', 'served_segment', 'served_variant', 'user') + search_fields = ('segments', 'user' 'session') + + def segments_display(self, obj): + return [segment.__str__() for segment in obj.segments.all()] + + segments_display.short_description = 'Segments' + + +class PersonalisationAdminGroup(ModelAdminGroup): + menu_label = 'Personalisation' + menu_icon = 'fa-magic' + items = (SegmentModelAdmin, SegmentVisitModelAdmin) + +modeladmin_register(PersonalisationAdminGroup) def toggle_segment_view(request): diff --git a/src/wagtail_personalisation/wagtail_hooks.py b/src/wagtail_personalisation/wagtail_hooks.py index 55ddd423..f4caab8b 100644 --- a/src/wagtail_personalisation/wagtail_hooks.py +++ b/src/wagtail_personalisation/wagtail_hooks.py @@ -14,6 +14,7 @@ from wagtail_personalisation import admin_urls, models, utils from wagtail_personalisation.adapters import get_segment_adapter +from wagtail_personalisation.models import SegmentVisit logger = logging.getLogger(__name__) @@ -85,6 +86,7 @@ def serve_variant(page, request, serve_args, serve_kwargs): metadata = metadata.metadata_for_segments(user_segments) if metadata: variant = metadata.first().variant.specific + SegmentVisit.create_segment_visit(page, request, metadata) return variant.serve(request, *serve_args, **serve_kwargs) diff --git a/tests/unit/test_adapter_session.py b/tests/unit/test_adapter_session.py index fa9bf96b..b08a62ce 100644 --- a/tests/unit/test_adapter_session.py +++ b/tests/unit/test_adapter_session.py @@ -67,36 +67,3 @@ def test_add_page_visit(rf, site): assert request.session['visit_count'][0]['count'] == 2 assert adapter.get_visit_count() == 2 - - -@pytest.mark.django_db -def test_update_visit_count(rf, site): - request = rf.get('/') - - adapter = adapters.SessionSegmentsAdapter(request) - - segment_1 = SegmentFactory(name='segment-1', persistent=True, visit_count=0) - segment_2 = SegmentFactory(name='segment-2', persistent=True, visit_count=0) - - adapter.set_segments([segment_1, segment_2]) - adapter.update_visit_count() - - segment_1.refresh_from_db() - segment_2.refresh_from_db() - - assert segment_1.visit_count == 1 - assert segment_2.visit_count == 1 - - -@pytest.mark.django_db -def test_update_visit_count_deleted_segment(rf, site): - request = rf.get('/') - - adapter = adapters.SessionSegmentsAdapter(request) - - segment_1 = SegmentFactory(name='segment-1', persistent=True, visit_count=0) - segment_2 = SegmentFactory(name='segment-2', persistent=True, visit_count=0) - - adapter.set_segments([segment_1, segment_2]) - segment_2.delete() - adapter.update_visit_count()