diff --git a/.github/report_nightly_build_failure.py b/.github/report_nightly_build_failure.py new file mode 100644 index 0000000..7ce29ec --- /dev/null +++ b/.github/report_nightly_build_failure.py @@ -0,0 +1,28 @@ + +""" +Called by GitHub Action when the nightly build fails. +This reports an error to the #nightly-build-failures Slack channel. +""" +import os +import requests + +if "SLACK_WEBHOOK_URL" in os.environ: + # https://docs.github.com/en/free-pro-team@latest/actions/reference/environment-variables#default-environment-variables + repository = os.environ["GITHUB_REPOSITORY"] + run_id = os.environ["GITHUB_RUN_ID"] + url = f"https://github.com/{repository}/actions/runs/{run_id}" + + print("Reporting to #nightly-build-failures slack channel") + response = requests.post( + os.environ["SLACK_WEBHOOK_URL"], + json={ + "text": f"A Nightly build failed. See {url}", + }, + ) + + print(f"Slack responded with: {response}") + +else: + print( + "Unable to report to #nightly-build-failures slack channel because SLACK_WEBHOOK_URL is not set" + ) diff --git a/.github/workflows/nightly-tests.yml b/.github/workflows/nightly-tests.yml new file mode 100644 index 0000000..0dcaaca --- /dev/null +++ b/.github/workflows/nightly-tests.yml @@ -0,0 +1,35 @@ +name: Nightly Wagtail test + +on: + schedule: + - cron: "20 1 * * *" + + workflow_dispatch: + +jobs: + nightly-test: + # Cannot check the existence of secrets, so limiting to repository name to prevent all forks to run nightly. + # See: https://github.com/actions/runner/issues/520 + if: ${{ github.repository == 'torchbox/wagtail-experiments' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install "git+https://github.com/wagtail/wagtail.git@main#egg=wagtail" + pip install -e .[testing] + - name: Test + id: test + continue-on-error: true + run: ./runtests.py + - name: Send Slack notification on failure + if: steps.test.outcome == 'failure' + run: | + python .github/report_nightly_build_failure.py + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..5d153f1 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,77 @@ + +name: CI + +on: + push: + branches: [ main ] + pull_request: + +# Current configuration: +# - django 3.2, python 3.8, wagtail 4.1, postgres +# - django 4.0, python 3.9, wagtail 4.2, sqlite +# - django 4.1, python 3.10, wagtail 5.0, postgres +# - django 4.2, python 3.11, wagtail 5.1, sqlite +# - django 3.2, python 3.10, wagtail main, sqlite (allow failures) +jobs: + test: + runs-on: ubuntu-latest + continue-on-error: ${{ matrix.experimental }} + strategy: + matrix: + include: + - python: "3.8" + django: "Django>=3.2,<4.0" + wagtail: "wagtail>=4.1,<4.2" + database: "postgresql" + experimental: false + - python: "3.9" + django: "Django>=4.0,<4.1" + wagtail: "wagtail>=4.2,<5.0" + database: "sqlite3" + experimental: false + - python: "3.10" + django: "Django>=4.1,<4.2" + wagtail: "wagtail>=5.0,<5.1" + database: "postgresql" + experimental: false + - python: "3.11" + django: "Django>=4.2,<4.3" + wagtail: "wagtail>=5.1,<5.2" + database: "sqlite3" + experimental: false + + - python: "3.11" + django: "Django>=4.1,<4.2" + wagtail: "git+https://github.com/wagtail/wagtail.git@main#egg=wagtail" + database: "sqlite3" + experimental: true + + services: + postgres: + image: postgres:14 + env: + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 + + steps: + - uses: actions/checkout@v3 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install "psycopg2>=2.9.3" + pip install "${{ matrix.django }}" + pip install "${{ matrix.wagtail }}" + pip install -e .[testing] + - name: Test + run: ./runtests.py + env: + DATABASE_ENGINE: django.db.backends.${{ matrix.database }} + DATABASE_HOST: localhost + DATABASE_USER: postgres + DATABASE_PASS: postgres diff --git a/.gitignore b/.gitignore index bbded1f..45f141f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.pyc /.tox/ /dist/ +/build/ /wagtail_experiments.egg-info/ diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 66c4ef0..0000000 --- a/.travis.yml +++ /dev/null @@ -1,39 +0,0 @@ -language: python -cache: pip - -matrix: - include: - - env: TOXENV=py27-dj18-wt17 - python: 2.7 - - env: TOXENV=py27-dj18-wt18 - python: 2.7 - - env: TOXENV=py27-dj19-wt19 - python: 2.7 - - env: TOXENV=py27-dj110-wt110 - python: 2.7 - - env: TOXENV=py27-dj110-wt111 - python: 2.7 - - env: TOXENV=py27-dj111-wt112 - python: 2.7 - - env: TOXENV=py27-dj111-wt113 - python: 2.7 - - env: TOXENV=py34-dj111-wt113 - python: 3.4 - - env: TOXENV=py35-dj111-wt21 - python: 3.5 - - env: TOXENV=py35-dj20-wt20 - python: 3.5 - - env: TOXENV=py36-dj20-wt21 - python: 3.6 - - env: TOXENV=py36-dj20-wt22 - python: 3.6 - - env: TOXENV=py36-dj21-wt23 - python: 3.6 - -# Package installation -install: - - pip install tox - -# Run the tests -script: - tox diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 8ea3c67..883e3c7 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,14 @@ Changelog ========= +0.3 (2023-08-10) +~~~~~~~~~~~~~~~~ + + * Added support for Wagtail 4.1 thru 5.0 + * Added support for Django 3.2 thru 4.2 + * Added docstrings to all 'experiment' functions and classes. + * Support internationalization for models + 0.2 (28.11.2018) ~~~~~~~~~~~~~~~~ diff --git a/README.rst b/README.rst index c889f65..c28316e 100644 --- a/README.rst +++ b/README.rst @@ -3,9 +3,6 @@ Wagtail Experiments =================== -.. image:: https://api.travis-ci.org/torchbox/wagtail-experiments.svg?branch=master - :target: https://travis-ci.org/torchbox/wagtail-experiments - **A/B testing for Wagtail** This module supports the creation of A/B testing experiments within a Wagtail site. Several alternative versions of a page are set up, and on visiting a designated control page, a user is presented with one of those variations, selected at random (using a simplified version of the `PlanOut algorithm `_). The number of visitors receiving each variation is logged, along with the number that subsequently go on to complete the experiment by visiting a designated goal page. @@ -14,7 +11,7 @@ This module supports the creation of A/B testing experiments within a Wagtail si Installation ------------ -wagtail-experiments is compatible with Wagtail 1.7 to 2.3, and Django 1.8 to 2.1. To install:: +wagtail-experiments is compatible with Wagtail 4.1 to 5.1, and Django 3.2 to 4.2. To install:: pip install wagtail-experiments @@ -29,7 +26,7 @@ and ensure that the apps ``wagtail.contrib.modeladmin`` and ``experiments`` are # ... ] -Then migrate:: +Then migrate:: ./manage.py migrate diff --git a/experiments/admin_urls.py b/experiments/admin_urls.py index 98553b6..93e4019 100644 --- a/experiments/admin_urls.py +++ b/experiments/admin_urls.py @@ -1,12 +1,10 @@ -from __future__ import absolute_import, unicode_literals - -from django.urls import path +from django.urls import re_path from experiments import views app_name = 'experiments' urlpatterns = [ - 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'), + re_path(r'^experiment/report/(\d+)/$', views.experiment_report, name='report'), + re_path(r'^experiment/select_winner/(\d+)/(\d+)/$', views.select_winner, name='select_winner'), + re_path(r'^experiment/report/preview/(\d+)/(\d+)/$', views.preview_for_report, name='preview_for_report'), ] diff --git a/experiments/backends/db.py b/experiments/backends/db.py index 7493c87..d155b05 100644 --- a/experiments/backends/db.py +++ b/experiments/backends/db.py @@ -1,13 +1,24 @@ -from __future__ import absolute_import, unicode_literals - import datetime - from django.db.models import F, Sum from experiments.models import ExperimentHistory #TODO: potentially the same session? clearing cookies def record_participant(experiment, user_id, variation, request): + ''' + If the user hasn't already participated in this experiment, + then save the variation the user viewed. + + Args: + experiment: instance of experiments.models.Experiment + user_id: the id for this user. + variation: variation user viewed. + request: django HttpRequest. + + Return: + Nothing + ''' + # abort if this user has participated already experiments_started = request.session.get('experiments_started', []) if experiment.id in experiments_started: @@ -23,8 +34,21 @@ def record_participant(experiment, user_id, variation, request): # increment the participant_count ExperimentHistory.objects.filter(pk=history.pk).update(participant_count=F('participant_count') + 1) - def record_completion(experiment, user_id, variation, request): + ''' + If the user has started this experiment, but not completed it yet, + then mark the user's participation as completed. + + Args: + experiment: instance of experiments.models.Experiment + user_id: the id for this user. + variation: variation user viewed. + request: django HttpRequest. + + Return: + Nothing + ''' + # abort if this user never started the experiment if experiment.id not in request.session.get('experiments_started', []): return @@ -46,6 +70,16 @@ def record_completion(experiment, user_id, variation, request): def get_report(experiment): + ''' + Generate a report about the experiment's results. + + Args: + experiment: instance of experiments.models.Experiment + + Return: + A report of experiment results as a dictionary. + ''' + result = {} result.setdefault('variations', []) diff --git a/experiments/models.py b/experiments/models.py index a5ef01b..9023cd5 100644 --- a/experiments/models.py +++ b/experiments/models.py @@ -1,26 +1,31 @@ -from __future__ import absolute_import, unicode_literals - from hashlib import sha1 from importlib import import_module - from django.conf import settings from django.db import models +from django.utils.translation import gettext_lazy as _ + +from wagtail.admin.panels import FieldPanel, PageChooserPanel, InlinePanel +from wagtail.models import Orderable, Page from modelcluster.fields import ParentalKey from modelcluster.models import ClusterableModel -try: - from wagtail.admin.edit_handlers import FieldPanel, PageChooserPanel, InlinePanel - from wagtail.core.models import Orderable -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailadmin.edit_handlers import FieldPanel, PageChooserPanel, InlinePanel - from wagtail.wagtailcore.models import Orderable - BACKEND = None def get_backend(): + ''' + Get the backend to use for wagtail-experiments. + + Args: + None + + Return: + Backend module. + If WAGTAIL_EXPERIMENTS_BACKEND not defined, defaults to experiments.backends.db. + ''' + global BACKEND if BACKEND is None: backend_name = getattr(settings, 'WAGTAIL_EXPERIMENTS_BACKEND', 'experiments.backends.db') @@ -30,10 +35,14 @@ def get_backend(): class Experiment(ClusterableModel): + ''' + Define an experiment for a page. + ''' + STATUS_CHOICES = [ - ('draft', "Draft"), - ('live', "Live"), - ('completed', "Completed"), + ('draft', _("Draft")), + ('live', _("Live")), + ('completed', _("Completed")), ] name = models.CharField(max_length=255) slug = models.SlugField(max_length=255) @@ -47,53 +56,128 @@ class Experiment(ClusterableModel): FieldPanel('name'), FieldPanel('slug'), PageChooserPanel('control_page'), - InlinePanel('alternatives', label="Alternatives"), + InlinePanel('alternatives', heading=_("Alternatives"), label=_("Alternative")), PageChooserPanel('goal'), FieldPanel('goal_url'), FieldPanel('status'), ] + + class Meta: + verbose_name = _('experiment') + verbose_name_plural = _('experiments') + + def __init__(self, *args, **kwargs): super(Experiment, self).__init__(*args, **kwargs) self._initial_status = self.status def activate_alternative_draft_content(self): - # For any alternative pages that are unpublished, copy the latest draft revision - # to the main table (with is_live=False) so that the revision shown as an alternative - # is not an out-of-date one + ''' + For any alternative pages that are unpublished, copy the latest + draft revision to the main table (with is_live=False) so that the + revision shown as an alternative is not an out-of-date one. + + Args: + self: instance of the class Experiment + + Return: + Nothing + ''' + for alternative in self.alternatives.select_related('page'): if not alternative.page.live: - revision = alternative.page.get_latest_revision_as_page() + revision = alternative.page.get_latest_revision_as_object() revision.live = False revision.has_unpublished_changes = True revision.save() def get_variations(self): - return [self.control_page] + [alt.page for alt in self.alternatives.select_related('page')] + ''' + Get all the variations for the control page. + + Return: + variations: a list with the control page and all alternatives + ''' + + variations = [self.control_page] + for alternative in self.alternatives.select_related('page'): + variations.append(alternative.page) + + return variations def get_variation_for_user(self, user_id): + ''' + Get a page variation for this user and request session. + + Args: + user_id: the id for this user + + Return: + variation: a page variation + ''' + variations = self.get_variations() # choose uniformly from variations, based on a hash of user_id and experiment.slug hash_input = "{0}.{1}".format(self.slug, user_id) hash_str = sha1(hash_input.encode('utf-8')).hexdigest()[:7] variation_index = int(hash_str, 16) % len(variations) + return variations[variation_index] def start_experiment_for_user(self, user_id, request): - """ - Record a new participant and return the variation for them to use - """ + ''' + Record a new participant and return the variation for them to use. + + Args: + user_id: the id for this user + request: django HttpRequest + + Return: + Variation the user will see + ''' + variation = self.get_variation_for_user(user_id) get_backend().record_participant(self, user_id, variation, request) return variation def record_completion_for_user(self, user_id, request): + ''' + Record the completion of the variation for the user. + + Args: + user_id: the id for this user + request: django HttpRequest + + Return: + Nothing + + Is the following appropriate? Or should we only consider it a win if + the user sees an experiment page and immediately goes to the goal? + If: + 1. The user goes to a experiment page + 2. Then wanders the site for hours + 3. Finally hits a goal page + Then: + We record a win for the experiment. It isn't. + ''' + backend = get_backend() variation = self.get_variation_for_user(user_id) backend.record_completion(self, user_id, variation, request) def select_winner(self, variation): + ''' + Save the variation that won. + + Args: + variation: winning variation + + Return: + Nothing + ''' + self.winning_variation = variation self.status = 'completed' self.save() @@ -103,6 +187,10 @@ def __str__(self): class Alternative(Orderable): + ''' + Alternative page for the control page. + ''' + experiment = ParentalKey(Experiment, related_name='alternatives', on_delete=models.CASCADE) page = models.ForeignKey('wagtailcore.Page', related_name='+', on_delete=models.CASCADE) @@ -110,11 +198,17 @@ class Alternative(Orderable): PageChooserPanel('page'), ] + class Meta: + verbose_name = _('alternative') + verbose_name_plural = _('alternatives') + class ExperimentHistory(models.Model): - """ - Records the number of participants and completions on a given day for a given variation of an experiment - """ + ''' + Maintains the number of participants and completions + on a given day for a given variation of an experiment. + ''' + experiment = models.ForeignKey(Experiment, related_name='history', on_delete=models.CASCADE) date = models.DateField() variation = models.ForeignKey('wagtailcore.Page', related_name='+', on_delete=models.CASCADE) @@ -125,3 +219,6 @@ class Meta: unique_together = [ ('experiment', 'date', 'variation'), ] + + verbose_name = _('experiment history') + verbose_name_plural = _('experiment histories') diff --git a/experiments/templates/experiments/report.html b/experiments/templates/experiments/report.html index afc2c6c..caa49ec 100644 --- a/experiments/templates/experiments/report.html +++ b/experiments/templates/experiments/report.html @@ -1,5 +1,5 @@ {% extends "wagtailadmin/base.html" %} -{% load i18n staticfiles %} +{% load i18n static %} {% block titletag %}{% trans "Experiment results" %}{% endblock %} @@ -76,7 +76,7 @@

