Add reverse code

.travis.yml

@@ -5,15 +5,11 @@ language: python
 matrix:
   include:
     - python: 3.6
-      env: TOXENV=lint
+      env: lint
     - python: 3.6
       env: TOXENV=py36-django20-wagtail20
-    - python: 3.6
-      env: TOXENV=py36-django20-wagtail20-geoip2
     - python: 3.6
       env: TOXENV=py36-django20-wagtail21
-    - python: 3.6
-      env: TOXENV=py36-django20-wagtail21-geoip2
 
 install:
   - pip install tox codecov

CHANGES

@@ -1,21 +1,3 @@
-0.12.0
-==================
- - Merged forks of Torchbox and Praekelt
- - Wagtail 2 compatibility
- - Makefile adjustments for portability
- - Adds simple segment forcing for superusers
- - Fix excluding pages without variant
- - Fix bug on visiting a segment page in the admin
- - Use Wagtail's logic in the page count in the dash
- - Prevent corrected summary item from counting the root page
- - Delete variants of a page that is being deleted
- - Add end user and developer documentation
- - Add an option to show a personalised block to everyone
- - Add origin country rule (#190)
- - Return 404 if variant page is accessed directly (#188)
- - Do not generate sitemap entries for variants (#187)
- - Remove restrictive wagtail dependency version constraint (#192)
-
 0.11.3
 ==================
  - Bugfix: Handle errors when testing an invalid visit count rule

docs/conf.py

@@ -62,10 +62,10 @@ author = 'Lab Digital BV'
 # built documents.
 #
 # The short X.Y version.
-version = '0.12.1'
+version = '0.12.0'
 
 # The full version, including alpha/beta/rc tags.
-release = '0.12.1'
+release = '0.12.0'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

docs/default_rules.rst

@@ -131,47 +131,3 @@ Is logged in        Whether the user is logged in or logged out.
 ==================  ==========================================================
 
 ``wagtail_personalisation.rules.UserIsLoggedInRule``
-
-
-Origin country rule
--------------------
-
-The origin country rule allows you to match visitors based on the origin
-country of their request. This rule requires to have set up a way to detect
-countries beforehand.
-
-==================  ==========================================================
-Option              Description
-==================  ==========================================================
-Country             What country user's request comes from.
-==================  ==========================================================
-
-You must have one of the following configurations set up in order to
-make it work.
-
-- Cloudflare IP Geolocation - ``cf-ipcountry`` HTTP header set with a value of
-  the alpha-2 country format.
-- CloudFront Geo-Targeting - ``cloudfront-viewer-country`` header set with a
-  value of the alpha-2 country format.
-- The last fallback is to use GeoIP2 module that is included with Django. This
-  requires setting up an IP database beforehand, see the Django's
-  `GeoIP2 instructions <https://docs.djangoproject.com/en/stable/ref/contrib/gis/geoip2/>`_
-  for more information. It will use IP of the request, using HTTP header
-  the ``x-forwarded-for`` HTTP header and ``REMOTE_ADDR`` server value as a
-  fallback. If you want to use a custom logic when obtaining IP address, please
-  set the ``WAGTAIL_PERSONALISATION_IP_FUNCTION`` setting to the function that takes a
-  request as an argument, e.g.
-
-  .. code-block:: python
-
-     # settings.py
-
-     WAGTAIL_PERSONALISATION_IP_FUNCTION = 'yourproject.utils.get_client_ip'
-
-
-     # yourproject/utils.py
-
-     def get_client_ip(request):
-         return request['HTTP_CF_CONNECTING_IP']
-
-``wagtail_personalisation.rules.OriginCountryRule``

sandbox/exampledata/personalisation.json

@@ -25,7 +25,7 @@
         "enable_date": "2017-06-02T10:58:39.389Z",
         "disable_date": "2017-06-02T10:34:51.722Z",
         "visit_count": 0,
-        "status": "enabled",
+        "enabled": true,
         "persistent": false,
         "match_any": false
     }
@@ -39,7 +39,7 @@
         "enable_date": "2017-06-02T10:57:44.497Z",
         "disable_date": "2017-06-02T10:57:39.984Z",
         "visit_count": 1,
-        "status": "enabled",
+        "enabled": true,
         "persistent": false,
         "match_any": false
     }

setup.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.12.1
+current_version = 0.12.0
 commit = true
 tag = true
 tag_name = {new_version}
@@ -15,7 +15,7 @@ python_paths = .
 [flake8]
 ignore = E731
 max-line-length = 120
-exclude = 
+exclude =
 	src/**/migrations/*.py
 
 [wheel]

setup.py

@@ -2,10 +2,9 @@ import re
 from setuptools import find_packages, setup
 
 install_requires = [
-    'wagtail>=2.0',
+    'wagtail>=2.0,<2.2',
     'user-agents>=1.1.0',
     'wagtailfontawesome>=1.1.3',
-    'pycountry',
 ]
 
 tests_require = [
@@ -35,7 +34,7 @@ with open('README.rst') as fh:
 
 setup(
     name='wagtail-personalisation',
-    version='0.12.1',
+    version='0.12.0',
     description='A Wagtail add-on for showing personalized content',
     author='Lab Digital BV and others',
     author_email='opensource@labdigital.nl',
@@ -60,7 +59,7 @@ setup(
         'Programming Language :: Python :: 3',
         'Programming Language :: Python :: 3.6',
         'Framework :: Django',
-        'Framework :: Django :: 2.0',
+        'Framework :: Django :: 2',
         'Topic :: Internet :: WWW/HTTP :: Site Management',
     ],
 )

src/wagtail_personalisation/blocks.py

@@ -8,7 +8,6 @@ from wagtail_personalisation.models import Segment
 
 
 def list_segment_choices():
-    yield -1, ("Show to everyone")
     for pk, name in Segment.objects.values_list('pk', 'name'):
         yield pk, name
 
@@ -36,19 +35,10 @@ class PersonalisedStructBlock(blocks.StructBlock):
         adapter = get_segment_adapter(request)
         user_segments = adapter.get_segments()
 
-        try:
-            segment_id = int(value['segment'])
-        except (ValueError, TypeError):
-            return ''
-
-        if segment_id > 0:
+        if value['segment']:
             for segment in user_segments:
-                if segment.id == segment_id:
+                if segment.id == int(value['segment']):
                     return super(PersonalisedStructBlock, self).render(
                         value, context)
 
-        if segment_id == -1:
-            return super(PersonalisedStructBlock, self).render(
-                value, context)
-
-        return ''
+        return ""

src/wagtail_personalisation/migrations/0024_segment_status_as_boolean.py

@@ -0,0 +1,45 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.2 on 2017-08-10 13:48
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+def forward(apps, schema_editor):
+    Segment = apps.get_model('wagtail_personalisation', 'Segment')
+
+    for segment in Segment.objects.all():
+        segment.enabled = segment.status == 'enabled'
+        segment.save()
+
+
+def backward(apps, schema_editor):
+    Segment = apps.get_model('wagtail_personalisation', 'Segment')
+
+    for segment in Segment.objects.all():
+        if segment.enabled:
+            segment.status = 'enabled'
+        else:
+            segment.status = 'disabled'
+
+        segment.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wagtail_personalisation', '0023_personalisablepagemetadata_variant_cascade'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='segment',
+            name='enabled',
+            field=models.BooleanField(default=True, help_text='Should the segment be active?'),
+        ),
+        migrations.RunPython(forward, reverse_code=backward),
+        migrations.RemoveField(
+            model_name='segment',
+            name='status',
+        ),
+    ]

src/wagtail_personalisation/models.py

@@ -10,7 +10,6 @@ from django.utils.functional import cached_property
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext_lazy as _
 from modelcluster.models import ClusterableModel
-import wagtail
 from wagtail.admin.edit_handlers import (
     FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel)
 from wagtail.core.models import Page
@@ -23,20 +22,12 @@ from .forms import SegmentAdminForm
 
 class SegmentQuerySet(models.QuerySet):
     def enabled(self):
-        return self.filter(status=self.model.STATUS_ENABLED)
+        return self.filter(enabled=True)
 
 
 @python_2_unicode_compatible
 class Segment(ClusterableModel):
     """The segment model."""
-    STATUS_ENABLED = 'enabled'
-    STATUS_DISABLED = 'disabled'
-
-    STATUS_CHOICES = (
-        (STATUS_ENABLED, _('Enabled')),
-        (STATUS_DISABLED, _('Disabled')),
-    )
-
     TYPE_DYNAMIC = 'dynamic'
     TYPE_STATIC = 'static'
 
@@ -51,8 +42,8 @@ class Segment(ClusterableModel):
     enable_date = models.DateTimeField(null=True, editable=False)
     disable_date = models.DateTimeField(null=True, editable=False)
     visit_count = models.PositiveIntegerField(default=0, editable=False)
-    status = models.CharField(
-        max_length=20, choices=STATUS_CHOICES, default=STATUS_ENABLED)
+    enabled = models.BooleanField(
+        default=True, help_text=_("Should the segment be active?"))
     persistent = models.BooleanField(
         default=False, help_text=_("Should the segment persist between visits?"))
     match_any = models.BooleanField(
@@ -112,7 +103,7 @@ class Segment(ClusterableModel):
             MultiFieldPanel([
                 FieldPanel('name', classname="title"),
                 FieldRowPanel([
-                    FieldPanel('status'),
+                    FieldPanel('enabled'),
                     FieldPanel('persistent'),
                 ]),
                 FieldPanel('match_any'),
@@ -179,9 +170,7 @@ class Segment(ClusterableModel):
         return segment_rules
 
     def toggle(self, save=True):
-        self.status = (
-            self.STATUS_ENABLED if self.status == self.STATUS_DISABLED
-            else self.STATUS_DISABLED)
+        self.enabled = not self.enabled
         if save:
             self.save()
 
@@ -303,15 +292,3 @@ class PersonalisablePageMixin:
             metadata = PersonalisablePageMetadata.objects.create(
                 canonical_page=self, variant=self)
         return metadata
-
-    def get_sitemap_urls(self, request=None):
-        # Do not generate sitemap entries for variants.
-        if not self.personalisation_metadata.is_canonical:
-            return []
-        if wagtail.VERSION >= (2, 2):
-            # Since Wagtail 2.2 you can pass request to the get_sitemap_urls
-            # method.
-            return super(PersonalisablePageMixin, self).get_sitemap_urls(
-                request=request
-            )
-        return super(PersonalisablePageMixin, self).get_sitemap_urls()

src/wagtail_personalisation/receivers.py

@@ -7,16 +7,16 @@ from wagtail_personalisation.models import Segment
 def check_status_change(sender, instance, *args, **kwargs):
     """Check if the status has changed. Alter dates accordingly."""
     try:
-        original_status = sender.objects.get(pk=instance.id).status
+        original_status = sender.objects.get(pk=instance.id).enabled
     except sender.DoesNotExist:
-        original_status = ""
+        original_status = None
 
-    if original_status != instance.status:
-        if instance.status == instance.STATUS_ENABLED:
+    if original_status != instance.enabled:
+        if instance.enabled is True:
             instance.enable_date = timezone.now()
             instance.visit_count = 0
             return instance
-        if instance.status == instance.STATUS_DISABLED:
+        if instance.enabled is False:
             instance.disable_date = timezone.now()
 
 

src/wagtail_personalisation/rules.py

@@ -1,11 +1,9 @@
 from __future__ import absolute_import, unicode_literals
-import logging
 
 import re
 from datetime import datetime
 from importlib import import_module
 
-import pycountry
 from django.apps import apps
 from django.conf import settings
 from django.contrib.sessions.models import Session
@@ -20,28 +18,8 @@ from user_agents import parse
 from wagtail.admin.edit_handlers import (
     FieldPanel, FieldRowPanel, PageChooserPanel)
 
-from wagtail_personalisation.utils import get_client_ip
-
 SessionStore = import_module(settings.SESSION_ENGINE).SessionStore
 
-logger = logging.getLogger(__name__)
-
-
-def get_geoip_module():
-    try:
-        from django.contrib.gis.geoip2 import GeoIP2
-        return GeoIP2
-    except ImportError:
-        logger.exception(
-            'GeoIP module is disabled. To use GeoIP for the origin\n'
-            'country personaliastion rule please set it up as per '
-            'documentation:\n'
-            'https://docs.djangoproject.com/en/stable/ref/contrib/gis/'
-            'geoip2/.\n'
-            'Wagtail-personalisation also works with Cloudflare and\n'
-            'CloudFront country detection, so you should not see this\n'
-            'warning if you use one of those.')
-
 
 @python_2_unicode_compatible
 class AbstractBaseRule(models.Model):
@@ -430,65 +408,3 @@ class UserIsLoggedInRule(AbstractBaseRule):
             'title': _('These visitors are'),
             'value': _('Logged in') if self.is_logged_in else _('Not logged in'),
         }
-
-
-COUNTRY_CHOICES = [(country.alpha_2.lower(), country.name)
-                   for country in pycountry.countries]
-
-
-class OriginCountryRule(AbstractBaseRule):
-    """
-    Test user against the country or origin of their request.
-
-    Using this rule requires setting up GeoIP2 on Django or using
-    CloudFlare or CloudFront geolocation detection.
-    """
-    country = models.CharField(
-        max_length=2, choices=COUNTRY_CHOICES,
-        help_text=_("Select origin country of the request that this rule will "
-                    "match against. This rule will only work if you use "
-                    "Cloudflare or CloudFront IP geolocation or if GeoIP2 "
-                    "module is configured.")
-    )
-
-    class Meta:
-        verbose_name = _("origin country rule")
-
-    def get_cloudflare_country(self, request):
-        """
-        Get country code that has been detected by Cloudflare.
-
-        Guide to the functionality:
-        https://support.cloudflare.com/hc/en-us/articles/200168236-What-does-Cloudflare-IP-Geolocation-do-
-        """
-        try:
-            return request.META['HTTP_CF_IPCOUNTRY'].lower()
-        except KeyError:
-            pass
-
-    def get_cloudfront_country(self, request):
-        try:
-            return request.META['HTTP_CLOUDFRONT_VIEWER_COUNTRY'].lower()
-        except KeyError:
-            pass
-
-    def get_geoip_country(self, request):
-        GeoIP2 = get_geoip_module()
-        if GeoIP2 is None:
-            return False
-        return GeoIP2().country_code(get_client_ip(request)).lower()
-
-    def get_country(self, request):
-        # Prioritise CloudFlare and CloudFront country detection over GeoIP.
-        functions = (
-            self.get_cloudflare_country,
-            self.get_cloudfront_country,
-            self.get_geoip_country,
-        )
-        for function in functions:
-            result = function(request)
-            if result:
-                return result
-
-    def test_user(self, request=None):
-        return (self.get_country(request) or '') == self.country.lower()

src/wagtail_personalisation/templates/modeladmin/wagtail_personalisation/segment/dashboard.html

@@ -22,7 +22,7 @@
                         <div class="nice-padding block_container">
                             {% if all_count %}
                                 {% for segment in object_list %}
-                                    <div class="block block--{{ segment.status }}" onclick="location.href = '{% url 'wagtail_personalisation_segment_modeladmin_edit' segment.pk %}'">
+                                    <div class="block block--{{ segment.enabled|yesno:"enabled,disabled" }}" onclick="location.href = '{% url 'wagtail_personalisation_segment_modeladmin_edit' segment.pk %}'">
                                         <h2>{{ segment }}</h2>
 
                                         <div class="inspect_container">
@@ -98,10 +98,10 @@
 
                                         {% if user_can_create %}
                                         <ul class="block_actions">
-                                            {% if segment.status == segment.STATUS_DISABLED %}
-                                                <li><a href="{% url 'segment:toggle' segment.pk %}" title="{% trans "Enable this segment" %}">enable</a></li>
-                                            {% elif segment.status == segment.STATUS_ENABLED %}
+                                            {% if segment.enabled %}
                                                 <li><a href="{% url 'segment:toggle' segment.pk %}" title="{% trans "Disable this segment" %}">disable</a></li>
+                                            {% else %}
+                                                <li><a href="{% url 'segment:toggle' segment.pk %}" title="{% trans "Enable this segment" %}">enable</a></li>
                                             {% endif %}
                                             <li><a href="{% url 'wagtail_personalisation_segment_modeladmin_edit' segment.pk %}" title="{% trans "Configure this segment" %}">configure this</a></li>
                                             {% if segment.is_static %}

src/wagtail_personalisation/utils.py

@@ -1,10 +1,8 @@
 import time
 
-from django.conf import settings
 from django.db.models import F
 from django.template.base import FilterExpression, kwarg_re
 from django.utils import timezone
-from django.utils.module_loading import import_string
 
 
 def impersonate_other_page(page, other_page):
@@ -118,17 +116,3 @@ def can_delete_pages(pages, user):
         if not variant.permissions_for_user(user).can_delete():
             return False
     return True
-
-
-def get_client_ip(request):
-    try:
-        func = import_string(settings.WAGTAIL_PERSONALISATION_IP_FUNCTION)
-    except AttributeError:
-        pass
-    else:
-        return func(request)
-    try:
-        x_forwarded_for = request.META['HTTP_X_FORWARDED_FOR']
-        return x_forwarded_for.split(',')[-1].strip()
-    except KeyError:
-        return request.META['REMOTE_ADDR']

src/wagtail_personalisation/views.py

@@ -83,7 +83,7 @@ class SegmentModelAdmin(ModelAdmin):
     delete_view_class = SegmentModelDeleteView
     menu_icon = 'fa-snowflake-o'
     add_to_settings_menu = False
-    list_display = ('name', 'persistent', 'match_any', 'status',
+    list_display = ('name', 'persistent', 'match_any', 'enabled',
                     'page_count', 'variant_count', 'statistics')
     index_view_extra_js = ['js/commons.js', 'js/index.js']
     index_view_extra_css = ['css/index.css']

src/wagtail_personalisation/wagtail_hooks.py

@@ -4,7 +4,6 @@ import logging
 
 from django.conf.urls import include, url
 from django.db import transaction
-from django.http import Http404
 from django.shortcuts import redirect, render
 from django.template.defaultfilters import pluralize
 from django.urls import reverse
@@ -105,13 +104,9 @@ def serve_variant(page, request, serve_args, serve_kwargs):
     adapter = get_segment_adapter(request)
     user_segments = adapter.get_segments()
 
-    metadata = page.personalisation_metadata
-
-    # If page is not canonical, don't serve it.
-    if not metadata.is_canonical:
-        raise Http404
-
     if user_segments:
+        metadata = page.personalisation_metadata
+
         # TODO: This is never more then one page? (fix query count)
         metadata = metadata.metadata_for_segments(user_segments)
         if metadata:

tests/factories/rule.py

@@ -46,8 +46,3 @@ class VisitCountRuleFactory(factory.DjangoModelFactory):
 
     class Meta:
         model = rules.VisitCountRule
-
-
-class OriginCountryRuleFactory(factory.DjangoModelFactory):
-    class Meta:
-        model = rules.OriginCountryRule

tests/factories/segment.py

@@ -7,7 +7,7 @@ from wagtail_personalisation import models
 
 class SegmentFactory(factory.DjangoModelFactory):
     name = 'TestSegment'
-    status = models.Segment.STATUS_ENABLED
+    enabled = True
 
     class Meta:
         model = models.Segment

tests/unit/test_adapter_session.py

@@ -64,7 +64,7 @@ def test_refresh_removes_disabled(rf):
     adapter.set_segments([segment_1, segment_2])
 
     adapter = adapters.SessionSegmentsAdapter(request)
-    segment_1.status = segment_1.STATUS_DISABLED
+    segment_1.enabled = False
     segment_1.save()
     adapter.refresh()
 

tests/unit/test_factories.py

@@ -15,14 +15,14 @@ from wagtail_personalisation.rules import TimeRule
 @pytest.mark.django_db
 def test_segment_create():
     factoried_segment = SegmentFactory()
-    segment = Segment(name='TestSegment', status='enabled')
+    segment = Segment(name='TestSegment', enabled=True)
     TimeRule(
         start_time=datetime.time(8, 0, 0),
         end_time=datetime.time(23, 0, 0),
         segment=segment)
 
     assert factoried_segment.name == segment.name
-    assert factoried_segment.status == segment.status
+    assert factoried_segment.enabled == segment.enabled
 
 
 @pytest.mark.django_db

tests/unit/test_models.py

@@ -60,16 +60,3 @@ def test_page_protection_when_deleting_segment(segmented_page):
     assert len(segment.get_used_pages())
     with pytest.raises(ProtectedError):
         segment.delete()
-
-
-@pytest.mark.django_db
-def test_sitemap_generation_for_canonical_pages_is_enabled(segmented_page):
-    canonical = segmented_page.personalisation_metadata.canonical_page
-    assert canonical.personalisation_metadata.is_canonical
-    assert canonical.get_sitemap_urls()
-
-
-@pytest.mark.django_db
-def test_sitemap_generation_for_variants_is_disabled(segmented_page):
-    assert not segmented_page.personalisation_metadata.is_canonical
-    assert not segmented_page.get_sitemap_urls()

tests/unit/test_rules_country_origin.py

@@ -1,203 +0,0 @@
-from importlib.util import find_spec
-from unittest.mock import call, MagicMock, patch
-
-import pytest
-
-from tests.factories.rule import OriginCountryRuleFactory
-from tests.factories.segment import SegmentFactory
-from wagtail_personalisation.rules import get_geoip_module
-
-
-skip_if_geoip2_installed = pytest.mark.skipif(
-    find_spec('geoip2'), reason='requires GeoIP2 to be not installed'
-)
-
-skip_if_geoip2_not_installed = pytest.mark.skipif(
-    not find_spec('geoip2'), reason='requires GeoIP2 to be installed.'
-)
-
-
-@pytest.mark.django_db
-def test_get_cloudflare_country_with_header(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/', HTTP_CF_IPCOUNTRY='PL')
-    assert rule.get_cloudflare_country(request) == 'pl'
-
-
-@pytest.mark.django_db
-def test_get_cloudflare_country_with_no_header(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/')
-    assert 'HTTP_CF_IPCOUNTRY' not in request.META
-    assert rule.get_cloudflare_country(request) is None
-
-
-@pytest.mark.django_db
-def test_get_cloudfront_country_with_header(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/', HTTP_CLOUDFRONT_VIEWER_COUNTRY='BY')
-    assert rule.get_cloudfront_country(request) == 'by'
-
-
-@pytest.mark.django_db
-def test_get_cloudfront_country_with_no_header(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/')
-    assert 'HTTP_CLOUDFRONT_VIEWER_COUNTRY' not in request.META
-    assert rule.get_cloudfront_country(request) is None
-
-
-@pytest.mark.django_db
-def test_get_geoip_country_with_remote_addr(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/', REMOTE_ADDR='173.254.89.34')
-    geoip_mock = MagicMock()
-    with patch('wagtail_personalisation.rules.get_geoip_module',
-               return_value=geoip_mock) as geoip_import_mock:
-        rule.get_geoip_country(request)
-    geoip_import_mock.assert_called_once()
-    geoip_mock.assert_called_once()
-    assert geoip_mock.mock_calls[1] == call().country_code('173.254.89.34')
-
-
-@pytest.mark.django_db
-def test_get_country_calls_all_methods(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/')
-
-    @patch.object(rule, 'get_geoip_country', return_value='')
-    @patch.object(rule, 'get_cloudflare_country', return_value='')
-    @patch.object(rule, 'get_cloudfront_country', return_value='')
-    def test_mock(cloudfront_mock, cloudflare_mock, geoip_mock):
-        country = rule.get_country(request)
-        cloudflare_mock.assert_called_once_with(request)
-        cloudfront_mock.assert_called_once_with(request)
-        geoip_mock.assert_called_once_with(request)
-        assert country is None
-
-    test_mock()
-
-
-@pytest.mark.django_db
-def test_get_country_does_not_use_all_detection_methods_unnecessarily(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/')
-
-    @patch.object(rule, 'get_geoip_country', return_value='')
-    @patch.object(rule, 'get_cloudflare_country', return_value='t1')
-    @patch.object(rule, 'get_cloudfront_country', return_value='')
-    def test_mock(cloudfront_mock, cloudflare_mock, geoip_mock):
-        country = rule.get_country(request)
-        cloudflare_mock.assert_called_once_with(request)
-        cloudfront_mock.assert_not_called()
-        geoip_mock.assert_not_called()
-        assert country == 't1'
-
-    test_mock()
-
-
-@pytest.mark.django_db
-def test_test_user_calls_get_country(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/')
-    with patch.object(rule, 'get_country') as get_country_mock:
-        rule.test_user(request)
-    get_country_mock.assert_called_once_with(request)
-
-
-@pytest.mark.django_db
-def test_test_user_returns_true_if_cloudflare_country_match(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/', HTTP_CF_IPCOUNTRY='GB')
-    assert rule.test_user(request) is True
-
-
-@pytest.mark.django_db
-def test_test_user_returns_false_if_cloudflare_country_doesnt_match(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/', HTTP_CF_IPCOUNTRY='NL')
-    assert not rule.test_user(request)
-
-
-@pytest.mark.django_db
-def test_test_user_returns_false_if_cloudfront_country_doesnt_match(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='GB')
-    request = rf.get('/', HTTP_CLOUDFRONT_VIEWER_COUNTRY='NL')
-    assert rule.test_user(request) is False
-
-
-@pytest.mark.django_db
-def test_test_user_returns_true_if_cloudfront_country_matches(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='se')
-    request = rf.get('/', HTTP_CLOUDFRONT_VIEWER_COUNTRY='SE')
-    assert rule.test_user(request) is True
-
-
-@skip_if_geoip2_not_installed
-@pytest.mark.django_db
-def test_test_user_geoip_module_matches(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='se')
-    request = rf.get('/', REMOTE_ADDR='123.120.0.2')
-    GeoIP2Mock = MagicMock()
-    GeoIP2Mock().configure_mock(**{'country_code.return_value': 'SE'})
-    GeoIP2Mock.reset_mock()
-    with patch('wagtail_personalisation.rules.get_geoip_module',
-               return_value=GeoIP2Mock):
-        assert rule.test_user(request) is True
-    assert GeoIP2Mock.mock_calls == [
-        call(),
-        call().country_code('123.120.0.2'),
-    ]
-
-
-@skip_if_geoip2_not_installed
-@pytest.mark.django_db
-def test_test_user_geoip_module_does_not_match(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='nl')
-    request = rf.get('/', REMOTE_ADDR='123.120.0.2')
-    GeoIP2Mock = MagicMock()
-    GeoIP2Mock().configure_mock(**{'country_code.return_value': 'SE'})
-    GeoIP2Mock.reset_mock()
-    with patch('wagtail_personalisation.rules.get_geoip_module',
-               return_value=GeoIP2Mock):
-        assert rule.test_user(request) is False
-    assert GeoIP2Mock.mock_calls == [
-        call(),
-        call().country_code('123.120.0.2')
-    ]
-
-
-@skip_if_geoip2_installed
-@pytest.mark.django_db
-def test_test_user_does_not_use_geoip_module_if_disabled(rf):
-    segment = SegmentFactory(name='Test segment')
-    rule = OriginCountryRuleFactory(segment=segment, country='se')
-    request = rf.get('/', REMOTE_ADDR='123.120.0.2')
-    assert rule.test_user(request) is False
-
-
-@skip_if_geoip2_installed
-def test_get_geoip_module_disabled():
-    with pytest.raises(ImportError):
-        from django.contrib.gis.geoip2 import GeoIP2  # noqa
-    assert get_geoip_module() is None
-
-
-@skip_if_geoip2_not_installed
-def test_get_geoip_module_enabled():
-    from django.contrib.gis.geoip2 import GeoIP2
-    assert get_geoip_module() is GeoIP2

tests/unit/test_static_dynamic_segments.py

@@ -12,7 +12,7 @@ from wagtail_personalisation.rules import TimeRule, VisitCountRule
 
 
 def form_with_data(segment, *rules):
-    model_fields = ['type', 'status', 'count', 'name', 'match_any', 'randomisation_percent']
+    model_fields = ['type', 'enabled', 'count', 'name', 'match_any', 'randomisation_percent']
 
     class TestSegmentAdminForm(SegmentAdminForm):
         class Meta:

tests/unit/test_utils.py

@@ -1,10 +1,8 @@
 import pytest
 
-from django.test import override_settings
-
 from tests.factories.page import ContentPageFactory
 from wagtail_personalisation.utils import (
-    can_delete_pages, get_client_ip, impersonate_other_page)
+    can_delete_pages, impersonate_other_page)
 
 
 @pytest.fixture
@@ -38,29 +36,3 @@ def test_can_delete_pages_with_superuser(rf, user, segmented_page):
 @pytest.mark.django_db
 def test_cannot_delete_pages_with_standard_user(user, segmented_page):
     assert not can_delete_pages([segmented_page], user)
-
-
-def test_get_client_ip_with_remote_addr(rf):
-    request = rf.get('/', REMOTE_ADDR='173.231.235.87')
-    assert get_client_ip(request) == '173.231.235.87'
-
-
-def test_get_client_ip_with_x_forwarded_for(rf):
-    request = rf.get('/', HTTP_X_FORWARDED_FOR='173.231.235.87',
-                     REMOTE_ADDR='10.0.23.24')
-    assert get_client_ip(request) == '173.231.235.87'
-
-
-@override_settings(
-    WAGTAIL_PERSONALISATION_IP_FUNCTION='some.non.existent.path'
-)
-def test_get_client_ip_custom_get_client_ip_function_does_not_exist(rf):
-    with pytest.raises(ImportError):
-        get_client_ip(rf.get('/'))
-
-
-@override_settings(
-    WAGTAIL_PERSONALISATION_IP_FUNCTION='tests.utils.get_custom_ip'
-)
-def test_get_client_ip_custom_get_client_ip_used(rf):
-    assert get_client_ip(rf.get('/')) == '123.123.123.123'

tests/unit/test_wagtail_hooks.py

@@ -1,7 +1,4 @@
 import pytest
-
-from django.http import Http404
-
 from wagtail.core.models import Page
 
 from tests.factories.segment import SegmentFactory
@@ -19,15 +16,6 @@ def test_serve_variant_no_variant(site, rf):
     assert result is None
 
 
-@pytest.mark.django_db
-def test_variant_accessed_directly_returns_404(segmented_page, rf):
-    request = rf.get('/')
-    args = tuple()
-    kwargs = {}
-    with pytest.raises(Http404):
-        wagtail_hooks.serve_variant(segmented_page, request, args, kwargs)
-
-
 @pytest.mark.django_db
 def test_serve_variant_with_variant_no_segment(site, rf, segmented_page):
     request = rf.get('/')

tests/utils.py

@@ -5,7 +5,3 @@ def render_template(value, **context):
     template = engines['django'].from_string(value)
     request = context.pop('request', None)
     return template.render(context, request)
-
-
-def get_custom_ip(request):
-    return '123.123.123.123'

tox.ini

@@ -1,15 +1,14 @@
 [tox]
-envlist = py{36}-django{20}-wagtail{20,21}{,-geoip2},lint
+envlist = py{36}-django{20}-wagtail{20,21},lint
 
 [testenv]
 basepython = python3.6
-commands = coverage run --parallel -m pytest -rs {posargs}
+commands = coverage run --parallel -m pytest {posargs}
 extras = test
 deps =
     django20: django>=2.0,<2.1
     wagtail20: wagtail>=2.0,<2.1
     wagtail21: wagtail>=2.1,<2.2
-    geoip2: geoip2
 
 [testenv:coverage-report]
 basepython = python3.6