RFC 53: Swappable Page model

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)
+