django-modelcluster icon indicating copy to clipboard operation
django-modelcluster copied to clipboard

Copy fails for a Wagtail page with no tags

Open higs4281 opened this issue 4 years ago • 7 comments

We've been getting server errors when pages without tags are copied.

I think I've traced the problem to contrib/taggit.py, where the passed tags value is a tuple consisting of an empty FakeQuerySet instance, which raises a ValueError such as this:

ValueError: Cannot add [] (<class 'modelcluster.queryset.FakeQuerySet'>). Expected <class 'django.db.models.base.ModelBase'> or str.

I can get the process to complete without error by adding this check to _ClusterTaggableManager's add method:

    def add(self, *tags):
        if not tags[0]:
            return
        if TAGGIT_VERSION >= (1, 3, 0):
            tag_objs = self._to_tag_model_instances(tags, {})
        else:
            tag_objs = self._to_tag_model_instances(tags)

If this is acceptable, and isn't a known issue or something that would be better addressed elsewhere, I'd be happy to open a PR.

We are running these versions:

  • wagtail==2.10.1
  • Django==2.2.16
  • Python 3.6.3

higs4281 avatar Sep 14 '20 18:09 higs4281

Addendum: They copy action does result in a copied page despite the server errors, but its new title doesn't show in the Wagtail admin.

higs4281 avatar Sep 14 '20 18:09 higs4281

Hi @higs4281, Please can you provide steps to reproduce this error independently? As far as I can see, passing a queryset to a TaggableManager's add method is not valid, so django-taggit's _to_tag_model_instances method is correctly rejecting it. If this is happening from within Wagtail code then that would be a bug to be investigated and fixed within Wagtail - however, when I test against the bakerydemo project (creating a blog post with no tags, then copying it) the copy succeeds with no error, so I suspect this is something specific to your project.

gasman avatar Sep 14 '20 19:09 gasman

I need to do more digging, because I now find that I get the error on pages with tags, so this does seem like an implementation issue. Thanks for the quick response. I can close this until I know more.

higs4281 avatar Sep 14 '20 19:09 higs4281

Hi,

I think I've traced the problem to contrib/taggit.py, where the passed tags value is a tuple consisting of an empty FakeQuerySet instance, which raises a ValueError such as this:

I am facing exactly the same issue, with or without tags on specific page models. Here follow a textual stack trace and the visual one with more details.

Internal Server Error: /admin/pages/27/copy/
Traceback (most recent call last):
  File "venv/lib/python3.9/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "venv/lib/python3.9/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "venv/lib/python3.9/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "venv/lib/python3.9/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/wagtail/admin/urls/__init__.py", line 127, in wrapper
    return view_func(request, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/wagtail/admin/auth.py", line 193, in decorated_view
    return view_func(request, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/wagtail/admin/auth.py", line 60, in wrapped_view_func
    return view_func(request, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/wagtail/admin/views/pages/copy.py", line 64, in copy
    new_page = page.specific.copy(
  File "venv/lib/python3.9/site-packages/wagtail/core/models.py", line 2053, in copy
    _copy_m2m_relations(specific_self, page_copy, exclude_fields=exclude_fields, update_attrs=base_update_attrs)
  File "venv/lib/python3.9/site-packages/wagtail/core/models.py", line 140, in _copy_m2m_relations
    getattr(target, field.name).set(value)
  File "venv/lib/python3.9/site-packages/taggit/utils.py", line 124, in inner
    return func(self, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/modelcluster/contrib/taggit.py", line 78, in set
    return super(_ClusterTaggableManager, self).set(*tags, clear=True)
  File "venv/lib/python3.9/site-packages/taggit/utils.py", line 124, in inner
    return func(self, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/taggit/managers.py", line 272, in set
    self.add(*tags, **kwargs)
  File "venv/lib/python3.9/site-packages/taggit/utils.py", line 124, in inner
    return func(self, *args, **kwargs)
  File "venv/lib/python3.9/site-packages/modelcluster/contrib/taggit.py", line 45, in add
    tag_objs = self._to_tag_model_instances(tags, {})
  File "venv/lib/python3.9/site-packages/taggit/managers.py", line 205, in _to_tag_model_instances
    raise ValueError(
ValueError: Cannot add [<Tag: plop>, <Tag: plip>] (<class 'modelcluster.queryset.FakeQuerySet'>). Expected <class 'django.db.models.base.ModelBase'> or str.
[10/Sep/2021 14:54:35] "POST /admin/pages/27/copy/ HTTP/1.1" 500 134999

Screenshot_2021-09-10 ValueError at admin pages 27 copy

Software versions:

  • django-modelcluster 5.1
  • django-taggit 1.3.0
  • wagtail 2.11.8

Best regards. François

fpoulain avatar Sep 10 '21 13:09 fpoulain

If that helps, the SitePage(Page)'s field mots_cles has been defined using:

      mots_cles = ClusterTaggableManager(                                         
          "Mots clés",                                                            
          through='MotsClesTag',                                                  
          blank=True,                                                             
          related_name='tags',                                                    
      )

with

  class MotsClesTag(TaggedItemBase):                                              
      content_object = ParentalKey(                                               
          'SitePage', on_delete=models.CASCADE, related_name='tagged_items'       
      )

It may be tricky to build a minimal example reproducing the bug, but if it's definitly needed I may try.

fpoulain avatar Sep 10 '21 14:09 fpoulain

FYI, we rather cheaply sidestepped the issue by editing our base page model to add tags and authors (another tag type for us) to the default_exclude_fields_in_copy list that is declared on the Wagtail base Page model. This stopped server errors and gave editors a clean copy work flow.

This keeps tags and author tags out of the admin request cycle, but tags and authors still get applied to the copied page.

    default_exclude_fields_in_copy = Page.default_exclude_fields_in_copy + [
        'tags',
        'authors'
    ]

higs4281 avatar Sep 10 '21 16:09 higs4281

default_exclude_fields_in_copy

It seems that the stable interface is exclude_fields_in_copy.

fpoulain avatar Sep 16 '21 14:09 fpoulain