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

InheritanceManagerMixin get_queryset not work with Manager.from_queryset [Django 1.9] #205

Closed
seunghokimj opened this issue Jan 28, 2016 · 9 comments

Comments

@seunghokimj
Copy link

Since QuerySet._clone() has been changed at Djang 1.9,
django/django@4c3bfe9#diff-5b0dda5eb9a242c15879dc9cd2121379R1043

InheritanceManagerMixin get_queryset always returns InheritanceQuerySet() because there is no 'klass'.

[Django < 1.9]
"c = klass(model=self.model, query=query, using=self._db, hints=self._hints)" clones new my Managers QuerySet.

However, at [Django 1.9]
"clone = self.class(model=self.model, query=query, using=self._db, hints=self._hints)" always clones InheritanceQuerySet.

So my Manager.from_queryset does not adapt.

@kezabelle
Copy link
Collaborator

Hi,
Are you using the latest PyPI release (2.4)? #192 was merged in November to fix what I believe is the issue (ce8deed), and should form part of the 2.4 release from December.

If you're not using the latest version, please try upgrading it to see if that resolves your issue. If you are using the latest version and it isn't working, please provide a minimal example of your code so it can be demonstrated how the previous fix missed a use-case.

@carljm
Copy link
Collaborator

carljm commented Jan 28, 2016

Closing pending more information.

@carljm carljm closed this as completed Jan 28, 2016
@seunghokimj
Copy link
Author

@kezabelle kezabelle
This is my sample code I attached.
when you access "/tests/get-queryset/" you can get Error

'InheritanceQuerySet' object has no attribute 'created_by'
Request Method: GET
Request URL: http://localhost:8000/tests/get-queryset/
Django Version: 1.9.1
Exception Type: AttributeError
Exception Value:
'InheritanceQuerySet' object has no attribute 'created_by'
...
...

So I tried to fix this issue.
This bellow code works.

class InheritanceManagerMixin(object):
    use_for_related_fields = True
    def get_queryset(self):
        # return InheritanceQuerySet(self.model)
        return self._queryset_class(self.model)

mysite.zip

@carljm
Copy link
Collaborator

carljm commented Jan 29, 2016

What version of django-model-utils?

@seunghokimj
Copy link
Author

The version is
Django==1.9.1
django-model-utils==2.4

@carljm
Copy link
Collaborator

carljm commented Jan 29, 2016

Your proposed "fix" would turn InheritanceManagerMixin into a no-op by default.

If you need to combine InheritanceQuerySet with another custom queryset, define your own manager that inherits InheritanceManagerMixin, and override its get_queryset method to return an instance of your own custom QuerySet class that inherits from InheritanceQuerySetMixin. See the note at the bottom of http://django-model-utils.readthedocs.org/en/latest/managers.html#mixins

@seunghokimj
Copy link
Author

I am sorry that I just uploaded my sample source code by ZIP file.

Could you please see below code?
https://gist.github.com/root-shkim/37e8395f94290012efa5

This custom manger NodeManager and queryset NodeQueySet inherit InheritanceManagerMixin and NodeManager overrides its get_queryset as well.

Please point out my code, when I misuse django-model-utils

@carljm
Copy link
Collaborator

carljm commented Feb 1, 2016

I see. The key information I was missing was that you are using from_queryset(). It's not changes to clone that are relevant here, it's that InheritanceManager was written before from_queryset() (and the _queryset_class class attribute) existed.

You can still make it work as-is, if you (as mentioned above) explicitly "return an instance of your own custom QuerySet class" from your get_queryset method, instead of calling super(). Your current get_queryset manager method just uses whatever queryset class is returned by super(), which will always be the basic InheritanceQuerySet given the current implementation of InheritanceManager.

But I would accept a PR to update InheritanceManagerMixin to respect self._queryset_class (and for InheritanceManager to have it set to InheritanceQuerySet).

@carljm carljm reopened this Feb 1, 2016
@seunghokimj
Copy link
Author

It is better to explicitly return an instance of custom QuerySet class instead of calling super().
Thanks @carljm for your kind.

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

3 participants