refactor and add tests

src/wagtail_personalisation/models.py

@@ -10,6 +10,7 @@ from wagtail.utils.decorators import cached_classmethod
 from wagtail.wagtailadmin.edit_handlers import (
     FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel, ObjectList,
     PageChooserPanel, TabbedInterface)
+from wagtail.wagtailcore.models import Page
 
 from wagtail_personalisation.forms import AdminPersonalisablePageForm
 from wagtail_personalisation.rules import AbstractBaseRule
@@ -137,6 +138,27 @@ class PersonalisablePageMixin(models.Model):
         """
         return not self.canonical_page and self.has_variations
 
+    def copy_for_segment(self, segment):
+        slug = "{}-{}".format(self.slug, segment.encoded_name())
+        title = "{} ({})".format(self.title, segment.name)
+        update_attrs = {
+            'title': title,
+            'slug': slug,
+            'segment': segment,
+            'live': False,
+            'canonical_page': self,
+            'is_segmented': True,
+        }
+
+        try:
+            return Page.objects.get(slug=slug, parent=self.parent).specific
+        except Page.DoesNotExist:
+            return self.copy(update_attrs=update_attrs, copy_revisions=False)
+
+    def variants_for_segments(self, segments):
+        return self.__class__.objects.filter(
+            canonical_page=self, segment__in=segments)
+
 
 @cached_classmethod
 def get_edit_handler(cls):

src/wagtail_personalisation/views.py

@@ -9,7 +9,7 @@ from wagtail.contrib.modeladmin.options import ModelAdmin, modeladmin_register
 from wagtail.contrib.modeladmin.views import IndexView
 from wagtail.wagtailcore.models import Page
 
-from wagtail_personalisation.models import PersonalisablePageMixin, Segment
+from wagtail_personalisation.models import Segment
 
 
 class SegmentModelIndexView(IndexView):
@@ -123,28 +123,11 @@ def copy_page_view(request, page_id, segment_id):
     :rtype: django.http.HttpResponseRedirect
 
     """
-    model = PersonalisablePageMixin.get_model()
     if request.user.has_perm('wagtailadmin.access_admin'):
         segment = get_object_or_404(Segment, pk=segment_id)
-        page = get_object_or_404(model, pk=page_id)
+        page = get_object_or_404(Page, pk=page_id).specific
-
+        new_page = page.copy_for_segment(segment)
-        slug = "{}-{}".format(page.slug, segment.encoded_name())
+        edit_url = reverse('wagtailadmin_pages:edit', args=[new_page.id])
-        title = "{} ({})".format(page.title, segment.name)
-        update_attrs = {
-            'title': title,
-            'slug': slug,
-            'segment': segment,
-            'live': False,
-            'canonical_page': page,
-            'is_segmented': True,
-        }
-
-        try:
-            variant = Page.objects.get(slug=slug, parent=page.parent)
-        except Page.DoesNotExist:
-            variant = page.copy(update_attrs=update_attrs, copy_revisions=False)
-
-        edit_url = reverse('wagtailadmin_pages:edit', args=[variant.id])
 
         return HttpResponseRedirect(edit_url)
 

src/wagtail_personalisation/wagtail_hooks.py

@@ -108,21 +108,7 @@ def _check_for_variations(segments, page):
     :rtype: wagtail_personalisation.models.PersonalisablePage or None
 
     """
-    try:
+    return page.variants_for_segments(segments)
-        model = PersonalisablePageMixin.__subclasses__()[0]
-    except IndexError:
-        return
-    for segment in segments:
-        page_class = page.__class__
-        if any(item == model for item in page_class.__bases__):
-
-            variation = page_class.objects.filter(
-                canonical_page=page, segment=segment)
-
-            if variation:
-                return variation
-
-    return None
 
 
 @hooks.register('register_page_listing_buttons')
@@ -132,10 +118,10 @@ def page_listing_variant_buttons(page, page_perms, is_parent=False):
 
     """
 
-    pages = Page.objects.filter(pk=page.pk)
+    if not hasattr(page, 'segment'):
+        return
+    pages = page.__class__.objects.filter(pk=page.pk)
     segments = Segment.objects.all()
-    if pages:
-        pages = [x.specific_class() for x in pages]
 
     if pages and len(segments) > 0 and not (
             any(item.segment for item in pages)):
@@ -155,10 +141,7 @@ def page_listing_more_buttons(page, page_perms, is_parent=False):
     create a new variant for the selected segment.
 
     """
-    try:
+    model = page.__class__
-        model = PersonalisablePageMixin.__subclasses__()[0]
-    except IndexError:
-        return
     segments = Segment.objects.all()
     available_segments = [
         item for item in segments

tests/conftest.py

@@ -5,6 +5,10 @@ from wagtail.wagtailcore.models import Page, Site
 from wagtail_factories import SiteFactory
 from tests.factories.page import PageFactory
 
+pytest_plugins = [
+    'tests.fixtures'
+]
+
 
 @pytest.fixture(scope='session')
 def django_db_setup(django_db_setup, django_db_blocker):

tests/factories/page.py

@@ -1,16 +1,15 @@
 from __future__ import absolute_import, unicode_literals
 
-from wagtail_factories import PageFactory
+import factory
+from django.utils.text import slugify
+from wagtail_factories.factories import MP_NodeFactory
 
 from tests.sandbox.pages.models import HomePage
-from wagtail_personalisation.models import PersonalisablePage
 
 
-class PersonalisablePageFactory(PageFactory):
+class PageFactory(MP_NodeFactory):
-    class Meta:
+    title = 'Test page'
-        model = PersonalisablePage
+    slug = factory.LazyAttribute(lambda obj: slugify(obj.title))
 
-
-class HomePageFactory(PageFactory):
     class Meta:
         model = HomePage

tests/factories/site.py

@@ -1,17 +1,7 @@
 import factory
-from django.utils.text import slugify
 from wagtail.wagtailcore.models import Site
-from wagtail_factories.factories import MP_NodeFactory
 
-from tests.sandbox.pages.models import HomePage
+from tests.factories.page import PageFactory
-
-
-class PageFactory(MP_NodeFactory):
-    title = 'Test page'
-    slug = factory.LazyAttribute(lambda obj: slugify(obj.title))
-
-    class Meta:
-        model = HomePage
 
 
 class SiteFactory(factory.DjangoModelFactory):

tests/fixtures.py

@@ -0,0 +1,17 @@
+import pytest
+
+from tests.factories.page import PageFactory
+from tests.factories.segment import SegmentFactory
+from tests.factories.site import SiteFactory
+
+
+@pytest.fixture
+def site():
+    return SiteFactory()
+
+
+@pytest.fixture
+def segmented_page(site):
+    page = PageFactory(parent=site.root_page)
+    segment = SegmentFactory()
+    return page.copy_for_segment(segment)

tests/sandbox/settings.py

@@ -31,10 +31,13 @@ STATICFILES_FINDERS = (
 
 USE_TZ = False
 
+BASE_DIR = os.path.dirname(os.path.abspath(__file__))
 TEMPLATES = [
     {
         'BACKEND': 'django.template.backends.django.DjangoTemplates',
-        'DIRS': [],
+        'DIRS': [
+            os.path.join(BASE_DIR, 'templates'),
+        ],
         'APP_DIRS': True,
         'OPTIONS': {
             'context_processors': [

tests/sandbox/templates/404.html

@@ -0,0 +1,9 @@
+{% extends "base.html" %}
+
+{% block body_class %}template-404{% endblock %}
+
+{% block content %}
+    <h1>Page not found</h1>
+
+    <h2>Sorry, this page could not be found.</h2>
+{% endblock %}

tests/sandbox/templates/500.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<!--[if lt IE 7]>      <html class="no-js lt-ie9 lt-ie8 lt-ie7"> <![endif]-->
+<!--[if IE 7]>         <html class="no-js lt-ie9 lt-ie8"> <![endif]-->
+<!--[if IE 8]>         <html class="no-js lt-ie9"> <![endif]-->
+<!--[if gt IE 8]><!--> <html class="no-js"> <!--<![endif]-->
+    <head>
+        <meta charset="utf-8" />
+        <meta http-equiv="X-UA-Compatible" content="IE=edge" />
+        <title>Internal server error</title>
+        <meta name="viewport" content="width=device-width, initial-scale=1" />
+    </head>
+    <body>
+        <h1>Internal server error</h1>
+
+        <h2>Sorry, there seems to be an error. Please try again soon.</h2>
+    </body>
+</html>

tests/sandbox/templates/base.html

@@ -0,0 +1,44 @@
+{% load static wagtailuserbar %}
+
+<!DOCTYPE html>
+<!--[if lt IE 7]>      <html class="no-js lt-ie9 lt-ie8 lt-ie7"> <![endif]-->
+<!--[if IE 7]>         <html class="no-js lt-ie9 lt-ie8"> <![endif]-->
+<!--[if IE 8]>         <html class="no-js lt-ie9"> <![endif]-->
+<!--[if gt IE 8]><!--> <html class="no-js"> <!--<![endif]-->
+    <head>
+        <meta charset="utf-8" />
+        <meta http-equiv="X-UA-Compatible" content="IE=edge" />
+        <title>
+            {% block title %}
+                {% if self.seo_title %}{{ self.seo_title }}{% else %}{{ self.title }}{% endif %}
+            {% endblock %}
+            {% block title_suffix %}
+                {% with self.get_site.site_name as site_name %}
+                    {% if site_name %}- {{ site_name }}{% endif %}
+                {% endwith %}
+            {% endblock %}
+        </title>
+        <meta name="description" content="" />
+        <meta name="viewport" content="width=device-width, initial-scale=1" />
+
+        {# Global stylesheets #}
+        <link rel="stylesheet" type="text/css" href="{% static 'css/sandbox.css' %}">
+
+        {% block extra_css %}
+            {# Override this in templates to add extra stylesheets #}
+        {% endblock %}
+    </head>
+
+    <body class="{% block body_class %}{% endblock %}">
+        {% wagtailuserbar %}
+
+        {% block content %}{% endblock %}
+
+        {# Global javascript #}
+        <script type="text/javascript" src="{% static 'js/sandbox.js' %}"></script>
+
+        {% block extra_js %}
+            {# Override this in templates to add extra javascript #}
+        {% endblock %}
+    </body>
+</html>

tests/sandbox/templates/pages/home_page.html

@@ -0,0 +1 @@
+{% extends "base.html" %}

tests/unit/test_factories.py

@@ -7,6 +7,8 @@ import pytest
 from tests.factories.rule import (
     DayRuleFactory, DeviceRuleFactory, ReferralRuleFactory, TimeRuleFactory)
 from tests.factories.segment import SegmentFactory
+from tests.factories.site import SiteFactory
+from tests.factories.page import PageFactory
 from wagtail_personalisation.models import Segment
 from wagtail_personalisation.rules import TimeRule
 
@@ -84,3 +86,17 @@ def test_create_segment_with_new_referral_rule():
         segment=segment)
 
     assert segment.referral_rule.regex_string == 'test.notest'
+
+
+@pytest.mark.django_db
+def test_site_factory():
+    site = SiteFactory()
+    assert site
+
+
+@pytest.mark.django_db
+def test_page_factory():
+    site = SiteFactory()
+    assert site.root_page
+    page = PageFactory(parent=site.root_page)
+    assert page.get_parent() == site.root_page

tests/unit/test_hooks.py

@@ -0,0 +1,13 @@
+import pytest
+
+from wagtail_personalisation import wagtail_hooks as hooks
+from wagtail_personalisation.models import Segment
+
+
+@pytest.mark.django_db
+def test_check_for_variations(segmented_page):
+    segments = Segment.objects.all()
+    page = segmented_page.canonical_page
+
+    variations = hooks._check_for_variations(segments, page)
+    assert variations

tests/unit/test_middleware.py

@@ -4,13 +4,13 @@ import datetime
 
 import pytest
 from freezegun import freeze_time
-from wagtail_factories import SiteFactory
 
 from tests.factories.page import PageFactory
 from tests.factories.rule import (
     DayRuleFactory, DeviceRuleFactory, QueryRuleFactory, ReferralRuleFactory,
     TimeRuleFactory, VisitCountRuleFactory)
 from tests.factories.segment import SegmentFactory
+from tests.factories.site import SiteFactory
 
 
 @pytest.mark.django_db

tests/unit/test_models.py

@@ -6,7 +6,6 @@ import pytest
 from django.http import HttpRequest
 from freezegun import freeze_time
 from wagtail.wagtailcore.models import Site
-from tests.factories.site import SiteFactory
 
 from wagtail_personalisation import rules
 
@@ -49,12 +48,22 @@ def test_visit_count_rule():
 # Test test
 
 @pytest.mark.django_db
-def test_test(rf):
+def test_test(rf, site):
-    site = SiteFactory()
-
     request = HttpRequest()
     request.path = '/'
     request.META['HTTP_HOST'] = 'localhost'
     request.META['SERVER_PORT'] = 8000
 
     assert Site.find_for_request(request) == site
+
+
+@pytest.mark.django_db
+class TestPage(object):
+    def test_page_copy_for_segment(self, segmented_page):
+        assert segmented_page
+
+    def test_page_has_variations(self, segmented_page):
+        assert not segmented_page.is_canonical
+        assert not segmented_page.has_variations
+        assert segmented_page.canonical_page.is_canonical
+        assert segmented_page.canonical_page.has_variations