Opened 10 years ago

Closed 10 years ago

#22552 closed Bug (invalid)

1.7 beta 3 regression - BoundField.value() tries to call GenericRelatedObjectManager instance without args

Reported by: stephenmcd Owned by: nobody
Component: contrib.contenttypes Version: 1.7-beta-2
Severity: Release blocker Keywords:
Cc: loic84 Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In Mezzanine we have a form widget for GFK fields. With Django 1.7 beta 3, we get the following error:

http://hastebin.com/qixuzotumi.mel

AFAICT: In Django 1.6, GenericRelatedObjectManager was not callable, but in 1.7 it implements __call__. BoundField.value() then calls an instance of it without args, but GenericRelatedObjectManager.__call__ expects a manager arg. GenericRelatedObjectManager also implements do_not_call_in_templates, which seems appropriate here. If I change BoundField.value() callable check to include and not getattr(data, "do_not_call_in_templates", False) then it resolves the issue.

Happy to submit a patch if this sounds like the correct fix - really not sure though, quite out of my depth.

Change History (7)

comment:1 by Tim Graham, 10 years ago

Cc: loic84 added
Component: Uncategorizedcontrib.contenttypes
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Change is 04a2a6b0f9cb6bb98edfe84bf4361216d60a4e38. What do you think Loic?

comment:2 by loic84, 10 years ago

Putting and not getattr(data, "do_not_call_in_templates", False) in BoundField.value() is ugly since this it's outside the template system.

I don't quite understand how you can provide a custom widget for GFK, it's a virtual field, it doesn't implement a formfield method, and it doesn't even inherit it from models.Field since it subclasses object directly.

Real support for virtualfields hasn't landed in Django yet (refs https://github.com/koniiiik/django/tree/soc2013/composite-fields).

Also GenericRelatedObjectManager is from GenericRelation not GenericForeignKey.

Could you provide more context? Maybe some examples or pointers to the mezzanine codebase?

comment:3 by stephenmcd, 10 years ago

Sorry I really rushed the bug report.

Some useful context might be where a related issue came up and was resolved as part of the 1.6.1 bugfix release: https://code.djangoproject.com/ticket/21428

Again that was for the example with GFKs using custom form fields in the admin,something Mezzanine has made heavy use of for a few years now.

I'm not sure how useful it will be to go over the Mezzanine code base, it's a very complex example. I might be able to dig up the old example we had for #21428 - either way it will take me some time.

Anyway here are the relevant parts:

Here is our GFK field:
https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/generic/fields.py#L164

Here is a base model that many subclass, eventually using it:
https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/core/models.py#L125

Here is an admin class with the above field in its fieldsets:
https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/core/admin.py#L34-L52

And finally, here is the actual widget that the admin's form will use for the GFK:
https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/generic/forms.py#L18

As you can see there are a significant number of layers to this and so without knowing much about Mezzanine I fear going over its code might not prove very useful. Again, I'll try and come up with a minimal code example if I can, it might just take me some time. Alternatively it's quite easy to reproduce this by installing the current master branch of Mezzanine from GitHub and running one of these admin change interfaces against Django 1.7 beta.

The guts of it is though it's always been possible for GFK fields to be exposed as form fields in the admin given a custom widget, and this has regressed due to the new __call__ implementation, and that one extra check in BoundField.value() will resolve it at least.

comment:4 by stephenmcd, 10 years ago

Also yes I've been referring to GenericRelation as GFK which really confuses things, sorry about that!

comment:5 by loic84, 10 years ago

@stephenmcd could you try https://gist.github.com/11458982?

With the latest master of both Mezzanine and Django I get ImportError: cannot import name 'conf' from 'mezzanine' when attempting to runserver (probably a circular import); as a result the patch is completely untested, let me know the outcome.

comment:6 by devo, 10 years ago

@loic84 - you're a superstar, that resolves the issue on my end. Thank you very much!

comment:7 by Tim Graham, 10 years ago

Resolution: invalid
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top