Move get_document_model to __init__ & add get_document_model_string

Avoids issues where models are not yet loaded and document model is needed.

- apply isort
- fix minor typos
- fixes #5614

CHANGELOG.txt

@@ -9,10 +9,12 @@ Changelog
  * Combine flake8 configurations (Sergey Fedoseev)
  * Improved diffing behavior for text fields (Aliosha Padovani)
  * Improve contrast of disabled inputs (Nick Smith)
+ * Added ``get_document_model_string`` function (WinterComes)
  * Fix: Rename documents listing column 'uploaded' to 'created' (LB (Ben Johnston))
  * Fix: Submenu items longer then the page height are no longer broken by the submenu footer (Igor van Spengen)
  * Fix: Unbundle the l18n library as it was bundled to avoid installation errors which have been resolved (Matt Westcott)
  * Fix: Prevent error when comparing pages that reference a model with a custom primary key (Fidel Ramos)
+ * Fix: Moved ``get_document_model`` location so it can be imported when Models are not yet loaded (WinterComes)
 
 2.7 LTS (06.11.2019)
 ~~~~~~~~~~~~~~~~~~~~

CONTRIBUTORS.rst

@@ -421,6 +421,7 @@ Contributors
 * Stefani Castellanos
 * Aliosha Padovani
 * Tom Readings
+* WinterComes
 
 Translators
 ===========

docs/advanced_topics/documents/custom_document_model.rst

@@ -1,3 +1,5 @@
+.. _custom_document_model:
+
 =====================
 Custom document model
 =====================
@@ -62,6 +64,8 @@ Then in your settings module:
 Referring to the document model
 ===============================
 
-.. module:: wagtail.documents.models
+.. module:: wagtail.documents
 
 .. autofunction:: get_document_model
+
+.. autofunction:: get_document_model_string

docs/releases/2.8.rst

@@ -18,6 +18,7 @@ Other features
  * Combine flake8 configurations (Sergey Fedoseev)
  * Improved diffing behavior for text fields (Aliosha Padovani)
  * Improve contrast of disabled inputs (Nick Smith)
+ * Added ``get_document_model_string`` function (WinterComes)
 
 
 Bug fixes
@@ -27,6 +28,7 @@ Bug fixes
  * Submenu items longer then the page height are no longer broken by the submenu footer (Igor van Spengen)
  * Unbundle the l18n library as it was bundled to avoid installation errors which have been resolved (Matt Westcott)
  * Prevent error when comparing pages that reference a model with a custom primary key (Fidel Ramos)
+ * Moved ``get_document_model`` location so it can be imported when Models are not yet loaded (WinterComes)
 
 
 Upgrade considerations
@@ -36,3 +38,9 @@ Removed support for Django 2.0
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Django 2.0 is no longer supported as of this release; please upgrade to Django 2.1 or above before upgrading Wagtail.
+
+
+``wagtail.documents.models.get_document_model`` has moved
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The ``get_document_model`` function should now be imported from ``wagtail.documents`` rather than ``wagtail.documents.models``. See :ref:`custom_document_model`.

wagtail/api/v2/signal_handlers.py

@@ -4,7 +4,7 @@ from django.urls import reverse
 from wagtail.contrib.frontend_cache.utils import purge_url_from_cache
 from wagtail.core.models import get_page_models
 from wagtail.core.signals import page_published, page_unpublished
-from wagtail.documents.models import get_document_model
+from wagtail.documents import get_document_model
 from wagtail.images import get_image_model
 
 from .utils import get_base_url

wagtail/api/v2/tests/test_documents.py

@@ -6,7 +6,7 @@ from django.test.utils import override_settings
 from django.urls import reverse
 
 from wagtail.api.v2 import signal_handlers
-from wagtail.documents.models import get_document_model
+from wagtail.documents import get_document_model
 
 
 class TestDocumentListing(TestCase):

wagtail/documents/__init__.py

@@ -1 +1,31 @@
+from django.conf import settings
+from django.core.exceptions import ImproperlyConfigured
+
 default_app_config = 'wagtail.documents.apps.WagtailDocsAppConfig'
