cleanups and bug fix

src/personalisation/admin.py

@@ -10,20 +10,23 @@ class TimeRuleAdminInline(admin.TabularInline):
 
 
 class ReferralRuleAdminInline(admin.TabularInline):
-    """Inline the Referral Rule into the administration interface for segments"""
+    """Inline the Referral Rule into the
+    administration interface for segments"""
     model = models.ReferralRule
     extra = 0
 
 
 class VisitCountRuleAdminInline(admin.TabularInline):
-    """Inline the Visit Count Rule into the administration interface for segments"""
+    """Inline the Visit Count Rule into the
+    administration interface for segments"""
     model = models.VisitCountRule
     extra = 0
 
 
 class SegmentAdmin(admin.ModelAdmin):
     """Add the inlines to the Segment admin interface"""
-    inlines = (TimeRuleAdminInline, ReferralRuleAdminInline, VisitCountRuleAdminInline)
+    inlines = (TimeRuleAdminInline,
+               ReferralRuleAdminInline, VisitCountRuleAdminInline)
 
 
 admin.site.register(models.Segment, SegmentAdmin)

src/personalisation/admin_urls.py

@@ -10,5 +10,6 @@ urlpatterns = [
         name='enable'),
     url(r'^segment/(?P<segment_id>[0-9]+)/disable/$', views.disable,
         name='disable'),
-    url(r'^personalisation/(?P<page_id>[0-9]+)/copy/(?P<segment_id>[0-9]+)$', views.copy_page_view, name='copy_page')
+    url(r'^personalisation/(?P<page_id>[0-9]+)/copy/(?P<segment_id>[0-9]+)$',
+        views.copy_page_view, name='copy_page')
 ]

src/personalisation/models.py

@@ -71,7 +71,8 @@ class TimeRule(AbstractBaseRule):
 @python_2_unicode_compatible
 class ReferralRule(AbstractBaseRule):
     """Referral rule to segment users based on a regex test"""
-    regex_string = models.TextField(_("Regex string to match the referer with"))
+    regex_string = models.TextField(
+        _("Regex string to match the referer with"))
 
     panels = [
         FieldPanel('regex_string'),
@@ -101,7 +102,8 @@ class VisitCountRule(AbstractBaseRule):
         ('less_than', _("Less than")),
         ('equal_to', _("Equal to")),
     )
-    operator = models.CharField(max_length=20, choices=OPERATOR_CHOICES, default="more_than")
+    operator = models.CharField(max_length=20,
+                                choices=OPERATOR_CHOICES, default="more_than")
     count = models.PositiveSmallIntegerField(default=0, null=True)
     counted_page = models.ForeignKey(
         'wagtailcore.Page',
@@ -157,8 +159,10 @@ class VisitCountRule(AbstractBaseRule):
 @python_2_unicode_compatible
 class QueryRule(AbstractBaseRule):
     """Query rule to segment users based on matching queries"""
-    parameter = models.SlugField(_("The query parameter to search for"), max_length=20)
+    parameter = models.SlugField(_("The query parameter to search for"),
-    value = models.SlugField(_("The value of the parameter to match"), max_length=20)
+                                 max_length=20)
+    value = models.SlugField(_("The value of the parameter to match"),
+                             max_length=20)
 
     panels = [
         FieldPanel('parameter'),
@@ -194,7 +198,8 @@ class Segment(ClusterableModel):
         ('enabled', 'Enabled'),
         ('disabled', 'Disabled'),
     )
-    status = models.CharField(max_length=20, choices=STATUS_CHOICES, default="enabled")
+    status = models.CharField(max_length=20, choices=STATUS_CHOICES,
+                              default="enabled")
 
     panels = [
         FieldPanel('name'),
@@ -240,6 +245,7 @@ def check_status_change(sender, instance, *args, **kwargs):
         if instance.status == "disabled":
             instance.disable_date = timezone.now()
 
+
 pre_save.connect(check_status_change, sender=Segment)
 
 
@@ -264,14 +270,17 @@ class AdminPersonalisablePageForm(WagtailAdminPageForm):
             }
 
             if page.is_segmented:
-                slug = "{}-{}".format(page.canonical_page.slug, segment.encoded_name())
+                slug = "{}-{}".format(
-                title = "{} ({})".format(page.canonical_page.title, segment.name)
+                    page.canonical_page.slug, segment.encoded_name())
+                title = "{} ({})".format(
+                    page.canonical_page.title, segment.name)
                 page.slug = slug
                 page.title = title
                 page.save()
                 return page
             else:
-                new_page = page.copy(update_attrs=update_attrs, copy_revisions=False)
+                new_page = page.copy(
+                    update_attrs=update_attrs, copy_revisions=False)
                 return new_page
 
         return page
@@ -291,8 +300,6 @@ class PersonalisablePage(Page):
     variation_panels = [
         MultiFieldPanel([
             FieldPanel('segment'),
-            # TOOD: Currently the user can only select pages of the 'PersonalisablePage' type.
-            # This is no longer acceptable as soon as a page inherits the 'PersonalisablePage'.
             PageChooserPanel('canonical_page', page_type=None),
         ])
     ]
@@ -300,7 +307,7 @@ class PersonalisablePage(Page):
     base_form_class = AdminPersonalisablePageForm
 
     def __str__(self):
-        return "{} ({})".format(self.title, self.segment)
+        return "{}".format(self.title)
 
     def get_variations(self, only_live=True):
         canonical_page = self.canonical_page or self
@@ -327,33 +334,6 @@ class PersonalisablePage(Page):
         )
         return variation_parent
 
