7

Refactor personalisable pages

Instead of working with django model mixins it now uses a separate
model entity to keep track of the personalized pages (metadata).

The current downside of this approach is that the segment of an existing
variant is no longer easily adjustable for now.
This commit is contained in:
Michael van Tellingen
2017-05-31 21:31:32 +02:00
parent c2735807b4
commit 7076973fc8
9 changed files with 157 additions and 70 deletions

View File

@ -1,9 +1,10 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.1 on 2017-05-31 10:20
# Generated by Django 1.11.1 on 2017-05-31 16:59
from __future__ import unicode_literals
from django.db import migrations, models
import django.db.models.deletion
import modelcluster.fields
class Migration(migrations.Migration):
@ -11,7 +12,8 @@ class Migration(migrations.Migration):
initial = True
dependencies = [
('wagtail_personalisation', '0009_auto_20170531_0428'),
('wagtailcore', '0033_remove_golive_expiry_help_text'),
('wagtail_personalisation', '0011_personalisablepagemetadata'),
]
operations = [
@ -19,9 +21,6 @@ class Migration(migrations.Migration):
name='HomePage',
fields=[
('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')),
('is_segmented', models.BooleanField(default=False)),
('canonical_page', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='variations', to='home.HomePage')),
('segment', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='segments', to='wagtail_personalisation.Segment')),
],
options={
'abstract': False,

View File

@ -5,5 +5,5 @@ from wagtail.wagtailcore.models import Page
from wagtail_personalisation.models import PersonalisablePageMixin
class HomePage(Page, PersonalisablePageMixin):
class HomePage(PersonalisablePageMixin, Page):
pass

View File

@ -0,0 +1,30 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.1 on 2017-05-31 14:28
from __future__ import unicode_literals
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
('wagtailcore', '0033_remove_golive_expiry_help_text'),
('wagtail_personalisation', '0010_auto_20170531_1101'),
]
operations = [
migrations.CreateModel(
name='PersonalisablePageMetadata',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('is_segmented', models.BooleanField(default=False)),
('canonical_page', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='personalisable_canonical_metadata', to='wagtailcore.Page')),
('segment', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='page_metadata', to='wagtail_personalisation.Segment')),
('variant', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='_personalisable_page_metadata', to='wagtailcore.Page')),
],
options={
'abstract': False,
},
),
]

View File

