Fixes issue 455 (incompatability with django master at 3.2)

[AdamDonna]

Fixes #455
Raised on django master in django/django@9c9a3fe

I'd like specific feedback on

• Agreed approach of handling django versions
• Documentation that will be necessary for maintenance in the future.
• Tests passing

[codecov]

Codecov Report

Merging #461 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #461      +/-   ##
==========================================
+ Coverage   88.80%   88.83%   +0.02%     
==========================================
  Files          34       34              
  Lines        2609     2616       +7     
==========================================
+ Hits         2317     2324       +7     
  Misses        292      292              

Impacted Files	Coverage Δ
polymorphic/query.py	91.45% <100.00%> (+0.26%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69bbe58...fa6808e. Read the comment docs.

[yakky]

Handling of the different django API looks perfectly reasonable to me

I would just indent the if get_django_version() ... in line with the other class methods, but it's a matter of taste

[AdamDonna]

I'm not sure we can indent that line it since it's operating on the class, I'm happy for you to push it up if you know how to make it work inline with the class definition 👍

[yakky]

I'm not sure we can indent that line it since it's operating on the class, I'm happy for you to push it up if you know how to make it work inline with the class definition +1

Sorry, the diff misled me to think you conditionally defined the method.

Something like the snippet below in the class body would work

if get_django_version() >= "3.2":
   def _filter_or_exclude(....):
      ....
else:
   def _filter_or_exclude(....):
      ....

See this example https://github.com/divio/django-filer/pull/1201/files#diff-71f149c65e62bb3b8fdcd50b4b00a845R105

[yakky]

But I would wait for @chrisglass input to check if he agrees on the approach

[AdamDonna]

Yeah cool, I never knew that was a thing. I much prefer that way so related methods are together and it's not a shock for future devs that there's a 2 definitions of _filter_or_exclude to handle
I've got that passing locally, it's ready to go depending on if @chrisglass agrees with the approach

[AdamDonna]

@chrisglass How do you feel about this solution?

[chrisglass]

This looks good - I'm a little bit sad that we have to have a conditional function declaration here, but since the signature changed upstream there's not much else we can do about it.

I'll merge this now, thanks a lot for the contribution!

👍