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..5b5bdb4 --- /dev/null +++ b/.github/workflows/nightly-tests.yml @@ -0,0 +1,36 @@ +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.12 + uses: actions/setup-python@v2 + with: + python-version: "3.12" + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install "git+https://github.com/wagtail/wagtail.git@main#egg=wagtail" + pip install wagtail-modeladmin + 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..2f8786a --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,71 @@ + +name: CI + +on: + push: + branches: [ main ] + pull_request: + +# Current configuration: +# - django 4.2, python 3.8, wagtail 5.2, sqlite +# - django 5.0, python 3.10, wagtail 6.0, postgres +# - django 5.0, python 3.12, wagtail main, sqlite (allow failures) +jobs: + test: + runs-on: ubuntu-latest + continue-on-error: ${{ matrix.experimental }} + strategy: + matrix: + include: + - python: "3.8" + django: "Django>=4.2,<4.3" + wagtail: "wagtail>=5.2,<5.3" + database: "sqlite3" + modeladmin: true + experimental: false + - python: "3.10" + django: "Django>=5.0,<5.1" + wagtail: "wagtail>=6.0,<6.1" + database: "postgresql" + modeladmin: true + experimental: false + + - python: "3.12" + django: "Django>=5.0,<5.1" + wagtail: "git+https://github.com/wagtail/wagtail.git@main#egg=wagtail" + database: "sqlite3" + modeladmin: true + 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: Install wagtail-modeladmin + if: ${{ matrix.modeladmin }} + run: pip install wagtail-modeladmin + - 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..301f99b 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,31 @@ Changelog ========= +0.4 (2024-02-13) +~~~~~~~~~~~~~~~~ + + * Added support for Wagtail 6.0 (prevent draft edits made after an experiment goes live from showing on the front-end) + * Added support for Django 5.0 + * Dropped support for Wagtail <5.2 and Django <4.2 + * Fix: Prevent potential incorrect ordering of alternatives on PostgreSQL + * Fix: Add missing migration for changes to meta options + + +0.3.1 (2023-11-06) +~~~~~~~~~~~~~~~~~~ + + * Use external wagtail-modeladmin package where available + * Added support for Wagtail 5.1 - 5.2 and provisional support for Wagtail 6.0 + + +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/MANIFEST.in b/MANIFEST.in index 90acaca..eae8c08 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1 +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 c889f65..3fedf0f 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,22 +11,24 @@ 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 5.2 to 6.0, and Django 4.2 to 5.0. It depends on the Wagtail ModelAdmin module, which is available as an external package as of Wagtail 5.0; we recommend using this rather than the bundled `wagtail.contrib.modeladmin` module to avoid deprecation warnings. The external package is required as of Wagtail 6.0. + +To install:: - pip install wagtail-experiments + pip install wagtail-experiments wagtail-modeladmin -and ensure that the apps ``wagtail.contrib.modeladmin`` and ``experiments`` are included in your project's ``INSTALLED_APPS``: +and ensure that the apps ``wagtail_modeladmin`` and ``experiments`` are included in your project's ``INSTALLED_APPS``: .. code-block:: python INSTALLED_APPS = [ # ... - 'wagtail.contrib.modeladmin', + 'wagtail_modeladmin', 'experiments', # ... ] -Then migrate:: +Then migrate:: ./manage.py migrate diff --git a/createmigrations.py b/createmigrations.py new file mode 100644 index 0000000..c9e55c8 --- /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']) \ No newline at end of file diff --git a/experiments/admin_urls.py b/experiments/admin_urls.py index 6d05a30..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.conf.urls import url +from django.urls import re_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'), + 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 4749113..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/middleware.py b/experiments/middleware.py new file mode 100644 index 0000000..3245add --- /dev/null +++ b/experiments/middleware.py @@ -0,0 +1,22 @@ +from .models import Experiment +from .utils import get_user_id + + +class GoalURLMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + current_url = request.path + 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 exp in experiments: + exp.record_completion_for_user(user_id, request) + + response = self.get_response(request) + return response \ No newline at end of file diff --git a/experiments/migrations/0006_auto_20180213_1541.py b/experiments/migrations/0006_auto_20180213_1541.py new file mode 100644 index 0000000..e540260 --- /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), + ), + ] \ No newline at end of file diff --git a/experiments/migrations/0006_downcase_verbose_names.py b/experiments/migrations/0006_downcase_verbose_names.py new file mode 100644 index 0000000..d08e99d --- /dev/null +++ b/experiments/migrations/0006_downcase_verbose_names.py @@ -0,0 +1,35 @@ +# Generated by Django 4.2.4 on 2024-02-12 21:51 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("experiments", "0005_make_goal_optional"), + ] + + operations = [ + migrations.AlterModelOptions( + name="alternative", + options={ + "ordering": ["sort_order"], + "verbose_name": "alternative", + "verbose_name_plural": "alternatives", + }, + ), + migrations.AlterModelOptions( + name="experiment", + options={ + "verbose_name": "experiment", + "verbose_name_plural": "experiments", + }, + ), + migrations.AlterModelOptions( + name="experimenthistory", + options={ + "verbose_name": "experiment history", + "verbose_name_plural": "experiment histories", + }, + ), + ] diff --git a/experiments/migrations/0007_alternative_revision.py b/experiments/migrations/0007_alternative_revision.py new file mode 100644 index 0000000..dc5ed59 --- /dev/null +++ b/experiments/migrations/0007_alternative_revision.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.4 on 2024-02-12 21:52 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("wagtailcore", "0078_referenceindex"), + ("experiments", "0006_downcase_verbose_names"), + ] + + operations = [ + migrations.AddField( + model_name="alternative", + name="revision", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to="wagtailcore.revision", + ), + ), + ] diff --git a/experiments/migrations/0007_auto_20181028_1516.py b/experiments/migrations/0007_auto_20181028_1516.py new file mode 100644 index 0000000..0a86399 --- /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'), + ), + ] \ No newline at end of file diff --git a/experiments/models.py b/experiments/models.py index 72def6c..8982523 100644 --- a/experiments/models.py +++ b/experiments/models.py @@ -1,27 +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.encoding import python_2_unicode_compatible +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,70 +34,159 @@ def get_backend(): return BACKEND -@python_2_unicode_compatible 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) 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 = [ 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 + ''' + Called when making an experiment live. Update the experiment record to store the + current page revisions of the alternatives - the live revision if it is live, or + else the current draft revision. This ensures that any draft edits made after the + experiment goes live are not displayed to users. + + 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.live = False - revision.has_unpublished_changes = True - revision.save() + if alternative.page.live: + alternative.revision = alternative.page.live_revision + else: + revision = alternative.page.get_latest_revision() + if not revision: + revision = alternative.page.save_revision() + alternative.revision = revision + alternative.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', 'revision'): + if alternative.revision and not alternative.page.live: + # retrieve the version of the page indicated by the revision, so that we're + # not reflecting drafts that have been made since the experiment was created + variations.append(alternative.revision.as_object()) + else: + 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,18 +196,29 @@ 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) + revision = models.ForeignKey('wagtailcore.Revision', related_name='+', on_delete=models.SET_NULL, null=True, blank=True) panels = [ PageChooserPanel('page'), ] + class Meta(Orderable.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 +229,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 ae960e5..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 ugettext as _ +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 05ca572..6b09641 100644 --- a/experiments/wagtail_hooks.py +++ b/experiments/wagtail_hooks.py @@ -1,23 +1,23 @@ -from __future__ import absolute_import, unicode_literals - -from django.conf.urls import include, url +from django.urls import include, re_path from django.contrib.admin.utils import quote +from django.urls import reverse -try: - from django.urls import reverse -except ImportError: # fallback for Django <=1.9 - from django.core.urlresolvers import reverse - -from django.utils.translation import ugettext_lazy as _ +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 +from wagtail import hooks try: - from wagtail.core import hooks -except ImportError: # fallback for Wagtail <2.0 - from wagtail.wagtailcore import hooks + from wagtail_modeladmin.helpers import ButtonHelper + from wagtail_modeladmin.options import ModelAdmin, modeladmin_register + from wagtail_modeladmin.views import CreateView, EditView +except ImportError: + try: + from wagtail.contrib.modeladmin.helpers import ButtonHelper + from wagtail.contrib.modeladmin.options import ModelAdmin, modeladmin_register + from wagtail.contrib.modeladmin.views import CreateView, EditView + except ImportError: + raise ImportError("wagtail-experiments requires the wagtail-modeladmin package.") + from .models import Experiment from .utils import get_user_id, impersonate_other_page @@ -26,7 +26,7 @@ @hooks.register('register_admin_urls') def register_admin_urls(): return [ - url(r'^experiments/', include(admin_urls, namespace='experiments')), + re_path(r'^experiments/', include(admin_urls, namespace='experiments')), ] @@ -56,6 +56,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() @@ -64,6 +67,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() @@ -72,6 +78,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 @@ -83,7 +92,25 @@ 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') if completed_experiments: @@ -108,7 +135,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/runtests.py b/runtests.py index 290aa85..3374921 100755 --- a/runtests.py +++ b/runtests.py @@ -6,4 +6,4 @@ from django.core.management import execute_from_command_line os.environ['DJANGO_SETTINGS_MODULE'] = 'tests.settings' -execute_from_command_line([sys.argv[0], 'test']) +execute_from_command_line([sys.argv[0], 'test'] + sys.argv[1:]) diff --git a/setup.py b/setup.py index eac34c2..025353d 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ setup( name='wagtail-experiments', - version='0.2', + version='0.4', 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', + 'Programming Language :: Python :: 3.11', + 'Programming Language :: Python :: 3.12', 'Framework :: Django', + 'Framework :: Django :: 4.2', + 'Framework :: Django :: 5.0', 'Framework :: Wagtail', - 'Framework :: Wagtail :: 1', - 'Framework :: Wagtail :: 2', + 'Framework :: Wagtail :: 5', + 'Framework :: Wagtail :: 6', ], ) + diff --git a/tests/models.py b/tests/models.py index 2a7345c..f278fff 100644 --- a/tests/models.py +++ b/tests/models.py @@ -1,13 +1,7 @@ -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.admin.panels import FieldPanel, InlinePanel +from wagtail.models import Orderable, Page, Site class SimplePage(Page): @@ -15,9 +9,15 @@ 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 + content_panels = Page.content_panels + [ + FieldPanel('body'), + InlinePanel('related_links', label="Related links"), + ] + class SimplePageRelatedLink(Orderable): page = ParentalKey(SimplePage, related_name='related_links') diff --git a/tests/settings.py b/tests/settings.py index 93ee049..92d42c1 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,10 +1,16 @@ -from __future__ import absolute_import, unicode_literals - import os import django from wagtail import VERSION as WAGTAIL_VERSION +try: + import wagtail_modeladmin +except ImportError: + HAS_MODELADMIN_PACKAGE = False +else: + HAS_MODELADMIN_PACKAGE = True + + DATABASES = { 'default': { 'ENGINE': os.environ.get('DATABASE_ENGINE', 'django.db.backends.sqlite3'), @@ -44,89 +50,53 @@ 'django.contrib.auth.context_processors.auth', 'django.contrib.messages.context_processors.messages', 'django.template.context_processors.request', + 'experiments.middleware.GoalURLMiddleware', ], 'debug': True, }, }, ] -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', - ] - - if WAGTAIL_VERSION < (2, 0): - MIDDLEWARE.append('wagtail.wagtailcore.middleware.SiteMiddleware') - else: - MIDDLEWARE.append('wagtail.core.middleware.SiteMiddleware') +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', +] -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_modeladmin' if HAS_MODELADMIN_PACKAGE else '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 fc1f379..1575788 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.test import Client, TestCase +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): @@ -185,6 +177,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' @@ -252,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): @@ -268,6 +298,13 @@ 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 + # fill in revision IDs (not in the fixture) + for alternative in self.experiment.alternatives.all(): + alternative.revision = alternative.page.specific.save_revision() + alternative.save() + + self.admin_home = reverse('wagtailadmin_home').strip('/') + def get_edit_postdata(self, **kwargs): alternatives = self.experiment.alternatives.all() @@ -296,21 +333,50 @@ def get_edit_postdata(self, **kwargs): postdata.update(kwargs) return postdata + def make_draft_page_edit(self, page, title, slug, body): + post_data = { + 'title': title, + 'slug': slug, + 'body': body, + 'related_links-TOTAL_FORMS': 0, + 'related_links-INITIAL_FORMS': 0, + 'related_links-MIN_NUM_FORMS': 0, + 'related_links-MAX_NUM_FORMS': 1000, + } + response = self.client.post( + reverse("wagtailadmin_pages:edit", args=(page.id,)), post_data + ) + self.assertRedirects( + response, reverse("wagtailadmin_pages:edit", args=(page.id,)) + ) + page.refresh_from_db() + + def get_homepage(self, user_id): + client = Client() + session = client.session + session['experiment_user_id'] = user_id + session.save() + response = client.get('/') + return response + 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, @@ -325,64 +391,79 @@ 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") def test_draft_page_content_is_activated_when_experiment_goes_live(self): # make a draft edit to homepage_alternative_1 - self.homepage_alternative_1.body = 'updated' - self.homepage_alternative_1.save_revision() + self.make_draft_page_edit(self.homepage_alternative_1, "Homepage alternative 1", "home-alternative-1", "updated") - # 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.") + # The version seen by users receiving that alternative should remain unchanged + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

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

