From 1829ed3dbe7c7c5be38527146056746167d8ac8e Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 1 Sep 2022 15:04:10 +0100 Subject: [PATCH] Avoid importing document model class from wagtail.documents.permissions and wagtail.documents.views.chooser Fixes #9118. Permission policies can now be initialised by passing a model string rather than a class; wagtail.admin.widgets.chooser.BaseChooser had this capability already. Between these, we can adjust wagtail.documents.views.chooser so that no models need to be imported at module load time. As a result, definitions that depend on this module (such as DocumentChooserBlock) can now be included in the same models file as a custom document model, without causing a circular import. --- CHANGELOG.txt | 1 + docs/releases/4.0.1.md | 1 + wagtail/documents/permissions.py | 7 ++-- wagtail/documents/views/chooser.py | 6 ++-- wagtail/permission_policies/base.py | 39 ++++++++++++++++++---- wagtail/permission_policies/collections.py | 7 ++-- 6 files changed, 47 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 23285592f0..d7af164aad 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -22,6 +22,7 @@ Changelog * Fix: On the Locked Pages report, limit the "locked by" filter to just users who have locked pages (Stefan Hammer) * Fix: Prevent JavaScript error when using StreamField on views without commenting support, such as snippets (Jacob Topp-Mugglestone) * Fix: Modify base template for new projects so that links opened from the preview panel open in a new window (Sage Abdullah) + * Fix: Prevent circular import error between custom document models and document chooser blocks (Matt Westcott) 4.0 (31.08.2022) diff --git a/docs/releases/4.0.1.md b/docs/releases/4.0.1.md index d2f3d9fc73..aaedf30ef4 100644 --- a/docs/releases/4.0.1.md +++ b/docs/releases/4.0.1.md @@ -16,3 +16,4 @@ depth: 1 * On the Locked Pages report, limit the "locked by" filter to just users who have locked pages (Stefan Hammer) * Prevent JavaScript error when using StreamField on views without commenting support, such as snippets (Jacob Topp-Mugglestone) * Modify base template for new projects so that links opened from the preview panel open in a new window (Sage Abdullah) + * Prevent circular import error between custom document models and document chooser blocks (Matt Westcott) diff --git a/wagtail/documents/permissions.py b/wagtail/documents/permissions.py index 6b29934707..2d321c480c 100644 --- a/wagtail/documents/permissions.py +++ b/wagtail/documents/permissions.py @@ -1,7 +1,8 @@ -from wagtail.documents import get_document_model -from wagtail.documents.models import Document +from wagtail.documents import get_document_model_string from wagtail.permission_policies.collections import CollectionOwnershipPermissionPolicy permission_policy = CollectionOwnershipPermissionPolicy( - get_document_model(), auth_model=Document, owner_field_name="uploaded_by_user" + get_document_model_string(), + auth_model="wagtaildocs.Document", + owner_field_name="uploaded_by_user", ) diff --git a/wagtail/documents/views/chooser.py b/wagtail/documents/views/chooser.py index 5bef3a1f33..249dc46466 100644 --- a/wagtail/documents/views/chooser.py +++ b/wagtail/documents/views/chooser.py @@ -21,7 +21,7 @@ from wagtail.admin.views.generic.chooser import ( from wagtail.admin.viewsets.chooser import ChooserViewSet from wagtail.admin.widgets import BaseChooser, BaseChooserAdapter from wagtail.blocks import ChooserBlock -from wagtail.documents import get_document_model +from wagtail.documents import get_document_model, get_document_model_string from wagtail.documents.permissions import permission_policy @@ -153,7 +153,7 @@ class BaseAdminDocumentChooser(BaseChooser): def __init__(self, **kwargs): super().__init__(**kwargs) - self.model = get_document_model() + self.model = get_document_model_string() def render_js_init(self, id_, name, value_data): return "new DocumentChooser({0});".format(json.dumps(id_)) @@ -206,6 +206,6 @@ class DocumentChooserViewSet(ChooserViewSet): viewset = DocumentChooserViewSet( "wagtaildocs_chooser", - model=get_document_model(), + model=get_document_model_string(), url_prefix="documents/chooser", ) diff --git a/wagtail/permission_policies/base.py b/wagtail/permission_policies/base.py index 94513b9fdd..4c9c8d77e8 100644 --- a/wagtail/permission_policies/base.py +++ b/wagtail/permission_policies/base.py @@ -5,6 +5,8 @@ from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured from django.db.models import Q from django.utils.functional import cached_property +from wagtail.coreutils import resolve_model_string + class BasePermissionPolicy: """ @@ -25,7 +27,19 @@ class BasePermissionPolicy: """ def __init__(self, model): - self.model = model + self._model_or_name = model + + @cached_property + def model(self): + model = resolve_model_string(self._model_or_name) + self.check_model(model) + return model + + def check_model(self, model): + # a hook that is called at the point that the model argument (which may be a string + # rather than a model class) is resolved to a model class, for subclasses to perform + # any necessary validation checks on that model class + pass # Basic user permission tests. Most policies are expected to override these, # since the default implementation is to query the set of permitted users @@ -175,9 +189,19 @@ class BaseDjangoAuthPermissionPolicy(BasePermissionPolicy): # records might use a custom User model but will typically still refer to the # permission records for auth.user. super().__init__(model) - self.auth_model = auth_model or self.model - self.app_label = self.auth_model._meta.app_label - self.model_name = self.auth_model._meta.model_name + self._auth_model_or_name = auth_model or model + + @cached_property + def auth_model(self): + return resolve_model_string(self._auth_model_or_name) + + @cached_property + def app_label(self): + return self.auth_model._meta.app_label + + @cached_property + def model_name(self): + return self.auth_model._meta.model_name @cached_property def _content_type(self): @@ -255,14 +279,17 @@ class OwnershipPermissionPolicy(BaseDjangoAuthPermissionPolicy): super().__init__(model, auth_model=auth_model) self.owner_field_name = owner_field_name + def check_model(self, model): + super().check_model(model) + # make sure owner_field_name is a field that exists on the model try: - self.model._meta.get_field(self.owner_field_name) + model._meta.get_field(self.owner_field_name) except FieldDoesNotExist: raise ImproperlyConfigured( "%s has no field named '%s'. To use this model with OwnershipPermissionPolicy, " "you must specify a valid field name as owner_field_name." - % (self.model, self.owner_field_name) + % (model, self.owner_field_name) ) def user_has_permission(self, user, action): diff --git a/wagtail/permission_policies/collections.py b/wagtail/permission_policies/collections.py index 05bd89a0b3..d47598cf67 100644 --- a/wagtail/permission_policies/collections.py +++ b/wagtail/permission_policies/collections.py @@ -218,14 +218,17 @@ class CollectionOwnershipPermissionPolicy( super().__init__(model, auth_model=auth_model) self.owner_field_name = owner_field_name + def check_model(self, model): + super().check_model(model) + # make sure owner_field_name is a field that exists on the model try: - self.model._meta.get_field(self.owner_field_name) + model._meta.get_field(self.owner_field_name) except FieldDoesNotExist: raise ImproperlyConfigured( "%s has no field named '%s'. To use this model with " "CollectionOwnershipPermissionPolicy, you must specify a valid field name as " - "owner_field_name." % (self.model, self.owner_field_name) + "owner_field_name." % (model, self.owner_field_name) ) def user_has_permission(self, user, action):