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

Failures on 3.2 #455

Closed
yakky opened this issue Aug 14, 2020 · 2 comments · Fixed by #461
Closed

Failures on 3.2 #455

yakky opened this issue Aug 14, 2020 · 2 comments · Fixed by #461

Comments

@yakky
Copy link
Member

yakky commented Aug 14, 2020

Apparently this change django/django@9c9a3fe in django master broke custom version of PolymorphicQuerySet._filter_or_exclude

Upstream change is weird, and we need to investigate the rationale before adapting our code

@AdamDonna
Copy link
Contributor

AdamDonna commented Aug 19, 2020

I've taken a quick look at this one and it looks like the api of _filter_or_exclude has change to take args, kwargs and negate as their respective types rather than retaining the old api.

This passes all tests for django master and above but isn't backwards compatable because the api of _filter_or_exclude has changed.

    def _filter_or_exclude(self, negate, args, kwargs):
        # We override this internal Django functon as it is used for all filter member functions.
        q_objects = translate_polymorphic_filter_definitions_in_args(
            queryset_model=self.model, args=args, using=self.db
        )
        # filter_field='data'
        additional_args = translate_polymorphic_filter_definitions_in_kwargs(
            queryset_model=self.model, kwargs=kwargs, using=self.db
        )
        args = list(q_objects) + additional_args
        return super(PolymorphicQuerySet, self)._filter_or_exclude(
            negate=negate, args=args, kwargs=kwargs
        )

I'm new to opensource so there a standard method of proxying we can use to be backwards compatible with django less than 3.2?

@AdamDonna
Copy link
Contributor

^^ Actually I have an idea. I'll have a PR up soon and we can discuss the relative merits of the approach on that and go from there 👍

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

Successfully merging a pull request may close this issue.

2 participants