+
+
+def get_document_model_string():
+    """
+    Get the dotted ``app.Model`` name for the document model as a string.
+    Useful for developers making Wagtail plugins that need to refer to the
+    document model, such as in foreign keys, but the model itself is not required.
+    """
+    return getattr(settings, 'WAGTAILDOCS_DOCUMENT_MODEL', 'wagtaildocs.Document')
+
+
+def get_document_model():
+    """
+    Get the document model from the ``WAGTAILDOCS_DOCUMENT_MODEL`` setting.
+    Defauts to the standard :class:`~wagtail.documents.models.Document` model
+    if no custom model is defined.
+    """
+    from django.apps import apps
+    model_string = get_document_model_string()
+    try:
+        return apps.get_model(model_string)
+    except ValueError:
+        raise ImproperlyConfigured("WAGTAILDOCS_DOCUMENT_MODEL must be of the form 'app_label.model_name'")
+    except LookupError:
+        raise ImproperlyConfigured(
+            "WAGTAILDOCS_DOCUMENT_MODEL refers to model '%s' that has not been installed" % model_string
+        )

wagtail/documents/api/v2/endpoints.py

@@ -1,7 +1,7 @@
 from wagtail.api.v2.endpoints import BaseAPIEndpoint
 from wagtail.api.v2.filters import FieldsFilter, OrderingFilter, SearchFilter
 
-from ...models import get_document_model
+from ... import get_document_model
 from .serializers import DocumentSerializer
 
 

wagtail/documents/blocks.py

@@ -7,7 +7,7 @@ from wagtail.core.blocks import ChooserBlock
 class DocumentChooserBlock(ChooserBlock):
     @cached_property
     def target_model(self):
-        from wagtail.documents.models import get_document_model
+        from wagtail.documents import get_document_model
         return get_document_model()
 
     @cached_property

wagtail/documents/models.py

@@ -1,9 +1,9 @@
 import hashlib
 import os.path
+import warnings
 from contextlib import contextmanager
 
 from django.conf import settings
-from django.core.exceptions import ImproperlyConfigured
 from django.db import models
 from django.dispatch import Signal
 from django.urls import reverse
@@ -14,12 +14,21 @@ from wagtail.admin.models import get_object_usage
 from wagtail.core.models import CollectionMember
 from wagtail.search import index
 from wagtail.search.queryset import SearchableQuerySetMixin
+from wagtail.utils.deprecation import RemovedInWagtail29Warning
 
 
 class DocumentQuerySet(SearchableQuerySetMixin, models.QuerySet):
     pass
 
 
+def get_document_model():
+    warnings.warn("wagtail.documents.models.get_document_model "
+                  "has been moved to wagtail.documents.get_document_model",
+                  RemovedInWagtail29Warning)
+    from wagtail.documents import get_document_model
+    return get_document_model()
+
+
 class AbstractDocument(CollectionMember, index.Indexed, models.Model):
     title = models.CharField(max_length=255, verbose_name=_('title'))
     file = models.FileField(upload_to='documents', verbose_name=_('file'))
@@ -163,29 +172,4 @@ class Document(AbstractDocument):
     )
 
 
-def get_document_model():
-    """
-    Get the document model from the ``WAGTAILDOCS_DOCUMENT_MODEL`` setting.
-    Defauts to the standard :class:`~wagtail.documents.models.Document` model
-    if no custom model is defined.
-    """
-    from django.conf import settings
-    from django.apps import apps
-
-    try:
-        app_label, model_name = settings.WAGTAILDOCS_DOCUMENT_MODEL.split('.')
-    except AttributeError:
-        return Document
-    except ValueError:
-        raise ImproperlyConfigured("WAGTAILDOCS_DOCUMENT_MODEL must be of the form 'app_label.model_name'")
-
-    document_model = apps.get_model(app_label, model_name)
-    if document_model is None:
-        raise ImproperlyConfigured(
-            "WAGTAILDOCS_DOCUMENT_MODEL refers to model '%s' that has not been installed" %
-            settings.WAGTAILDOCS_DOCUMENT_MODEL
-        )
-    return document_model
-
-
 document_served = Signal(providing_args=['request'])