-    def create_variation(self, segment, copy_fields=False, parent=None):
-        slug = "{}-{}".format(self.slug, segment.encoded_name())
-
-        if not parent:
-            parent = self.get_variation_parent(segment)
-
-        update_attrs = {
-            'title': self.title,
-            'slug': slug,
-            'segment': segment,
-            'live': False,
-            'canonical_page': self,
-        }
-
-        if copy_fields:
-            kwargs = {'update_attrs': update_attrs}
-            if parent != self.get_parent():
-                kwargs['to'] = parent
-
-            new_page = self.copy(**kwargs)
-        else:
-            model_class = self.content_type.model_class()
-            new_page = model_class(**update_attrs)
-            parent.add_child(instance=new_page)
-
-        return new_page
-
     @cached_property
     def has_variations(self):
         return self.variations.exists()
@@ -373,7 +353,8 @@ def get_edit_handler(cls):
     if cls.promote_panels:
         tabs.append(ObjectList(cls.promote_panels, heading=_("Promote")))
     if cls.settings_panels:
-        tabs.append(ObjectList(cls.settings_panels, heading=_("Settings"), classname='settings'))
+        tabs.append(ObjectList(cls.settings_panels, heading=_("Settings"),
+                               classname='settings'))
 
     edit_handler = TabbedInterface(tabs, base_form_class=cls.base_form_class)
     return edit_handler.bind_to_model(cls)

src/personalisation/views.py

@@ -1,7 +1,7 @@
 from __future__ import absolute_import, unicode_literals
 
 from django.http import HttpResponseRedirect
-from django.shortcuts import get_object_or_404, render, reverse
+from django.shortcuts import get_object_or_404, reverse
 from django.utils.translation import ugettext_lazy as _
 from wagtail.contrib.modeladmin.views import CreateView
 

src/personalisation/wagtail_hooks.py

@@ -2,14 +2,12 @@ import logging
 import time
 
 from django.conf.urls import include, url
-from django.core.urlresolvers import reverse
 from django.shortcuts import reverse
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext_lazy as _
 from wagtail.contrib.modeladmin.options import ModelAdmin, modeladmin_register
-from wagtail.contrib.modeladmin.views import IndexView
+from wagtail.wagtailadmin.widgets import (
-from wagtail.wagtailadmin.widgets import (Button, ButtonWithDropdownFromHook,
+    Button, ButtonWithDropdownFromHook)
-                                          PageListingButton)
 from wagtail.wagtailcore import hooks
 
 from personalisation import admin_urls
@@ -19,6 +17,7 @@ from personalisation.utils import impersonate_other_page
 
 logger = logging.getLogger()
 
+
 @hooks.register('register_admin_urls')
 def register_admin_urls():
     """Adds the administration urls for the personalisation apps."""
@@ -43,6 +42,7 @@ class SegmentModelAdmin(ModelAdmin):
 
 modeladmin_register(SegmentModelAdmin)
 
+
 @hooks.register('before_serve_page')
 def set_visit_count(page, request, serve_args, serve_kwargs):
     if 'visit_count' not in request.session:
@@ -67,7 +67,8 @@ def set_visit_count(page, request, serve_args, serve_kwargs):
                 request.session['visit_count'][index]['count'] = newcount
                 request.session.modified = True
             else:
-                # Counter doesn't exist. Create a new counter with count value 1.
+                # Counter doesn't exist.
+                # Create a new counter with count value 1.
                 create_new_counter(page, request)
     else:
         # No counters exist. Create a new counter with count value 1.
@@ -92,7 +93,8 @@ def segment_user(page, request, serve_args, serve_kwargs):
             _add_segment_to_user(segment, request)
 
     if request.session['segments']:
-        logger.info("User has been added to the following segments: {}".format(request.session['segments']))
+        logger.info("User has been added to the following segments: {}"
+                    .format(request.session['segments']))
 
         for seg in request.session['segments']:
             segment = Segment.objects.get(pk=seg['id'])
@@ -112,7 +114,8 @@ def _test_rules(rules, request):
                     rule.start_time,
                     rule.end_time))
             if result and rule.__class__.__name__ == "ReferralRule":
-                print("User segmented. Referral matches {}.".format(rule.regex_string))
+                print("User segmented. Referral matches {}.".format(
+                    rule.regex_string))
             if result and rule.__class__.__name__ == "VisitCountRule":
                 print("User segmented. Visited {} {} {} times.".format(
                     rule.counted_page,
@@ -144,13 +147,15 @@ def _add_segment_to_user(segment, request):
         }
         request.session['segments'].append(segdict)
 
+
 @hooks.register('before_serve_page')
 def serve_variation(page, request, serve_args, serve_kwargs):
     user_segments = []
 
     for segment in request.session['segments']:
         try:
-            user_segment = Segment.objects.get(pk=segment['id'], status='enabled')
+            user_segment = Segment.objects.get(pk=segment['id'],
+                                               status='enabled')
         except Segment.DoesNotExist:
             user_segment = None
         if user_segment:
@@ -166,25 +171,28 @@ def serve_variation(page, request, serve_args, serve_kwargs):
 
             return variation.serve(request, *serve_args, **serve_kwargs)
 
+
 def _check_for_variations(segments, page):
     for segment in segments:
         page_class = page.__class__
         if not any(item == PersonalisablePage for item in page_class.__bases__):
             page_class = PersonalisablePage
 
-        variation = page_class.objects.filter(canonical_page=page, segment=segment)
+        variation = page_class.objects.filter(
+            canonical_page=page, segment=segment)
 
         if variation:
             return variation
 
     return None
 
+
 @hooks.register('register_page_listing_buttons')
 def page_listing_variant_buttons(page, page_perms, is_parent=False):
     personalisable_page = PersonalisablePage.objects.filter(pk=page.pk)
     segments = Segment.objects.all()
 
-    if personalisable_page and len(segments) > 0 and not any(item.segment for item in personalisable_page):
+    if personalisable_page and len(segments) > 0 and not (any(item.segment for item in personalisable_page)):
         yield ButtonWithDropdownFromHook(
             _('Variants'),
             hook_name='register_page_listing_variant_buttons',
@@ -197,11 +205,11 @@ def page_listing_variant_buttons(page, page_perms, is_parent=False):
 @hooks.register('register_page_listing_variant_buttons')
 def page_listing_more_buttons(page, page_perms, is_parent=False):
     segments = Segment.objects.all()
-    current_page = PersonalisablePage.objects.filter(pk=page.pk)
+    available_segments = [item for item in segments if not PersonalisablePage.objects.filter(segment=item, pk=page.pk)]
-    available_segments = [item for item in segments if not PersonalisablePage.objects.filter(segment=item)]
 
     for segment in available_segments:
-        yield Button(segment.name, reverse('segment:copy_page', args=[page.id, segment.id]),
+        yield Button(segment.name,
+                     reverse('segment:copy_page', args=[page.id, segment.id]),
                      attrs={"title": _('Create this variant')})
 
 
@@ -214,6 +222,7 @@ class SegmentSummaryPanel(object):
         title = _("Segments")
         return mark_safe('<li class="icon icon-group"><a href="{}"><span>{}</span>{}</a></li>'.format(target_url, segment_count, title))
 
+
 @hooks.register('construct_homepage_summary_items')
 def add_segment_summary_panel(request, summary_items):
     return summary_items.append(SegmentSummaryPanel())