diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 0000000..eae8c08 --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,5 @@ +include LICENSE +include README.rst +include CHANGELOG.txt +recursive-include experiments/static * +recursive-include experiments/templates * diff --git a/README.rst b/README.rst index 0827769..51f5425 100644 --- a/README.rst +++ b/README.rst @@ -14,7 +14,7 @@ This module supports the creation of A/B testing experiments within a Wagtail si Installation ------------ -wagtail-experiments is compatible with Wagtail 1.7, and Django 1.8 to 1.10. To install:: +wagtail-experiments is compatible with Wagtail 2.3, and Django 2+ To install:: pip install wagtail-experiments diff --git a/createmigrations.py b/createmigrations.py new file mode 100644 index 0000000..6e663ff --- /dev/null +++ b/createmigrations.py @@ -0,0 +1,9 @@ +#!/usr/bin/env python + +import sys +import os + +from django.core.management import execute_from_command_line + +os.environ['DJANGO_SETTINGS_MODULE'] = 'tests.settings' +execute_from_command_line([sys.argv[0], 'makemigrations']) diff --git a/experiments/admin_urls.py b/experiments/admin_urls.py index 4134b62..6be01ef 100644 --- a/experiments/admin_urls.py +++ b/experiments/admin_urls.py @@ -1,10 +1,12 @@ from __future__ import absolute_import, unicode_literals -from django.conf.urls import url +from django.urls import path from experiments import views +app_name='experiments' + urlpatterns = [ - url(r'^experiment/report/(\d+)/$', views.experiment_report, name='report'), - url(r'^experiment/select_winner/(\d+)/(\d+)/$', views.select_winner, name='select_winner'), - url(r'^experiment/report/preview/(\d+)/(\d+)/$', views.preview_for_report, name='preview_for_report'), + path('experiment/report//', views.experiment_report, name='report'), + path('experiment/select_winner///', views.select_winner, name='select_winner'), + path('experiment/report/preview///', views.preview_for_report, name='preview_for_report'), ] diff --git a/experiments/middleware.py b/experiments/middleware.py new file mode 100644 index 0000000..3bbeb17 --- /dev/null +++ b/experiments/middleware.py @@ -0,0 +1,24 @@ +try: + from django.utils.deprecation import MiddlewareMixin +except ImportError: + MiddlewareMixin = object + +from . models import Experiment +from .utils import get_user_id + + +class GoalURLMiddleware(MiddlewareMixin): + def process_request(self, request): + current_url = request.path + # does the current URL matches the goal URL for a live experiment? + experiments = Experiment.objects.filter( + goal_url__contains=current_url, + status='live' + ) + if experiments.exists(): + # let's complete all experiment that match this URL + user_id = get_user_id(request) + for experiment in experiments: + experiment.record_completion_for_user(user_id, request) + # If the current_url is not an experiment's goal_url, then don't do anything + return None diff --git a/experiments/migrations/0001_initial.py b/experiments/migrations/0001_initial.py index d7a7899..d63ae42 100644 --- a/experiments/migrations/0001_initial.py +++ b/experiments/migrations/0001_initial.py @@ -3,6 +3,7 @@ from django.db import migrations, models import modelcluster.fields +import django.db.models.deletion class Migration(migrations.Migration): @@ -29,8 +30,8 @@ class Migration(migrations.Migration): ('id', models.AutoField(primary_key=True, auto_created=True, verbose_name='ID', serialize=False)), ('name', models.CharField(max_length=255)), ('slug', models.SlugField(max_length=255)), - ('control_page', models.ForeignKey(related_name='+', to='wagtailcore.Page')), - ('goal', models.ForeignKey(related_name='+', to='wagtailcore.Page')), + ('control_page', models.ForeignKey(related_name='+', to='wagtailcore.Page', on_delete=django.db.models.deletion.SET_NULL)), + ('goal', models.ForeignKey(related_name='+', to='wagtailcore.Page', on_delete=django.db.models.deletion.SET_NULL)), ], options={ 'abstract': False, @@ -44,6 +45,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='alternative', name='page', - field=models.ForeignKey(related_name='+', to='wagtailcore.Page'), + field=models.ForeignKey(related_name='+', to='wagtailcore.Page', on_delete=django.db.models.deletion.SET_NULL), ), ] diff --git a/experiments/migrations/0002_experiment_history.py b/experiments/migrations/0002_experiment_history.py index ba88bb3..156abbe 100644 --- a/experiments/migrations/0002_experiment_history.py +++ b/experiments/migrations/0002_experiment_history.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from django.db import migrations, models +import django.db.models.deletion class Migration(migrations.Migration): @@ -19,8 +20,8 @@ class Migration(migrations.Migration): ('date', models.DateField()), ('participant_count', models.PositiveIntegerField(default=0)), ('completion_count', models.PositiveIntegerField(default=0)), - ('experiment', models.ForeignKey(to='experiments.Experiment', related_name='history')), - ('variation', models.ForeignKey(to='wagtailcore.Page', related_name='+')), + ('experiment', models.ForeignKey(to='experiments.Experiment', related_name='history', on_delete=django.db.models.deletion.SET_NULL)), + ('variation', models.ForeignKey(to='wagtailcore.Page', related_name='+', on_delete=django.db.models.deletion.SET_NULL)), ], ), migrations.AlterUniqueTogether( diff --git a/experiments/migrations/0006_auto_20180213_1541.py b/experiments/migrations/0006_auto_20180213_1541.py new file mode 100644 index 0000000..0ecfdba --- /dev/null +++ b/experiments/migrations/0006_auto_20180213_1541.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.6 on 2018-02-13 20:41 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('experiments', '0005_make_goal_optional'), + ] + + operations = [ + migrations.AddField( + model_name='experiment', + name='goal_url', + field=models.CharField(blank=True, default='', max_length=255), + ), + migrations.AlterField( + model_name='experiment', + name='status', + field=models.CharField(choices=[('draft', 'Draft'), ('live', 'Live'), ('completed', 'Completed')], db_index=True, default='draft', max_length=10), + ), + ] diff --git a/experiments/migrations/0007_auto_20181028_1516.py b/experiments/migrations/0007_auto_20181028_1516.py new file mode 100644 index 0000000..ed8026b --- /dev/null +++ b/experiments/migrations/0007_auto_20181028_1516.py @@ -0,0 +1,34 @@ +# Generated by Django 2.1.2 on 2018-10-28 20:16 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('experiments', '0006_auto_20180213_1541'), + ] + + operations = [ + migrations.AlterField( + model_name='alternative', + name='page', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='wagtailcore.Page'), + ), + migrations.AlterField( + model_name='experiment', + name='control_page', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='wagtailcore.Page'), + ), + migrations.AlterField( + model_name='experimenthistory', + name='experiment', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='history', to='experiments.Experiment'), + ), + migrations.AlterField( + model_name='experimenthistory', + name='variation', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='wagtailcore.Page'), + ), + ] diff --git a/experiments/models.py b/experiments/models.py index b93873a..6026879 100644 --- a/experiments/models.py +++ b/experiments/models.py @@ -9,8 +9,8 @@ from modelcluster.fields import ParentalKey from modelcluster.models import ClusterableModel -from wagtail.wagtailadmin.edit_handlers import FieldPanel, PageChooserPanel, InlinePanel -from wagtail.wagtailcore.models import Orderable +from wagtail.admin.edit_handlers import FieldPanel, PageChooserPanel, InlinePanel +from wagtail.core.models import Orderable BACKEND = None @@ -36,7 +36,8 @@ class Experiment(ClusterableModel): slug = models.SlugField(max_length=255) control_page = models.ForeignKey('wagtailcore.Page', related_name='+', on_delete=models.CASCADE) goal = models.ForeignKey('wagtailcore.Page', related_name='+', on_delete=models.SET_NULL, null=True, blank=True) - status = models.CharField(max_length=10, choices=STATUS_CHOICES, default='draft') + goal_url = models.CharField(max_length=255, blank=True, default='') + status = models.CharField(max_length=10, choices=STATUS_CHOICES, default='draft', db_index=True) winning_variation = models.ForeignKey('wagtailcore.Page', related_name='+', on_delete=models.SET_NULL, null=True) panels = [ @@ -45,6 +46,7 @@ class Experiment(ClusterableModel): PageChooserPanel('control_page'), InlinePanel('alternatives', label="Alternatives"), PageChooserPanel('goal'), + FieldPanel('goal_url'), FieldPanel('status'), ] diff --git a/experiments/views.py b/experiments/views.py index 839aafc..156772c 100644 --- a/experiments/views.py +++ b/experiments/views.py @@ -5,8 +5,8 @@ from django.shortcuts import get_object_or_404, redirect, render from django.utils.translation import ugettext as _ -from wagtail.wagtailadmin import messages -from wagtail.wagtailcore.models import Page +from wagtail.admin import messages +from wagtail.core.models import Page from .models import Experiment, get_backend from .utils import get_user_id, impersonate_other_page, percentage diff --git a/experiments/wagtail_hooks.py b/experiments/wagtail_hooks.py index fa0e4ca..1b8cbae 100644 --- a/experiments/wagtail_hooks.py +++ b/experiments/wagtail_hooks.py @@ -1,14 +1,14 @@ from __future__ import absolute_import, unicode_literals -from django.conf.urls import include, url +from django.urls import include, path from django.contrib.admin.utils import quote -from django.core.urlresolvers import reverse +from django.urls import reverse from django.utils.translation import ugettext_lazy as _ from experiments import admin_urls from wagtail.contrib.modeladmin.helpers import ButtonHelper from wagtail.contrib.modeladmin.options import ModelAdmin, modeladmin_register from wagtail.contrib.modeladmin.views import CreateView, EditView -from wagtail.wagtailcore import hooks +from wagtail.core import hooks from .models import Experiment from .utils import get_user_id, impersonate_other_page @@ -17,7 +17,7 @@ @hooks.register('register_admin_urls') def register_admin_urls(): return [ - url(r'^experiments/', include(admin_urls, app_name='experiments', namespace='experiments')), + path('experiments/', include(admin_urls, namespace='experiments')), ] diff --git a/setup.py b/setup.py index 1bf81e6..84e1622 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ setup( name='wagtail-experiments', - version='0.1.2', + version='0.1.3', description="A/B testing for Wagtail", author='Matthew Westcott', author_email='matthew.westcott@torchbox.com', @@ -20,11 +20,12 @@ 'License :: OSI Approved :: BSD License', 'Operating System :: OS Independent', 'Programming Language :: Python', - 'Programming Language :: Python :: 2', - 'Programming Language :: Python :: 2.7', 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.3', 'Programming Language :: Python :: 3.4', + 'Programming Language :: Python :: 3.5', + 'Programming Language :: Python :: 3.6', + 'Programming Language :: Python :: 3.7', 'Framework :: Django', ], ) diff --git a/tests/migrations/0001_initial.py b/tests/migrations/0001_initial.py index f49a265..136427d 100644 --- a/tests/migrations/0001_initial.py +++ b/tests/migrations/0001_initial.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from django.db import migrations, models +import django.db.models.deletion class Migration(migrations.Migration): @@ -14,7 +15,7 @@ class Migration(migrations.Migration): migrations.CreateModel( name='SimplePage', fields=[ - ('page_ptr', models.OneToOneField(primary_key=True, parent_link=True, to='wagtailcore.Page', auto_created=True, serialize=False)), + ('page_ptr', models.OneToOneField(primary_key=True, parent_link=True, to='wagtailcore.Page', auto_created=True, serialize=False, on_delete=django.db.models.deletion.SET_NULL)), ('body', models.TextField()), ], options={ diff --git a/tests/migrations/0003_auto_20181028_1516.py b/tests/migrations/0003_auto_20181028_1516.py new file mode 100644 index 0000000..6e4e652 --- /dev/null +++ b/tests/migrations/0003_auto_20181028_1516.py @@ -0,0 +1,19 @@ +# Generated by Django 2.1.2 on 2018-10-28 20:16 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('tests', '0002_simplepagerelatedlink'), + ] + + operations = [ + migrations.AlterField( + model_name='simplepage', + name='page_ptr', + field=models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page'), + ), + ] diff --git a/tests/models.py b/tests/models.py index e9bb515..bb2effe 100644 --- a/tests/models.py +++ b/tests/models.py @@ -3,7 +3,7 @@ from django.db import models from modelcluster.fields import ParentalKey -from wagtail.wagtailcore.models import Page, Orderable +from wagtail.core.models import Page, Orderable class SimplePage(Page): diff --git a/tests/settings.py b/tests/settings.py index 13834d0..d387328 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -54,11 +54,11 @@ 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', - 'wagtail.wagtailcore.middleware.SiteMiddleware', + 'wagtail.core.middleware.SiteMiddleware', + 'experiments.middleware.GoalURLMiddleware', ) else: MIDDLEWARE_CLASSES = ( @@ -66,11 +66,10 @@ 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', - 'wagtail.wagtailcore.middleware.SiteMiddleware', + 'wagtail.core.middleware.SiteMiddleware', ) INSTALLED_APPS = ( @@ -78,13 +77,13 @@ 'tests', 'wagtail.contrib.modeladmin', - 'wagtail.wagtailsearch', - 'wagtail.wagtailsites', - 'wagtail.wagtailusers', - 'wagtail.wagtailimages', - 'wagtail.wagtaildocs', - 'wagtail.wagtailadmin', - 'wagtail.wagtailcore', + 'wagtail.search', + 'wagtail.sites', + 'wagtail.users', + 'wagtail.images', + 'wagtail.documents', + 'wagtail.admin', + 'wagtail.core', 'taggit', diff --git a/tests/tests.py b/tests/tests.py index 74ff87f..6d374a5 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1,9 +1,9 @@ from __future__ import absolute_import, unicode_literals from django.contrib.auth.models import User -from django.core.urlresolvers import reverse +from django.urls import reverse from django.test import TestCase -from wagtail.wagtailcore.models import Page +from wagtail.core.models import Page from experiments.models import Experiment, ExperimentHistory @@ -176,6 +176,43 @@ def test_completion_through_direct_url(self): self.assertEqual(history_record.participant_count, 1) self.assertEqual(history_record.completion_count, 1) + def test_completion_is_logged_for_url_outside_wagtail(self): + # Remove the goal page and add a URL that doesn't map to a page instead + success_url = '/success-url-1/' + self.experiment.goal = None + self.experiment.goal_url = success_url # no Page maps to this url + self.experiment.save() + self.assertEqual(Page.objects.filter(slug=success_url).count(), 0) + # User 11111111-1111-1111-1111-111111111111 + session = self.client.session + session['experiment_user_id'] = '11111111-1111-1111-1111-111111111111' + session.save() + self.client.get('/') + + # history record should show 1 participant, 0 completions + history_record = ExperimentHistory.objects.get( + experiment=self.experiment, variation=self.homepage + ) + self.assertEqual(history_record.participant_count, 1) + self.assertEqual(history_record.completion_count, 0) + + self.client.get(success_url) + + # history record should show 1 participant, 1 completion + history_record = ExperimentHistory.objects.get( + experiment=self.experiment, variation=self.homepage + ) + self.assertEqual(history_record.participant_count, 1) + self.assertEqual(history_record.completion_count, 1) + + # repeated completions from the same user should not update the count + self.client.get(success_url) + history_record = ExperimentHistory.objects.get( + experiment=self.experiment, variation=self.homepage + ) + self.assertEqual(history_record.participant_count, 1) + self.assertEqual(history_record.completion_count, 1) + def test_completion_through_direct_url_is_not_counted_if_experiment_not_started(self): session = self.client.session session['experiment_user_id'] = '11111111-1111-1111-1111-111111111111' diff --git a/tests/urls.py b/tests/urls.py index 1bf3f06..3eec2c7 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -1,19 +1,19 @@ from __future__ import absolute_import, unicode_literals -from django.conf.urls import include, url +from django.urls import include, path, re_path -from wagtail.wagtailadmin import urls as wagtailadmin_urls -from wagtail.wagtailcore import urls as wagtail_urls +from wagtail.admin import urls as wagtailadmin_urls +from wagtail.core import urls as wagtail_urls from experiments import views as experiment_views urlpatterns = [ - url(r'^admin/', include(wagtailadmin_urls)), + path('admin/', include(wagtailadmin_urls)), - url(r'^experiments/complete/([^\/]+)/$', experiment_views.record_completion), + re_path(r'^experiments/complete/([^\/]+)/$', experiment_views.record_completion), # For anything not caught by a more specific rule above, hand over to # Wagtail's serving mechanism - url(r'', include(wagtail_urls)), + path('', include(wagtail_urls)), ] diff --git a/tox.ini b/tox.ini index 8a0ffad..7a4e053 100644 --- a/tox.ini +++ b/tox.ini @@ -1,17 +1,13 @@ [tox] -envlist = py{27,34,35}-dj{18,19,110}-wt{17} +envlist = py{36}-dj{21}-wt{17} [testenv] basepython = - py27: python2.7 - py34: python3.4 - py35: python3.5 + py36: python3.6 deps = - dj18: Django>=1.8,<1.9 - dj19: Django>=1.9,<1.10 - dj110: Django>=1.10,<1.11 + dj110: Django>=2.1 - wt17: wagtail>=1.7,<1.8 + wt17: wagtail==2.3 commands = ./runtests.py diff --git a/wagtail_experiments b/wagtail_experiments new file mode 100644 index 0000000..e69de29