wagtail/documents/permissions.py

@@ -1,5 +1,6 @@
 from wagtail.core.permission_policies.collections import CollectionOwnershipPermissionPolicy
-from wagtail.documents.models import Document, get_document_model
+from wagtail.documents import get_document_model
+from wagtail.documents.models import Document
 
 permission_policy = CollectionOwnershipPermissionPolicy(
     get_document_model(),

wagtail/documents/rich_text/__init__.py

@@ -2,11 +2,11 @@ from django.core.exceptions import ObjectDoesNotExist
 from django.utils.html import escape
 
 from wagtail.core.rich_text import LinkHandler
-from wagtail.documents.models import get_document_model
-
+from wagtail.documents import get_document_model
 
 # Front-end conversion
 
+
 class DocumentLinkHandler(LinkHandler):
     identifier = 'document'
 

wagtail/documents/rich_text/contentstate.py

@@ -1,11 +1,10 @@
 from draftjs_exporter.dom import DOM
-
 from wagtail.admin.rich_text.converters.html_to_contentstate import LinkElementHandler
-from wagtail.documents.models import get_document_model
-
+from wagtail.documents import get_document_model
 
 # draft.js / contentstate conversion
 
+
 def document_link_entity(props):
     """
     Helper to construct elements of the form

wagtail/documents/rich_text/editor_html.py

@@ -1,11 +1,11 @@
 from django.utils.html import escape
 
 from wagtail.admin.rich_text.converters import editor_html
-from wagtail.documents.models import get_document_model
-
+from wagtail.documents import get_document_model
 
 # hallo.js / editor-html conversion
 
+
 class DocumentLinkHandler:
     @staticmethod
     def get_db_attributes(tag):

wagtail/documents/signal_handlers.py

@@ -1,7 +1,7 @@
 from django.db import transaction
 from django.db.models.signals import post_delete
 
-from wagtail.documents.models import get_document_model
+from wagtail.documents import get_document_model
 
 
 def post_delete_file_cleanup(instance, **kwargs):

wagtail/documents/tests/test_admin_views.py

@@ -9,7 +9,7 @@ from django.test.utils import override_settings
 from django.urls import reverse
 
 from wagtail.core.models import Collection, GroupCollectionPermission, Page
-from wagtail.documents import models
+from wagtail.documents import get_document_model, models
 from wagtail.documents.tests.utils import get_test_document_file
 from wagtail.tests.testapp.models import CustomDocument, EventPage, EventPageRelatedLink
 from wagtail.tests.utils import WagtailTestUtils
@@ -434,7 +434,7 @@ class TestMultipleDocumentUploader(TestCase, WagtailTestUtils):
         self.login()
 
         # Create a document for running tests on
-        self.doc = models.get_document_model().objects.create(
+        self.doc = get_document_model().objects.create(
             title="Test document",
             file=get_test_document_file(),
         )
@@ -499,7 +499,7 @@ class TestMultipleDocumentUploader(TestCase, WagtailTestUtils):
         self.assertTrue(response.context['doc'].file_hash)
 
         # check that it is in the root collection
-        doc = models.get_document_model().objects.get(title='test.png')
+        doc = get_document_model().objects.get(title='test.png')
         root_collection = Collection.get_first_root_node()
         self.assertEqual(doc.collection, root_collection)
 
@@ -507,7 +507,7 @@ class TestMultipleDocumentUploader(TestCase, WagtailTestUtils):
         self.assertIn('form', response.context)
         self.assertEqual(
             set(response.context['form'].fields),
-            set(models.get_document_model().admin_form_fields) - {'file', 'collection'},
+            set(get_document_model().admin_form_fields) - {'file', 'collection'},
         )
         self.assertEqual(response.context['form'].initial['title'], 'test.png')
 
@@ -548,7 +548,7 @@ class TestMultipleDocumentUploader(TestCase, WagtailTestUtils):
         self.assertTrue(response.context['doc'].file_hash)
 
         # check that it is in the 'evil plans' collection
-        doc = models.get_document_model().objects.get(title='test.png')
+        doc = get_document_model().objects.get(title='test.png')
         root_collection = Collection.get_first_root_node()
         self.assertEqual(doc.collection, evil_plans_collection)
 
@@ -556,7 +556,7 @@ class TestMultipleDocumentUploader(TestCase, WagtailTestUtils):
         self.assertIn('form', response.context)
         self.assertEqual(
             set(response.context['form'].fields),
-            set(models.get_document_model().admin_form_fields) - {'file'} | {'collection'},
+            set(get_document_model().admin_form_fields) - {'file'} | {'collection'},
         )
         self.assertEqual(response.context['form'].initial['title'], 'test.png')
 
@@ -686,7 +686,7 @@ class TestMultipleDocumentUploader(TestCase, WagtailTestUtils):
         self.assertEqual(response['Content-Type'], 'application/json')
 
         # Make sure the document is deleted
-        self.assertFalse(models.get_document_model().objects.filter(id=self.doc.id).exists())
+        self.assertFalse(get_document_model().objects.filter(id=self.doc.id).exists())
 
         # Check JSON
         response_json = json.loads(response.content.decode())
@@ -723,7 +723,7 @@ class TestMultipleCustomDocumentUploaderNoCollection(TestMultipleCustomDocumentU
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        Document = models.get_document_model()
+        Document = get_document_model()
         fields = tuple(f for f in Document.admin_form_fields if f != 'collection')
         cls.__patcher = mock.patch.object(Document, 'admin_form_fields', fields)
         cls.__patcher.start()

wagtail/documents/tests/test_models.py

@@ -1,14 +1,20 @@
+import warnings
+
+from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Group, Permission
+from django.core.exceptions import ImproperlyConfigured
 from django.core.files.base import ContentFile
 from django.db import transaction
 from django.test import TestCase, TransactionTestCase
 from django.test.utils import override_settings
 
 from wagtail.core.models import Collection, GroupCollectionPermission
-from wagtail.documents import models, signal_handlers
-from wagtail.documents.models import get_document_model
+from wagtail.documents import get_document_model, get_document_model_string, models, signal_handlers
 from wagtail.images.tests.utils import get_test_image_file
+from wagtail.tests.testapp.models import CustomDocument
+from wagtail.tests.utils import WagtailTestUtils
+from wagtail.utils.deprecation import RemovedInWagtail29Warning
 
 
 class TestDocumentQuerySet(TestCase):
@@ -160,3 +166,50 @@ class TestFilesDeletedForCustomModels(TestFilesDeletedForDefaultModels):
     def test_document_model(self):
         cls = get_document_model()
         self.assertEqual('%s.%s' % (cls._meta.app_label, cls.__name__), 'tests.CustomDocument')
+
+
+class TestGetDocumentModel(WagtailTestUtils, TestCase):
+    @override_settings(WAGTAILDOCS_DOCUMENT_MODEL='tests.CustomDocument')
+    def test_custom_get_document_model(self):
+        """Test get_document_model with a custom document model"""
+        self.assertIs(get_document_model(), CustomDocument)
+
+    @override_settings(WAGTAILDOCS_DOCUMENT_MODEL='tests.CustomDocument')
+    def test_custom_get_document_model_string(self):
+        """Test get_document_model_string with a custom document model"""
+        self.assertEqual(get_document_model_string(), 'tests.CustomDocument')
+
+    @override_settings()
+    def test_standard_get_document_model(self):
+        """Test get_document_model with no WAGTAILDOCS_DOCUMENT_MODEL"""
+        del settings.WAGTAILDOCS_DOCUMENT_MODEL
+        from wagtail.documents.models import Document
+        self.assertIs(get_document_model(), Document)
+
+    @override_settings()
+    def test_standard_get_document_model_string(self):
+        """Test get_document_model_string with no WAGTAILDOCS_DOCUMENT_MODEL"""
+        del settings.WAGTAILDOCS_DOCUMENT_MODEL
+        self.assertEqual(get_document_model_string(), 'wagtaildocs.Document')
+
+    @override_settings(WAGTAILDOCS_DOCUMENT_MODEL='tests.UnknownModel')
+    def test_unknown_get_document_model(self):
+        """Test get_document_model with an unknown model"""
+        with self.assertRaises(ImproperlyConfigured):
+            get_document_model()
+
+    @override_settings(WAGTAILDOCS_DOCUMENT_MODEL='invalid-string')
+    def test_invalid_get_document_model(self):
+        """Test get_document_model with an invalid model string"""
+        with self.assertRaises(ImproperlyConfigured):
+            get_document_model()
+
+    def test_deprecated_get_document_model(self):
+        from wagtail.documents.models import Document
+        from wagtail.documents.models import get_document_model
+        with warnings.catch_warnings(record=True) as ws:
+            warnings.simplefilter('always')
+
+            self.assertIs(Document, get_document_model())
+            self.assertEqual(len(ws), 1)
+            self.assertIs(ws[0].category, RemovedInWagtail29Warning)

wagtail/documents/views/chooser.py

@@ -8,8 +8,8 @@ from wagtail.admin.forms.search import SearchForm
 from wagtail.admin.modal_workflow import render_modal_workflow
 from wagtail.core import hooks
 from wagtail.core.models import Collection
+from wagtail.documents import get_document_model
 from wagtail.documents.forms import get_document_form
-from wagtail.documents.models import get_document_model
 from wagtail.documents.permissions import permission_policy
 from wagtail.search import index as search_index
 

wagtail/documents/views/documents.py

@@ -11,8 +11,8 @@ from wagtail.admin.auth import PermissionPolicyChecker, permission_denied
 from wagtail.admin.forms.search import SearchForm
 from wagtail.admin.models import popular_tags_for_model
 from wagtail.core.models import Collection
+from wagtail.documents import get_document_model
 from wagtail.documents.forms import get_document_form
-from wagtail.documents.models import get_document_model
 from wagtail.documents.permissions import permission_policy
 from wagtail.search import index as search_index
 

wagtail/documents/views/multiple.py

@@ -10,8 +10,8 @@ from wagtail.admin.auth import PermissionPolicyChecker
 from wagtail.core.models import Collection
 from wagtail.search.backends import get_search_backends
 
+from .. import get_document_model
 from ..forms import get_document_form, get_document_multi_form
-from ..models import get_document_model
 from ..permissions import permission_policy
 
 permission_checker = PermissionPolicyChecker(permission_policy)

wagtail/documents/views/serve.py

@@ -9,7 +9,8 @@ from django.urls import reverse
 from wagtail.core import hooks
 from wagtail.core.forms import PasswordViewRestrictionForm
 from wagtail.core.models import CollectionViewRestriction
-from wagtail.documents.models import document_served, get_document_model
+from wagtail.documents import get_document_model
+from wagtail.documents.models import document_served
 from wagtail.utils import sendfile_streaming_backend
 from wagtail.utils.sendfile import sendfile
 

wagtail/documents/wagtail_hooks.py

@@ -16,10 +16,9 @@ from wagtail.admin.staticfiles import versioned_static
 from wagtail.core import hooks
 from wagtail.core.models import BaseViewRestriction
 from wagtail.core.wagtail_hooks import require_wagtail_login
-from wagtail.documents import admin_urls
+from wagtail.documents import admin_urls, get_document_model
 from wagtail.documents.api.admin.endpoints import DocumentsAdminAPIEndpoint
 from wagtail.documents.forms import GroupDocumentPermissionFormSet
-from wagtail.documents.models import get_document_model
 from wagtail.documents.permissions import permission_policy
 from wagtail.documents.rich_text import DocumentLinkHandler
 from wagtail.documents.rich_text.contentstate import ContentstateDocumentLinkConversionRule

wagtail/documents/widgets.py

@@ -6,7 +6,7 @@ from django.utils.translation import ugettext_lazy as _
 
 from wagtail.admin.staticfiles import versioned_static
 from wagtail.admin.widgets import AdminChooser
-from wagtail.documents.models import get_document_model
+from wagtail.documents import get_document_model
 
 
 class AdminDocumentChooser(AdminChooser):