{% trans "Conversion rate / Day" %}

['{{ variation.title|escapejs }}', {% for history_entry in variation_report.history %} - '{{ history_entry.conversion_rate|floatformat:2|escapejs }}'{% if not forloop.last %},{% endif %} + '{{ history_entry.conversion_rate|stringformat:".2f"|escapejs }}'{% if not forloop.last %},{% endif %} {% endfor %} ]{% if not forloop.last %},{% endif %} {% endfor %} diff --git a/experiments/utils.py b/experiments/utils.py index faa4b73..7e5d6ca 100644 --- a/experiments/utils.py +++ b/experiments/utils.py @@ -1,13 +1,38 @@ -from __future__ import absolute_import, unicode_literals - import uuid +from .models import Alternative + def get_user_id(request): + ''' + Get user id for this request. A user ID is assigned randomly + for each request session. + + Args: + request: django HttpRequest. + + Return: + User ID for the request as a str. + + To do: + Require unique user ID for each session. + ''' + return request.session.setdefault('experiment_user_id', str(uuid.uuid4())) def percentage(fraction, population): + ''' + Calc percentage. + + Args: + fraction: Portion of population. + population: Population count. + + Return: + Percentage as float. + ''' + try: return float(fraction) / float(population) * 100 except (ValueError, ZeroDivisionError, TypeError): @@ -15,7 +40,17 @@ def percentage(fraction, population): def impersonate_other_page(page, other): - """Modify the title and tree location data of `page` to resemble `other`""" + ''' + Modify the tree location data of `page` to resemble `other`. + + Args: + page: the page to modify + other: the page to impersonate + + Return: + None + ''' + page.path = other.path page.depth = other.depth page.url_path = other.url_path diff --git a/experiments/views.py b/experiments/views.py index 7b71445..49ca51b 100644 --- a/experiments/views.py +++ b/experiments/views.py @@ -1,22 +1,26 @@ -from __future__ import absolute_import, unicode_literals - from django.core.exceptions import PermissionDenied from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.utils.translation import gettext as _ -try: - from wagtail.admin import messages - from wagtail.core.models import Page -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailadmin import messages - from wagtail.wagtailcore.models import Page +from wagtail.admin import messages +from wagtail.models import Page from .models import Experiment, get_backend from .utils import get_user_id, impersonate_other_page, percentage def record_completion(request, slug): + ''' + Record the completion of the experiment for the user. + + Args: + request: django HttpRequest. + slug: the experiment's slug. + + Return: + OK as the HttpResponse + ''' experiment = get_object_or_404(Experiment, slug=slug) user_id = get_user_id(request) @@ -25,7 +29,17 @@ def record_completion(request, slug): def experiment_report(request, experiment_id): - # TODO: Decide if we need a custom permission to access reports + ''' + Generate a report for the experiment. + + Args: + request: django HttpRequest. + experiment_id: the primary key for the experiment. + + Return: + The experiment's report in rendered HTML using + the experiments/report.html for formatting. + ''' backend = get_backend() experiment = get_object_or_404(Experiment, pk=experiment_id) @@ -59,6 +73,21 @@ def experiment_report(request, experiment_id): def select_winner(request, experiment_id, variation_id): + ''' + Record the winner for the experiment if user has permission + to change the experiment. + + Args: + request: django HttpRequest. + experiment_id: the primary key for the experiment. + variation_id: the primary key for the variation. + + Return: + If request is a POST and the user has permission to change experiment, + then redirect to generate a report. + + If request is not a POST, then return nothing. + ''' if not request.user.has_perm('experiments.change_experiment'): raise PermissionDenied experiment = get_object_or_404(Experiment, pk=experiment_id) @@ -74,14 +103,24 @@ def select_winner(request, experiment_id, variation_id): return redirect('experiments:report', experiment.pk) - def preview_for_report(request, experiment_id, page_id): + ''' + Preview a report if the user has permission to publish. + + Args: + request: django HttpRequest. + experiment_id: the primary key for the experiment. + page_id: the primary key for the page. + + Return: + The rendered page by wagtail. + ''' experiment = get_object_or_404(Experiment, pk=experiment_id) page = get_object_or_404(Page, id=page_id).specific if not page.permissions_for_user(request.user).can_publish(): raise PermissionDenied - # hack the title and page-tree-related fields to match the control page + # hack the page-tree-related fields to match the control page impersonate_other_page(page, experiment.control_page) # pass in the real user request rather than page.dummy_request(), so that request.user diff --git a/experiments/wagtail_hooks.py b/experiments/wagtail_hooks.py index 380d908..16793d4 100644 --- a/experiments/wagtail_hooks.py +++ b/experiments/wagtail_hooks.py @@ -1,24 +1,13 @@ -from __future__ import absolute_import, unicode_literals - -from django.conf.urls import include -from django.urls import path +from django.urls import include, re_path from django.contrib.admin.utils import quote - -try: - from django.urls import reverse -except ImportError: # fallback for Django <=1.9 - from django.core.urlresolvers import reverse +from django.urls import reverse from django.utils.translation import gettext_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 - -try: - from wagtail.core import hooks -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailcore import hooks +from wagtail import hooks from .models import Experiment from .utils import get_user_id, impersonate_other_page @@ -27,7 +16,7 @@ @hooks.register('register_admin_urls') def register_admin_urls(): return [ - path('experiments/', include(admin_urls, namespace='experiments')), + re_path(r'^experiments/', include(admin_urls, namespace='experiments')), ] @@ -57,6 +46,9 @@ def get_buttons_for_obj(self, obj, exclude=[], classnames_add=[], class CreateExperimentView(CreateView): def form_valid(self, form): + # Called when the creation form is submitted and valid. + # If the form's status is "live", then the alternative + # draft content is activated response = super(CreateExperimentView, self).form_valid(form) if form.instance.status == 'live': form.instance.activate_alternative_draft_content() @@ -65,6 +57,9 @@ def form_valid(self, form): class EditExperimentView(EditView): def form_valid(self, form): + # Called when the edit form is submitted and valid. + # If the form's status is changing from "draft" to "live", then + # the alternative draft content is activated response = super(EditExperimentView, self).form_valid(form) if self.instance._initial_status == 'draft' and self.instance.status == 'live': self.instance.activate_alternative_draft_content() @@ -73,6 +68,9 @@ def form_valid(self, form): class ExperimentModelAdmin(ModelAdmin): + ''' + Define the admin interface for experiments via the ModelAdmin app. + ''' model = Experiment add_to_settings_menu = True button_helper_class = ExperimentButtonHelper @@ -84,6 +82,23 @@ class ExperimentModelAdmin(ModelAdmin): @hooks.register('before_serve_page') def check_experiments(page, request, serve_args, serve_kwargs): + ''' + Check whether the page is a control or goal of an experiment. + If the page is a control page, run the experiment. + If the page is a goal page, log a completion. + + Args: + page: page being served. + request: django HttpRequest. + serve_args: non-keyword arguments for page being served. + serve_kwargs: keyword arguments for the page being served + + Return: + If the page is a control page in a live, yet uncompleted, experiment + and the variation page for this user is not the same as the page, + then show the variation page. Otherwise, return nothing. + ''' + # If the page being served is the goal page of an experiment, log a completion completed_experiments = Experiment.objects.filter(goal=page, status='live') @@ -110,7 +125,7 @@ def check_experiments(page, request, serve_args, serve_kwargs): variation = variation.specific - # hack the title and page-tree-related fields to match the control page + # hack the page-tree-related fields to match the control page impersonate_other_page(variation, page) return variation.serve(request, *serve_args, **serve_kwargs) diff --git a/setup.py b/setup.py index 216ac9e..1b37bb7 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ setup( name='wagtail-experiments', - version='0.2.1', + version='0.3', description="A/B testing for Wagtail", author='Matthew Westcott', author_email='matthew.westcott@torchbox.com', @@ -13,6 +13,7 @@ include_package_data=True, license='BSD', long_description=open('README.rst').read(), + python_requires=">=3.8", classifiers=[ 'Development Status :: 4 - Beta', 'Environment :: Web Environment', @@ -20,17 +21,17 @@ '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', + 'Programming Language :: Python :: 3.8', + 'Programming Language :: Python :: 3.9', + 'Programming Language :: Python :: 3.10', 'Framework :: Django', + 'Framework :: Django :: 3.2', + 'Framework :: Django :: 4.0', + 'Framework :: Django :: 4.1', + 'Framework :: Django :: 4.2', 'Framework :: Wagtail', - 'Framework :: Wagtail :: 1', - 'Framework :: Wagtail :: 2', + 'Framework :: Wagtail :: 4', + 'Framework :: Wagtail :: 5', ], ) + diff --git a/tests/models.py b/tests/models.py index 2a7345c..694ea42 100644 --- a/tests/models.py +++ b/tests/models.py @@ -1,13 +1,6 @@ -from __future__ import absolute_import, unicode_literals - from django.db import models - from modelcluster.fields import ParentalKey - -try: - from wagtail.core.models import Page, Orderable -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailcore.models import Page, Orderable +from wagtail.models import Orderable, Page, Site class SimplePage(Page): @@ -15,7 +8,8 @@ class SimplePage(Page): def get_context(self, request): context = super(SimplePage, self).get_context(request) - context['breadcrumb'] = self.get_ancestors(inclusive=True).filter(depth__gte=request.site.root_page.depth) + site = Site.find_for_request(request) + context['breadcrumb'] = self.get_ancestors(inclusive=True).filter(depth__gte=site.root_page.depth) return context diff --git a/tests/settings.py b/tests/settings.py index b760128..7c2d516 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,5 +1,3 @@ -from __future__ import absolute_import, unicode_literals - import os import django @@ -51,83 +49,46 @@ }, ] -if django.VERSION >= (1, 10): - MIDDLEWARE = [ - 'django.middleware.common.CommonMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.middleware.csrf.CsrfViewMiddleware', - 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.messages.middleware.MessageMiddleware', - 'django.middleware.clickjacking.XFrameOptionsMiddleware', - ] +MIDDLEWARE = [ + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.middleware.common.CommonMiddleware', + 'django.middleware.csrf.CsrfViewMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'django.contrib.messages.middleware.MessageMiddleware', + 'django.contrib.sites.middleware.CurrentSiteMiddleware', + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.middleware.clickjacking.XFrameOptionsMiddleware', + 'wagtail.contrib.redirects.middleware.RedirectMiddleware', +] - if WAGTAIL_VERSION < (2, 0): - MIDDLEWARE.append('wagtail.wagtailcore.middleware.SiteMiddleware') - else: - MIDDLEWARE.append('wagtail.core.middleware.SiteMiddleware') - -else: - MIDDLEWARE_CLASSES = ( - 'django.middleware.common.CommonMiddleware', - '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', - ) - -if WAGTAIL_VERSION < (2, 0): - INSTALLED_APPS = ( - 'experiments', - 'tests', - - 'wagtail.contrib.modeladmin', - 'wagtail.wagtailsearch', - 'wagtail.wagtailsites', - 'wagtail.wagtailusers', - 'wagtail.wagtailimages', - 'wagtail.wagtaildocs', - 'wagtail.wagtailadmin', - 'wagtail.wagtailcore', - - 'taggit', - - 'django.contrib.admin', - 'django.contrib.auth', - 'django.contrib.contenttypes', - 'django.contrib.sessions', - 'django.contrib.messages', - 'django.contrib.staticfiles', - ) -else: - INSTALLED_APPS = ( - 'experiments', - 'tests', - - 'wagtail.contrib.modeladmin', - 'wagtail.search', - 'wagtail.sites', - 'wagtail.users', - 'wagtail.images', - 'wagtail.documents', - 'wagtail.admin', - 'wagtail.core', - - 'taggit', - - 'django.contrib.admin', - 'django.contrib.auth', - 'django.contrib.contenttypes', - 'django.contrib.sessions', - 'django.contrib.messages', - 'django.contrib.staticfiles', - ) +INSTALLED_APPS = [ + 'experiments', + 'tests', + + 'wagtail.contrib.modeladmin', + 'wagtail.search', + 'wagtail.sites', + 'wagtail.users', + 'wagtail.images', + 'wagtail.documents', + 'wagtail.admin', + 'wagtail', + + 'taggit', + + 'django.contrib.admin', + 'django.contrib.auth', + 'django.contrib.contenttypes', + 'django.contrib.sessions', + 'django.contrib.messages', + 'django.contrib.staticfiles', + ] PASSWORD_HASHERS = ( 'django.contrib.auth.hashers.MD5PasswordHasher', # don't use the intentionally slow default password hasher ) +DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' + WAGTAIL_SITE_NAME = 'wagtail-experiments test' +WAGTAILADMIN_BASE_URL = 'http://127.0.0.1:8000' diff --git a/tests/tests.py b/tests/tests.py index fc1c7fc..83cb877 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1,23 +1,15 @@ -from __future__ import absolute_import, unicode_literals +from django import __version__ as DJANGO_VERSION from django.contrib.auth.models import User - -try: - from django.urls import reverse -except ImportError: # fallback for Django <=1.9 - from django.core.urlresolvers import reverse - from django.test import TestCase - -try: - from wagtail.core.models import Page -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailcore.models import Page +from django.urls import reverse +from wagtail.models import Page from experiments.models import Experiment, ExperimentHistory class TestFrontEndView(TestCase): + fixtures = ['test.json'] def setUp(self): @@ -59,7 +51,7 @@ def test_selected_variation_depends_on_user_id(self): for x in range(0, 5): response = self.client.get('/') self.assertEqual(response.status_code, 200) - self.assertContains(response, '