@ -1,11 +1,13 @@
from __future__ import absolute_import, unicode_literals
from django.db import models
from django.db import models, transaction
from django.template.defaultfilters import slugify
from django.utils.encoding import python_2_unicode_compatible
from django.utils.functional import cached_property
from django.utils.translation import ugettext_lazy as _
from modelcluster.fields import ParentalKey
from modelcluster.models import ClusterableModel
from wagtail.wagtailcore.models import Page
from wagtail.utils.decorators import cached_classmethod
from wagtail.wagtailadmin.edit_handlers import (
FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel, ObjectList,
@ -96,31 +98,24 @@ class Segment(ClusterableModel):
self.save()
class PersonalisablePageMixin(models.Model):
class PersonalisablePageMetadata(ClusterableModel):
"""The personalisable page model. Allows creation of variants with linked
segments.
"""
class Meta:
abstract = True
canonical_page = models.ForeignKey(
'self', related_name='variations', on_delete=models.SET_NULL,
Page, related_name='personalisable_canonical_metadata',
on_delete=models.SET_NULL,
blank=True, null=True
)
segment = models.ForeignKey(
Segment, related_name='pages', on_delete=models.PROTECT,
blank=True, null=True
)
is_segmented = models.BooleanField(default=False)
variation_panels = [
MultiFieldPanel([
FieldPanel('segment'),
PageChooserPanel('canonical_page', page_type=None),
])
]
variant = models.OneToOneField(
Page, related_name='_personalisable_page_metadata')
segment = models.ForeignKey(
Segment, related_name='page_metadata', null=True, blank=True)
is_segmented = models.BooleanField(default=False)
base_form_class = AdminPersonalisablePageForm
@ -136,6 +131,14 @@ class PersonalisablePageMixin(models.Model):
"""
return self.variations.exists()
@cached_property
def variations(self):
return (
PersonalisablePageMetadata.objects
.filter(canonical_page_id=self.canonical_page_id)
.exclude(variant_id=self.variant_id)
.exclude(variant_id=self.canonical_page_id))
@cached_property
def is_canonical(self):
"""Return a boolean indicating whether or not the personalisable page
@ -147,53 +150,56 @@ class PersonalisablePageMixin(models.Model):
:rtype: bool
"""
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
return self.canonical_page_id == self.variant_id
def copy_for_segment(self, segment):
slug = "{}-{}".format(self.slug, segment.encoded_name())
title = "{} ({})".format(self.title, segment.name)
page = self.canonical_page
slug = "{}-{}".format(page.slug, segment.encoded_name())
title = "{} ({})".format(page.title, segment.name)
update_attrs = {
'title': title,
'slug': slug,
'segment': segment,
'live': False,
'canonical_page': self,
'is_segmented': True,
}
return self.copy(update_attrs=update_attrs, copy_revisions=False)
with transaction.atomic():
new_page = self.canonical_page.copy(
update_attrs=update_attrs, copy_revisions=False)
PersonalisablePageMetadata.objects.create(
canonical_page=page,
variant=new_page,
segment=segment,
is_segmented=True)
return new_page
def variants_for_segments(self, segments):
return self.__class__.objects.filter(
canonical_page=self, segment__in=segments)
return (
self.__class__.objects
.filter(
canonical_page_id=self.canonical_page_id,
segment__in=segments))
def get_unused_segments(self):
if self.is_canonical:
return (
Segment.objects
.exclude(page_metadata__canonical_page_id=self.canonical_page_id))
return Segment.objects.none()
@cached_classmethod
def get_edit_handler(cls):
"""Add additional edit handlers to pages that are allowed to have
variations.
class PersonalisablePageMixin(object):
"""The personalisable page model. Allows creation of variants with linked
segments.
"""
tabs = []
if cls.content_panels:
tabs.append(ObjectList(cls.content_panels, heading=_("Content")))
if cls.variation_panels:
tabs.append(ObjectList(cls.variation_panels, heading=_("Variations")))
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'))
edit_handler = TabbedInterface(tabs, base_form_class=cls.base_form_class)
return edit_handler.bind_to_model(cls)
PersonalisablePageMixin.get_edit_handler = get_edit_handler
@cached_property
def personalisable_metadata(self):
try:
metadata = self._personalisable_page_metadata
except AttributeError:
metadata = PersonalisablePageMetadata.objects.create(
canonical_page=self, variant=self)
return metadata

View File

@ -122,11 +122,13 @@ def copy_page_view(request, page_id, segment_id):
if request.user.has_perm('wagtailadmin.access_admin'):
segment = get_object_or_404(Segment, pk=segment_id)
page = get_object_or_404(Page, pk=page_id).specific
variants = page.variants_for_segments([segment])
metadata = page.personalisable_metadata
variants = metadata.variants_for_segments([segment])
if variants.exists():
variant = variants.first()
else:
variant = page.copy_for_segment(segment)
variant = metadata.copy_for_segment(segment)
edit_url = reverse('wagtailadmin_pages:edit', args=[variant.id])
return HttpResponseRedirect(edit_url)

View File

@ -10,7 +10,7 @@ from wagtail.wagtailadmin.site_summary import SummaryItem
from wagtail.wagtailadmin.widgets import Button, ButtonWithDropdownFromHook
from wagtail.wagtailcore import hooks
from wagtail_personalisation import admin_urls
from wagtail_personalisation import admin_urls, models
from wagtail_personalisation.adapters import get_segment_adapter
from wagtail_personalisation.models import PersonalisablePageMixin, Segment
from wagtail_personalisation.utils import impersonate_other_page
@ -79,7 +79,8 @@ def serve_variation(page, request, serve_args, serve_kwargs):
user_segments = adapter.get_segments()
if user_segments:
variations = page.variants_for_segments(user_segments)
metadata = page.personalisable_metadata
variations = metadata.variants_for_segments(user_segments)
if variations:
variation = variations.first()
impersonate_other_page(variation, page)
@ -92,7 +93,11 @@ def page_listing_variant_buttons(page, page_perms, is_parent=False):
the page (if any) and a 'Create a new variant' button.
"""
if isinstance(page, PersonalisablePageMixin) and page.get_unused_segments():
if not isinstance(page, models.PersonalisablePageMixin):
return
metadata = page.personalisable_metadata
if metadata.is_canonical and metadata.get_unused_segments():
yield ButtonWithDropdownFromHook(
_('Variants'),
hook_name='register_page_listing_variant_buttons',
@ -109,7 +114,11 @@ def page_listing_more_buttons(page, page_perms, is_parent=False):
create a new variant for the selected segment.
"""
for segment in page.get_unused_segments():
if not isinstance(page, models.PersonalisablePageMixin):
return
metadata = page.personalisable_metadata
for segment in metadata.get_unused_segments():
yield Button(segment.name,
reverse('segment:copy_page', args=[page.pk, segment.pk]),
attrs={"title": _('Create this variant')},

View File

@ -23,7 +23,7 @@ def site():
def segmented_page(site):
page = HomePageFactory(parent=site.root_page)
segment = SegmentFactory()
return page.copy_for_segment(segment)
return page.personalisable_metadata.copy_for_segment(segment)
@pytest.fixture()

View File

@ -0,0 +1,39 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.1 on 2017-05-31 14:28
from __future__ import unicode_literals
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('pages', '0002_auto_20170531_0915'),
]
operations = [
migrations.RemoveField(
model_name='homepage',
name='canonical_page',
),
migrations.RemoveField(
model_name='homepage',
name='is_segmented',
),
migrations.RemoveField(
model_name='homepage',
name='segment',
),
migrations.RemoveField(
model_name='specialpage',
name='canonical_page',
),
migrations.RemoveField(
model_name='specialpage',
name='is_segmented',
),
migrations.RemoveField(
model_name='specialpage',
name='segment',
),
]

View File

@ -63,7 +63,9 @@ class TestPage(object):
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
assert not segmented_page.personalisable_metadata.is_canonical
assert not segmented_page.personalisable_metadata.has_variations
canonical = segmented_page.personalisable_metadata.canonical_page
assert canonical.personalisable_metadata.is_canonical
assert canonical.personalisable_metadata.has_variations