wagtail / django-modelcluster

Django extension to allow working with 'clusters' of models as a single unit, independently of the database
BSD 3-Clause "New" or "Revised" License
485 stars 66 forks source link

Unhelpful messaging when model attribute name shadows child relation #178

Open jams2 opened 1 year ago

jams2 commented 1 year ago

Given the following definitions:

class MyPage(Page):
    @cached_property
    def frobnicators(self):
        return 42

class Frobnicator(Orderable):
    page = ParentalKey(MyPage, related_name="frobnicators", on_delete=models.CASCADE)

An error like the following (model names don't match the above, I took this from a client project) is shown when attempting to save an instance of MyPage:

Traceback (most recent call last):
  File "/home/app/.cache/pypoetry/virtualenvs/my_app-9TtSrW0h-py3.11/lib/python3.11/site-packages/wagtail/test/utils/page_tests.py", line 28, in setUpClass
    super().setUpClass()
  File "/home/app/.cache/pypoetry/virtualenvs/my_app-9TtSrW0h-py3.11/lib/python3.11/site-packages/django/test/testcases.py", line 1466, in setUpClass
    cls.setUpTestData()
  File "/app/my_app/collections/tests/test_pages.py", line 107, in setUpTestData
    self.explorer_index_page.add_child(instance=self.highlight_gallery_page)
  File "/home/app/.cache/pypoetry/virtualenvs/my_app-9TtSrW0h-py3.11/lib/python3.11/site-packages/treebeard/mp_tree.py", line 1091, in add_child
    return MP_AddChildHandler(self, **kwargs).process()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/app/.cache/pypoetry/virtualenvs/my_app-9TtSrW0h-py3.11/lib/python3.11/site-packages/treebeard/mp_tree.py", line 393, in process
    newobj.save()
  File "/usr/local/lib/python3.11/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/home/app/.cache/pypoetry/virtualenvs/my_app-9TtSrW0h-py3.11/lib/python3.11/site-packages/wagtail/models/__init__.py", line 1464, in save
    result = super().save(**kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/app/.cache/pypoetry/virtualenvs/my_app-9TtSrW0h-py3.11/lib/python3.11/site-packages/modelcluster/models.py", line 205, in save
    getattr(self, relation).commit()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'QuerySet' object has no attribute 'commit'

On a large codebase, and particularly for developers less familiar with the inner workings of modelcluster (and models in general), this can be quite confusing. Can we detect the case that an attribute on the parent model shadows a relation name and raise a more descriptive error?