Welcome to our site! It's lovely to meet you.

') + self.assertContains(response, "

Welcome to our site! It's lovely to meet you.

") self.assertContains(response, 'a lovely link') def test_participant_is_logged(self): @@ -289,7 +281,8 @@ def test_completed_status(self): response = self.client.get('/') self.assertEqual(response.status_code, 200) - self.assertContains(response, "

Oh, it's you. What do you want?

") + self.assertContains(response, "Home") + self.assertContains(response, "What do you want?") class TestAdmin(TestCase): @@ -305,6 +298,8 @@ def setUp(self): self.homepage_alternative_1 = Page.objects.get(url_path='/home/home-alternative-1/').specific self.homepage_alternative_2 = Page.objects.get(url_path='/home/home-alternative-2/').specific + self.admin_home = reverse('wagtailadmin_home').strip('/') + def get_edit_postdata(self, **kwargs): alternatives = self.experiment.alternatives.all() @@ -336,18 +331,21 @@ def get_edit_postdata(self, **kwargs): def test_experiments_menu_item(self): response = self.client.get(reverse('wagtailadmin_home')) self.assertEqual(response.status_code, 200) - self.assertContains(response, 'href="/admin/experiments/experiment/"') + try: + self.assertEqual(response.context_data['page_title'], 'Dashboard') + except AssertionError: # fallback for Django <2.0 + self.assertContains(response, f'href="/{self.admin_home}/experiments/experiment/"') def test_experiments_index(self): - response = self.client.get('/admin/experiments/experiment/') + response = self.client.get(f'/{self.admin_home}/experiments/experiment/') self.assertEqual(response.status_code, 200) self.assertContains(response, 'Homepage text') def test_experiment_new(self): - response = self.client.get('/admin/experiments/experiment/create/') + response = self.client.get(f'/{self.admin_home}/experiments/experiment/create/') self.assertEqual(response.status_code, 200) - response = self.client.post('/admin/experiments/experiment/create/', { + response = self.client.post(f'/{self.admin_home}/experiments/experiment/create/', { 'name': "Another experiment", 'slug': 'another-experiment', 'control_page': self.homepage_alternative_1.pk, @@ -362,18 +360,18 @@ def test_experiment_new(self): 'goal': self.homepage.pk, 'status': 'draft', }) - self.assertRedirects(response, '/admin/experiments/experiment/') + self.assertRedirects(response, f'/{self.admin_home}/experiments/experiment/') self.assertTrue(Experiment.objects.filter(slug='another-experiment').exists()) def test_experiment_edit(self): - response = self.client.get('/admin/experiments/experiment/edit/%d/' % self.experiment.pk) + response = self.client.get(f'/{self.admin_home}/experiments/experiment/edit/%d/' % self.experiment.pk) self.assertEqual(response.status_code, 200) response = self.client.post( - '/admin/experiments/experiment/edit/%d/' % self.experiment.pk, + f'/{self.admin_home}/experiments/experiment/edit/{self.experiment.pk}/', self.get_edit_postdata(name="Homepage text updated") ) - self.assertRedirects(response, '/admin/experiments/experiment/') + self.assertRedirects(response, f'/{self.admin_home}/experiments/experiment/') experiment = Experiment.objects.get(pk=self.experiment.pk) self.assertEqual(experiment.name, "Homepage text updated") @@ -388,7 +386,7 @@ def test_draft_page_content_is_activated_when_experiment_goes_live(self): # submit an edit to the experiment, but preserve its live status self.client.post( - '/admin/experiments/experiment/edit/%d/' % self.experiment.pk, + f'/{self.admin_home}/experiments/experiment/edit/{self.experiment.pk}/', self.get_edit_postdata() ) # editing an already-live experiment should not update the page content @@ -397,7 +395,7 @@ def test_draft_page_content_is_activated_when_experiment_goes_live(self): # make the experiment draft self.client.post( - '/admin/experiments/experiment/edit/%d/' % self.experiment.pk, + f'/{self.admin_home}/experiments/experiment/edit/{self.experiment.pk}/', self.get_edit_postdata(status='draft') ) # page content should still be unchanged @@ -406,7 +404,7 @@ def test_draft_page_content_is_activated_when_experiment_goes_live(self): # set the experiment from draft to live self.client.post( - '/admin/experiments/experiment/edit/%d/' % self.experiment.pk, + f'/{self.admin_home}/experiments/experiment/edit/{self.experiment.pk}/', self.get_edit_postdata(status='live') ) # page content should be updated to follow the draft revision now @@ -418,8 +416,12 @@ def test_draft_page_content_is_activated_when_creating_experiment_as_live(self): self.homepage_alternative_1.body = 'updated' self.homepage_alternative_1.save_revision() + # live database entry should not have been updated yet + homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific + self.assertEqual(homepage_alternative_1.body, "Welcome to our site! It's lovely to meet you.") + # create a new experiment with an immediate live status - response = self.client.post('/admin/experiments/experiment/create/', { + response = self.client.post(f'/{self.admin_home}/experiments/experiment/create/', { 'name': "Another experiment", 'slug': 'another-experiment', 'control_page': self.homepage.pk, @@ -435,7 +437,7 @@ def test_draft_page_content_is_activated_when_creating_experiment_as_live(self): 'status': 'live', }) - self.assertRedirects(response, '/admin/experiments/experiment/') + self.assertRedirects(response, f'/{self.admin_home}/experiments/experiment/') # page content should be updated to follow the draft revision now homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific @@ -451,12 +453,12 @@ def test_draft_page_content_is_not_activated_on_published_pages(self): # make the experiment draft self.client.post( - '/admin/experiments/experiment/edit/%d/' % self.experiment.pk, + f'/{self.admin_home}/experiments/experiment/edit/{self.experiment.pk}/', self.get_edit_postdata(status='draft') ) # set the experiment from draft to live self.client.post( - '/admin/experiments/experiment/edit/%d/' % self.experiment.pk, + f'/{self.admin_home}/experiments/experiment/edit/{self.experiment.pk}/', self.get_edit_postdata(status='live') ) @@ -473,7 +475,7 @@ def test_draft_page_content_is_not_activated_on_published_pages_when_creating_ex self.homepage_alternative_1.save_revision() # create a new experiment with an immediate live status - response = self.client.post('/admin/experiments/experiment/create/', { + response = self.client.post(f'/{self.admin_home}/experiments/experiment/create/', { 'name': "Another experiment", 'slug': 'another-experiment', 'control_page': self.homepage.pk, @@ -489,44 +491,39 @@ def test_draft_page_content_is_not_activated_on_published_pages_when_creating_ex 'status': 'live', }) - self.assertRedirects(response, '/admin/experiments/experiment/') + self.assertRedirects(response, f'/{self.admin_home}/experiments/experiment/') # page content should still be unchanged homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific self.assertEqual(homepage_alternative_1.body, "Welcome to our site! It's lovely to meet you.") def test_experiment_delete(self): - response = self.client.get('/admin/experiments/experiment/delete/%d/' % self.experiment.pk) + response = self.client.get(f'/{self.admin_home}/experiments/experiment/delete/{self.experiment.pk}/') self.assertEqual(response.status_code, 200) self.assertContains(response, "Are you sure you want to delete this experiment?") - response = self.client.post('/admin/experiments/experiment/delete/%d/' % self.experiment.pk) - self.assertRedirects(response, '/admin/experiments/experiment/') + response = self.client.post(f'/{self.admin_home}/experiments/experiment/delete/{self.experiment.pk}/') + self.assertRedirects(response, f'/{self.admin_home}/experiments/experiment/') self.assertFalse(Experiment.objects.filter(slug='homepage-text').exists()) def test_show_report(self): - response = self.client.get('/admin/experiments/experiment/report/%d/' % self.experiment.pk) + response = self.client.get(f'/{self.admin_home}/experiments/experiment/report/{self.experiment.pk}/') self.assertEqual(response.status_code, 200) def test_select_winner(self): response = self.client.post( - '/admin/experiments/experiment/select_winner/%d/%d/' % ( - self.experiment.pk, self.homepage_alternative_1.pk - ) + f'/{self.admin_home}/experiments/experiment/select_winner/{self.experiment.pk}/{self.homepage_alternative_1.pk}/' ) self.assertRedirects( response, - '/admin/experiments/experiment/report/%d/' % self.experiment.pk - ) + f'/{self.admin_home}/experiments/experiment/report/{self.experiment.pk}/') experiment = Experiment.objects.get(pk=self.experiment.pk) self.assertEqual(experiment.status, 'completed') self.assertEqual(experiment.winning_variation.pk, self.homepage_alternative_1.pk) def test_preview(self): response = self.client.get( - '/admin/experiments/experiment/report/preview/%d/%d/' % ( - self.experiment.pk, self.homepage_alternative_1.pk - ) + f'/{self.admin_home}/experiments/experiment/report/preview/{self.experiment.pk}/{self.homepage_alternative_2.pk}/' ) self.assertEqual(response.status_code, 200) - self.assertContains(response, "Home") + self.assertContains(response, 'Home') diff --git a/tests/urls.py b/tests/urls.py index 39c538e..1c5790b 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -1,23 +1,17 @@ -from __future__ import absolute_import, unicode_literals -from django.conf.urls import include, url - -try: - from wagtail.admin import urls as wagtailadmin_urls - from wagtail.core import urls as wagtail_urls -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailadmin import urls as wagtailadmin_urls - from wagtail.wagtailcore import urls as wagtail_urls +from django.urls import include, re_path +from wagtail import urls as wagtail_urls +from wagtail.admin import urls as wagtailadmin_urls from experiments import views as experiment_views urlpatterns = [ - url(r'^admin/', include(wagtailadmin_urls)), + re_path(r'^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)), + re_path(r'', include(wagtail_urls)), ] diff --git a/wagtail_experiments b/wagtail_experiments new file mode 100644 index 0000000..e69de29