Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#16913 closed Bug (duplicate)

Admin error when trying to delete object with a GenericRelation

Reported by: r1cky Owned by: carljm
Component: contrib.admin Version: 1.3
Severity: Normal Keywords:
Cc: jsocol Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you have a model A with a GenericForeignKey and a model B with a GenericRelation to A:

from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic
from django.db import models


class ModelA(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey('content_type', 'object_id')


class ModelB(models.Model):
    model_a = generic.GenericRelation(ModelA, related_name='my_related')

Create a ModelA and a related Model B:

b = ModelB.objects.create()
a = b.model_a.create()

Then try to delete B in the admin and you get a 500:

Traceback:
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/contrib/admin/options.py" in wrapper
  328.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  88.         response = view_func(request, *args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/contrib/admin/sites.py" in inner
  192.             return view(request, *args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapper
  25.             return bound_func(*args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/utils/decorators.py" in bound_func
  21.                 return func(self, *args2, **kwargs2)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/db/transaction.py" in inner
  209.                 return func(*args, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/contrib/admin/options.py" in delete_view
  1255.             [obj], opts, request.user, self.admin_site, using)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/contrib/admin/util.py" in get_deleted_objects
  76.     collector.collect(objs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/contrib/admin/util.py" in collect
  127.             return super(NestedObjects, self).collect(objs, source_attr=source_attr, **kwargs)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/db/models/deletion.py" in collect
  180.                                  nullable=True)
File "/Users/rlr/.virtualenvs/django/lib/python2.6/site-packages/django/contrib/admin/util.py" in collect
  123.                 self.add_edge(getattr(obj, source_attr), obj)

Exception Type: AttributeError at /admin/testapp/modelb/1/delete/
Exception Value: 'ModelA' object has no attribute 'my_related'

It looks like Generic Relations don't add the reverse manager to the related model (not sure if that is a bug or a feauture?) but the NestedObjects collector assumes that it will be there.

I added created a test that reproduces the issue and a potential fix: https://github.com/rlr/django/commit/8d147677fc

Change History (8)

comment:1 Changed 3 years ago by jsocol

  • Cc jsocol added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by carljm

  • Owner changed from nobody to carljm
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

Does this only occur if the GenericRelation has a related_name explicitly specified?

What's the purpose of specifying related_name on the GenericRelation in that example, since a reverse manager isn't added (a reverse manager doesn't really make sense, since you already have content_object; in a sense, the GenericRelation *is* the reverse manager for the GFK)?

If the answers to the above two questions are "yes" and "no purpose" (I don't have time at the moment to dig into it and see), it may be that the real bug here is just that GenericRelation shouldn't accept a useless related_name argument.

comment:3 follow-up: Changed 3 years ago by r1cky

The reason I needed related_name is that, in my case (1), the GenericRelation is actually in an abstract base class and we inherit from it into two models with the same name in separate apps. If I don't set it, I get an error about collisions during model validation when starting the dev server:

forums.thread: Accessor for m2m field 'watches' clashes with related m2m field 'Watch.thread_set'. Add a related_name argument to the definition for 'watches'.
kbforums.thread: Accessor for m2m field 'watches' clashes with related m2m field 'Watch.thread_set'. Add a related_name argument to the definition for 'watches'.

We have a model named Thread in two different apps. I guess that is a different bug? The validation of models probably shouldn't complain in this case.

(1) https://github.com/erikrose/django-tidings/blob/master/tidings/models.py#L118

comment:4 in reply to: ↑ 3 ; follow-up: Changed 3 years ago by carljm

Replying to r1cky:

The reason I needed related_name is that, in my case (1), the GenericRelation is actually in an abstract base class and we inherit from it into two models with the same name in separate apps. If I don't set it, I get an error about collisions during model validation when starting the dev server:

Yeah, that's a bug. If no reverse manager is actually added, there's no collision. Mind filing that one too?

Any chance you could temporarily remove one of those apps, remove the related_name, and see if the admin bug still happens?

comment:5 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by r1cky

Replying to carljm:

Yeah, that's a bug. If no reverse manager is actually added, there's no collision. Mind filing that one too?

Sure, will do!

Any chance you could temporarily remove one of those apps, remove the related_name, and see if the admin bug still happens?

Removing one of the apps and removing the related_name does fix the issue. I guess what I did is a workaround for the real bug :-)

comment:6 in reply to: ↑ 5 Changed 3 years ago by r1cky

Replying to r1cky:

Replying to carljm:

Yeah, that's a bug. If no reverse manager is actually added, there's no collision. Mind filing that one too?

Sure, will do!

Filed #16920

comment:7 Changed 3 years ago by carljm

  • Resolution set to duplicate
  • Status changed from assigned to closed

Thanks!

Closing this as duplicate of #16920, since that's the real bug here.

comment:8 Changed 2 years ago by bobah22@…

until the bug is not fixed, here's snippet to workaround for your case

Note: See TracTickets for help on using tickets.
Back to Top