") + self.assertNotContains(response, "

updated

") # 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 - 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.") + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

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

") + self.assertNotContains(response, "

updated

") # 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 - 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.") + # content should revert to the control page + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

Welcome to our site!

") + self.assertNotContains(response, "

updated

") # 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 - homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific - self.assertEqual(homepage_alternative_1.body, 'updated') + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

updated

") + self.assertNotContains(response, "

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

") def test_draft_page_content_is_activated_when_creating_experiment_as_live(self): # make a draft edit to homepage_alternative_1 self.homepage_alternative_1.body = 'updated' - self.homepage_alternative_1.save_revision() + self.make_draft_page_edit(self.homepage_alternative_1, "Homepage alternative 1", "home-alternative-1", "updated") + + # The version seen by users receiving that alternative should remain unchanged + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

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

") + self.assertNotContains(response, "

updated

") + + self.experiment.delete() # 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, @@ -398,45 +479,47 @@ 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 - self.assertEqual(homepage_alternative_1.body, 'updated') + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

updated

") + self.assertNotContains(response, "

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

") def test_draft_page_content_is_not_activated_on_published_pages(self): # publish homepage_alternative_1 self.homepage_alternative_1.save_revision().publish() # make a draft edit to homepage_alternative_1 - self.homepage_alternative_1.body = 'updated' - self.homepage_alternative_1.save_revision() + self.make_draft_page_edit(self.homepage_alternative_1, "Homepage alternative 1", "home-alternative-1", "updated") # 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') ) - # 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.") + # page content seen by users receiving that alternative should remain unchanged + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

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

") + self.assertNotContains(response, "

updated

") def test_draft_page_content_is_not_activated_on_published_pages_when_creating_experiment_as_live(self): # publish homepage_alternative_1 self.homepage_alternative_1.save_revision().publish() # make a draft edit to homepage_alternative_1 - self.homepage_alternative_1.body = 'updated' - self.homepage_alternative_1.save_revision() + self.make_draft_page_edit(self.homepage_alternative_1, "Homepage alternative 1", "home-alternative-1", "updated") # 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, @@ -452,44 +535,41 @@ 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.") + # page content seen by users receiving that alternative should remain unchanged + response = self.get_homepage('33333333-3333-3333-3333-333333333333') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "

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

") + self.assertNotContains(response, "

updated

") 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/tox.ini b/tox.ini deleted file mode 100644 index 9608748..0000000 --- a/tox.ini +++ /dev/null @@ -1,34 +0,0 @@ -[tox] -envlist = py{27,34,35,36,37}-dj{18,19,110,111,20,21}-wt{17,18,19,110,111,112,113,20,21,22,23} - -[testenv] -basepython = - py27: python2.7 - py34: python3.4 - py35: python3.5 - py36: python3.6 - py37: python3.7 - -deps = - dj18: Django>=1.8,<1.9 - dj18: djangorestframework>=3.6,<3.7 - dj19: Django>=1.9,<1.10 - dj19: djangorestframework>=3.6,<3.7 - dj110: Django>=1.10,<1.11 - dj111: Django>=1.11,<1.12 - dj20: Django>=2.0,<2.1 - dj21: Django>=2.1,<2.2 - - wt17: wagtail>=1.7,<1.8 - wt18: wagtail>=1.8,<1.9 - wt19: wagtail>=1.9,<1.10 - wt110: wagtail>=1.10,<1.11 - wt111: wagtail>=1.11,<1.12 - wt112: wagtail>=1.12,<1.13 - wt113: wagtail>=1.13,<1.14 - wt20: wagtail>=2.0,<2.1 - wt21: wagtail>=2.1,<2.2 - wt22: wagtail>=2.2,<2.3 - wt23: wagtail>=2.3,<2.4 - -commands = ./runtests.py diff --git a/wagtail_experiments b/wagtail_experiments new file mode 100644 index 0000000..e69de29