coderedcorp / wagtail-seo

Search engine and social media optimization for Wagtail.
Other
62 stars 18 forks source link

Allow use with non-Page models #43

Open dustinblanchard opened 2 years ago

dustinblanchard commented 2 years ago

Our application has a mix of Page and other types of models. Could we add an additional Mixin that is not dependent on Page that allows us to manually specify which fields to use?

DylannCordel commented 2 months ago

Code available here: #67

vsalvino commented 2 months ago

This is kind of a controversial change - it goes against wagtail's philosophy by using snippets as a "page". On one hand, I do not want to encourage this kind of practice as it creates unmaintainable code and goes against best practices. On the other hand, I understand that sometimes you are stuck in a situation (legacy code, etc.) where you need to make it work.

Can you all provide more context as to why you need/want this change? Having some case studies / use-cases would be really good to know about. Generally speaking we get zero feedback about how people are using this package, despite having thousands of pip installs!

I'll take a look at the pull request, but please note this may take some time as there are a couple other major refactors in progress that also need to be reviewed.

DylannCordel commented 2 months ago

Hi @vsalvino

Thanks for your reply. I understand your reluctance. I'll try to explain why we often use our own models for some « Detail » page and how we code it:


# extract from webu_wagtail/models/pages.py

from copy import deepcopy
from typing import Callable
from typing import Tuple
from typing import Type

from django.http import HttpRequest
from django.http import HttpResponse
from django.utils.module_loading import import_string
from django.utils.translation import gettext_lazy as _
from django.views import View

from wagtail.contrib.routable_page.models import RoutablePageMixin
from wagtail.contrib.routable_page.models import re_path
from wagtail.coreutils import camelcase_to_underscore
from wagtail.models import Page
from wagtail.models import PageBase

__all__ = ["BasePage"]

def build_serve_sub_view(name) -> Callable:
    """
    Attention, cette fonction doit rester à l'extérieur du __new__ de MultiViewPageBase
    sinon pour une raison obscure de construction dynamique de fonction et de référence,
    "name" vaudra toujours le dernier de la boucle for.
    Alors qu'en gardant un appel externe, le name est résolu à l'appel et reste bien
    inchangé lorsque "serve_sub_view" sera réellement appelé.
    """

    def serve_sub_view(self, request: HttpRequest, **kwargs) -> HttpResponse:
        return self._serve_specific_view(request, name, **kwargs)

    serve_sub_view.__name__ = f"serve_{name}_view"
    return serve_sub_view

class MultiViewPageBase(PageBase):
    """
    Fabrication automatique des méthodes et cie pour faciliter l'utilisation
    conjointe de page Wagtail et de vue django grace à la clase `ViewsMeta`
    ex d'utilisation:

        class BlogPage(BasePage):
            class ViewsMeta:
                urls = {
                    "article_list": {
                            "path": "",
                            "class": ArticleListView,
                            "init_kwargs": {"some_arg": "bar"},
                        }
                    ],
                    "article_detail": {
                        "path": r"^(?P<slug>[\w-]+)/$",
                        "class": "myapp.views.ArticleDetailView",
                    }
                }
    """

    def __new__(cls, cls_name:str, cls_bases, cls_attrs):
        tmp_views_meta = cls_attrs.pop("ViewsMeta", None)
        views_meta_kwargs = {
            "classes": {},
            "classes_initkwargs": {},
        }
        cls_attrs["_views_meta"] = type("ViewsMeta", (), views_meta_kwargs)
        sub_views = getattr(tmp_views_meta, "sub_views", None) if tmp_views_meta else None
        if not sub_views:
            return super().__new__(cls, cls_name, cls_bases, cls_attrs)

        for name, view_conf in sub_views.items():
            assert (
                name.isidentifier() and name == name.lower()
            ), f"`{name}` n'est pas un nom valide pour pouvoir générer une méthode"

            method_name = f"serve_{name}_view"
            if method_name in cls_attrs:
                continue

            pattern = view_conf.get("path", None)
            # assert issubclass(
            #     view_conf["class"], WagtailPageViewMixin
            # ), f"{view_conf["class"]} must extend WagtailPageViewMixin"
            views_meta_kwargs["classes"][name] = view_conf["class"]
            serve_sub_view = build_serve_sub_view(name)
            cls_attrs[method_name] = re_path(pattern=pattern, name=name)(serve_sub_view)
            _initkwargs = view_conf.get("classes_initkwargs")
            if _initkwargs:
                views_meta_kwargs["classes_initkwargs"][name] = _initkwargs
        return super().__new__(cls, cls_name, cls_bases, cls_attrs)

class BasePage(RoutablePageMixin, Page, metaclass=MultiViewPageBase):
    """
    Mixin permettant de lier une vue à une page wagtail pour que ce soit lui qui gère les URLs
    """

    class Meta:
        abstract = True

    def get_template(self, request: HttpRequest, *args, **kwargs) -> str:
        name = self.__class__.__name__
        if name.endswith("Page"):
            name = name[0:-4]
        name = camelcase_to_underscore(name)
        if request.headers.get("x-requested-with") == "XMLHttpRequest":
            name += ".ajax"
        return "%s/pages/%s.html" % (self._meta.app_label, name)

    def get_view_class(self, name:str="default") -> View:
        view_cls = self._views_meta.classes.get(name, None)
        if isinstance(view_cls, str):
            view_cls = import_string(view_cls)
        return view_cls

    def get_view_class_initkwargs(self, name:str="default", **kwargs) -> dict:
        base_kwargs = deepcopy(self._views_meta.classes_initkwargs.get(name, None) or {})
        base_kwargs.update(kwargs)
        return base_kwargs

    def get_view_func(self, name:str="default") -> Callable:
        view_class = self.get_view_class(name)
        return view_class.as_view(**(self.get_view_class_initkwargs(name)))

    def _serve_specific_view(self, request: HttpRequest, name:str, **kwargs) -> HttpResponse:
        view_class = self.get_view_class(name)
        if not view_class:
            return super().serve(request)
        request.is_preview = getattr(request, "is_preview", False)
        wagtail_context = self.get_context(request)
        view = self.get_view_func(name)
        return view(request, wagtail_context=wagtail_context, **kwargs)
# webu_wagtail/views/_bases.py

class WagtailPageViewMixin:
    """
    Permet d'utiliser une page wagtail comme une vue django
    """

    context_object_name = "page"

    def get_template_names(self):
        return [self.wagtail_context["page"].get_template(self.request)]

    def setup(self, request, *args, **kwargs):
        self.wagtail_context = kwargs.pop("wagtail_context", {})
        return super().setup(request, *args, **kwargs)

    def get_context_data(self, **context):
        context = {**self.wagtail_context, **context}
        context.pop("self", None)
        return super().get_context_data(**context)

Now, we can use our own "pure django" ListView and DetailView but keeping the strong of wagtail: display it under a page tree, allowing the website editor to move it if he wants.

Here is an exemple of models used on tracedirecte.com, a travel agency:

As you can see, only the tdfront module is the "standard cms" part of this project. But there are some other Models wich have Detail page (for eg, a Destination, an Agency, a TravelGuide...), but there are also some specific logic behind those models (which are not front related). Furthermore, we have custom logic of display, permissions, etc. and have a really small part of "cms" (for eg. a Destination has a StreamField for it's content).

That's why we only use wagtail Page for real front Pages without specific "backend" behaviors :). I hope it's not so fuzzy as explanation :-D

vsalvino commented 2 months ago

Thanks for the explanation, that is really good detail to know. I will also pass this on to the Wagtail core team :)