Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy fails for a Wagtail page with no tags #134

Open
higs4281 opened this issue Sep 14, 2020 · 7 comments
Open

Copy fails for a Wagtail page with no tags #134

higs4281 opened this issue Sep 14, 2020 · 7 comments

Comments

@higs4281
Copy link

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
Copy link
Author

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.

@gasman
Copy link
Contributor

gasman commented Sep 14, 2020

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.

@higs4281
Copy link
Author

higs4281 commented Sep 14, 2020

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.

@fpoulain
Copy link

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

@kaedroho kaedroho reopened this Sep 10, 2021
@fpoulain
Copy link

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.

@higs4281
Copy link
Author

higs4281 commented Sep 10, 2021

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'
    ]

@fpoulain
Copy link

default_exclude_fields_in_copy

It seems that the stable interface is exclude_fields_in_copy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants