diff --git a/text/053-swappable-page-model.md b/text/053-swappable-page-model.md new file mode 100644 index 00000000..85890294 --- /dev/null +++ b/text/053-swappable-page-model.md @@ -0,0 +1,232 @@ +# RFC 53: Swappable Page model + +* RFC: 53 +* Author: Sonny Baker +* Created: 2020-06-19 +* Last Modified: 2020-06-20 + +## Abstract + +This RFC proposes that Wagtail should give users the option to specify a custom `Page` model, +rather than inheriting the concrete one provided at `wagtail.core.models.Page`. + +## Why? + +Consider a Wagtail site with 3 page types - `BlogPage`, `PressReleasePage` and `CompetitionPage`. +Each of these pages can be assigned a category and marked as featured content: + +```python + +class Category(models.Model): + name = models.CharField( + max_length=255, + null=True, + blank=True + ) + +class PageBase(Page): + class Meta: + abstract = True + + category = models.ForeignKey( + 'foo.Category', + null=True, + blank=False, + on_delete=models.SET_NULL, + related_name='+' + ) + + is_featured = models.BooleanField( + verbose_name="Feature this page on the home page", + default=False + ) + +class BlogPage(PageBase): + pass + +class PressReleasePage(PageBase): + pass + +class CompetitionPage(PageBase): + pass + +``` + +On the homepage of the site, we'd like to show all the featured pages, and display the name +of the category they belong to. Currently, this query would fail: + +```python +featured = Page.objects.select_related('category', 'owner').filter(is_featured=True) +``` + +Because `is_featured` and `category` do not exist on the concrete `Page` model. The solution would +be to query each model individually, and then combine the querysets - a cumbersome and slow operation +that is difficult to scale. + +We may also want to show a list of the most recently published pages of multiple types and their category. +The `specific()` method exists on `PageQuerySet`, but it comes at the cost of extra database queries. +Additionally, [annotations are not currently possible when using `specific()`](https://github.com/wagtail/wagtail/issues/2340#issuecomment-496987132), +meaning the following query: + +```python +page_types = tuple([BlogPage, PressReleasePage, ]) +author_name = Concat('owner__first_name', models.Value(' '), 'owner__last_name') +news_pages = ( + Page.objects + .type(page_types) + .specific() + .annotate(author_name=author_name) + .order_by('-first_published_at') +) +``` + +Would return a list of pages as their specific classes and `news_pages.first().category` would work, +but `news_pages.first().owner_name` will throw an exception, even though the query ran without error. + +## Specification + +Django has undocumented support for [swappable models](https://code.djangoproject.com/ticket/19103), +most commonly used when specifying a custom `auth.User` model. +[A relatively simple third party library](https://github.com/wq/django-swappable-models) provides an +API for leveraging this feature. + +Wagtail core would provide an abstract version of the existing `Page` model +(with default attributes, methods, object manager, queryset etc), and helper methods to retrieve the +correct page model, falling back to a default: + +```python +class AbstractPage(MP_Node, index.Indexed, ClusterableModel, metaclass=PageBase): + + objects = PageManager() + + title = models.CharField( + verbose_name=_('title'), + max_length=255, + help_text=_("The page title as you'd like it to be seen by the public") + ) + + # other default fields... + + class Meta: + swappable = swapper.swappable_setting('wagtailcore', 'Page') + +class Page(AbstractPage): + + class Meta(AbstractPage.Meta): + pass + + +def get_page_model(): + """ + Get the site model from the ``WAGTAILCORE_PAGE_MODEL`` setting. + Defaults to the standard :class:`~wagtail.sites.models.Page` model + if no custom model is defined. + """ + Page = swapper.load_model("wagtailcore", "Page") + return Page + + +def get_page_model_string(): + """ + Get the dotted ``app.Model`` name for the page model as a string. + """ + return getattr(settings, 'WAGTAILCORE_PAGE_MODEL', 'wagtailcore.Page') + +``` + +This would be inherited by the user's custom `Page` class: + +```python +from wagtail.core.models import AbstractPage + +class Page(AbstractPage): + category = models.ForeignKey( + 'foo.Category', + null=True, + blank=False, + on_delete=models.SET_NULL, + related_name='+' + ) + + is_featured = models.BooleanField( + verbose_name="Feature this page on the home page", + default=False + ) + + class Meta(AbstractPage.Meta): + pass +``` + +And the project `settings.py` file will point to the dot notated string representation of that model: + +```python +WAGTAILCORE_PAGE_MODEL = 'foo.Page' +``` + +This would mean future references to the page model can be made like so: + +```python +class FooPage(Page): + related_page = models.ForeignKey( + get_page_model_string(), + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name='+' + ) +``` + +And previously complex multi-model Page queries become trivial: + +```python +author_name = Concat('owner__first_name', models.Value(' '), 'owner__last_name') +featured_foo_posts = ( + Page.objects + .live() + .filter(category__name='Foo', is_featured=True) + .annotate( + tag_count=Count('tags'), + author_name=author_name + ) + .order_by('-first_published_at') + .values('title', 'slug', 'tag_count', 'author_name') +) +``` + + +### Rationale +* Improves performance - fewer lookups as shared page attributes will be on the concrete `Page` model +* Reduces code complexity - filtering by custom attributes like `Page.objects.filter(language='fr')` +* Removing `.specific()` from queries will allow annotations of `Page` querysets +* Allows developers to control routing and URL generation +* Consistent with custom `Image` and `Document` (and [potentially `Site`](https://github.com/wagtail/wagtail/pull/5457)) models +* Simplifies managing future extensions with mixins - e.g Translation, Experiments, Personalisation + +### Caveats / Considerations +* This is a major change to Wagtail, and must be tested thoroughly. It would be be best soft-launched +as an experimental feature, to collect user feedback. +* Swappable models is an _intentionally undocumented_ (or "stealth alpha") feature - +[User models were the pilot program for this](https://code.djangoproject.com/ticket/19103). Though its +use in the `auth` library has shown it to be stable, relying on a private API is still a risk, and another solution may be preferable. +* Would mean updating core migrations. +* Page revisions will need to be adapted to work with generic foreign keys. +* Could present issues when adding (or removing) fields from the abstract Page base (ie. clashing attribute names). +* Potentially adds a layer of upfront complexity for new users. The existing custom `Image` and +`Document` model implementations are good examples of optional features available to those who need it, +but ignorable for common Wagtail installations. +* Could make future support queries harder to deal with. +* Difficult to switch to a custom model mid-project. Documentation should specify this in the same +manner as Django does with [custom user models](https://docs.djangoproject.com/en/3.0/topics/auth/customizing/#changing-to-a-custom-user-model-mid-project). + +## Open Questions +* How do we implement this in a way that is frictionless for new starters? +* What are the implications for third-party packages? +* What happens to required attributes like `title` if user has the option to override? Do we protect them somehow? +* What other consequences might there be for introducing this change? + +## References / further reading +* [Open PR for swappable `Site` model](https://github.com/wagtail/wagtail/pull/5457/) +* [Open feature request](https://github.com/wagtail/wagtail/issues/3282) +* [Open GitHub issue](https://github.com/wagtail/wagtail/issues/836) +* [Google groups discussion](https://groups.google.com/forum/#!msg/wagtail/4459qj1tNiU/D91COykMSmcJ) +