Merge pull request #146 from LabD/simplifications+optimisations

Handles both Python 2 & 3, and multiple optimisations & simplifications.

src/wagtail_personalisation/adapters.py

@@ -23,28 +23,22 @@ def __init__(self, request):
 
     def setup(self):
         """Prepare the adapter for segment storage."""
-        return None
 
     def get_segments(self):
         """Return the segments stored in the adapter storage."""
-        return None
 
     def get_segment_by_id(self):
         """Return a single segment stored in the adapter storage."""
-        return None
 
     def add(self):
         """Add a new segment to the adapter storage."""
-        return None
 
     def refresh(self):
         """Refresh the segments stored in the adapter storage."""
-        return None
 
     def _test_rules(self, rules, request, match_any=False):
         """Tests the provided rules to see if the request still belongs
         to a segment.
-
         :param rules: The rules to test for
         :type rules: list of wagtail_personalisation.rules
         :param request: The http request
@@ -53,20 +47,12 @@ def _test_rules(self, rules, request, match_any=False):
         :type match_any: bool
         :returns: A boolean indicating the segment matches the request
         :rtype: bool
-
         """
-        if len(rules) > 0:
-            for rule in rules:
-                result = rule.test_user(request)
-                if match_any:
-                    if result is True:
-                        return result
-
-                elif result is False:
-                    return False
-            if not match_any:
-                return True
-        return False
+        if not rules:
+            return False
+        if match_any:
+            return any(rule.test_user(request) for rule in rules)
+        return all(rule.test_user(request) for rule in rules)
 
     class Meta:
         abstract = True
@@ -95,7 +81,7 @@ def get_segments(self):
 
         segments = (
             Segment.objects
-            .filter(status=Segment.STATUS_ENABLED)
+            .enabled()
             .filter(persistent=True)
             .in_bulk(segment_ids))
 
@@ -134,8 +120,9 @@ def get_segment_by_id(self, segment_id):
         :rtype: wagtail_personalisation.models.Segment or None
 
         """
-        segments = self.get_segments()
-        return next((s for s in segments if s.pk == segment_id), None)
+        for segment in self.get_segments():
+            if segment.pk == segment_id:
+                return segment
 
     def add_page_visit(self, page):
         """Mark the page as visited by the user"""
@@ -179,20 +166,20 @@ def refresh(self):
         still apply to the requesting visitor.
 
         """
-        all_segments = Segment.objects.filter(status=Segment.STATUS_ENABLED)
+        enabled_segments = Segment.objects.enabled()
+        rule_models = AbstractBaseRule.get_descendant_models()
 
         current_segments = self.get_segments()
-        rules = AbstractBaseRule.__subclasses__()
 
         # Run tests on all remaining enabled segments to verify applicability.
         additional_segments = []
-        for segment in all_segments:
+        for segment in enabled_segments:
             segment_rules = []
-            for rule in rules:
-                segment_rules += rule.objects.filter(segment=segment)
+            for rule_model in rule_models:
+                segment_rules.extend(rule_model.objects.filter(segment=segment))
 
-            result = self._test_rules(
-                segment_rules, self.request, match_any=segment.match_any)
+            result = self._test_rules(segment_rules, self.request,
+                                      match_any=segment.match_any)
 
             if result:
                 additional_segments.append(segment)
@@ -209,8 +196,6 @@ def refresh(self):
 
 def get_segment_adapter(request):
     """Return the Segment Adapter for the given request"""
-    try:
-        return request.segment_adapter
-    except AttributeError:
+    if not hasattr(request, 'segment_adapter'):
         request.segment_adapter = SEGMENT_ADAPTER_CLASS(request)
-        return request.segment_adapter
+    return request.segment_adapter

src/wagtail_personalisation/blocks.py

@@ -8,8 +8,8 @@
 
 
 def list_segment_choices():
-    for segment in Segment.objects.all():
-        yield (segment.pk, segment.name)
+    for pk, name in Segment.objects.values_list('pk', 'name'):
+        yield pk, name
 
 
 class PersonalisedStructBlock(blocks.StructBlock):

src/wagtail_personalisation/models.py

@@ -61,9 +61,9 @@ def __init__(self, *args, **kwargs):
             ], heading="Segment"),
             MultiFieldPanel([
                 InlinePanel(
-                    "{}_related".format(rule._meta.db_table),
-                    label=rule.__str__,
-                ) for rule in AbstractBaseRule.__subclasses__()
+                    "{}_related".format(rule_model._meta.db_table),
+                    label=rule_model._meta.verbose_name,
+                ) for rule_model in AbstractBaseRule.__subclasses__()
             ], heading=_("Rules")),
         ]
 
@@ -82,12 +82,19 @@ def get_active_days(self):
 
     def get_rules(self):
         """Retrieve all rules in the segment."""
-        rules = AbstractBaseRule.__subclasses__()
         segment_rules = []
-        for rule in rules:
-            segment_rules += rule.objects.filter(segment=self)
+        for rule_model in AbstractBaseRule.get_descendant_models():
+            segment_rules.extend(
+                rule_model._default_manager.filter(segment=self))
         return segment_rules
 
+    def toggle(self, save=True):
+        self.status = (
+            self.STATUS_ENABLED if self.status == self.STATUS_DISABLED
+            else self.STATUS_DISABLED)
+        if save:
+            self.save()
+
 
 class PersonalisablePageMixin(models.Model):
     """The personalisable page model. Allows creation of variants with linked
@@ -103,7 +110,7 @@ class Meta:
         blank=True, null=True
     )
     segment = models.ForeignKey(
-        Segment, related_name='+', on_delete=models.PROTECT,
+        Segment, related_name='pages', on_delete=models.PROTECT,
         blank=True, null=True
     )
     is_segmented = models.BooleanField(default=False)
@@ -117,9 +124,6 @@ class Meta:
 
     base_form_class = AdminPersonalisablePageForm
 
-    def __str__(self):
-        return "{}".format(self.title)
-
     @cached_property
     def has_variations(self):
         """Return a boolean indicating whether or not the personalisable page
@@ -137,12 +141,20 @@ def is_canonical(self):
         """Return a boolean indicating whether or not the personalisable page
         is a canonical page.
 
-        :returns: A boolean indicating whether or not the personalisable page
+        :returns: A boolean indicating whether or not the personalisable
+        page
                   is a canonical page.
         :rtype: bool
 
         """
-        return not self.canonical_page and self.has_variations
+        return self.canonical_page_id is None
+
+    def get_unused_segments(self):
+        if not hasattr(self, '_unused_segments'):
+            self._unused_segments = (
+                Segment.objects.exclude(pages__canonical_page=self)
+                if self.is_canonical else Segment.objects.none())
+        return self._unused_segments
 
     def copy_for_segment(self, segment):
         slug = "{}-{}".format(self.slug, segment.encoded_name())