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

Handle path_infos / reverse_path_infos cached properties (Dj4.1 compatibility) #792

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

gasman
Copy link
Contributor

@gasman gasman commented Feb 22, 2022

In django/django@a697424 / django/django#14750, Django has introduced two new cached properties path_infos and reverse_path_infos to be used in preference to the get_path_info() and get_reverse_path_info() methods when not passing a filtered_relation argument.

TaggableManager is patching these methods, so to avoid Django bypassing them (specifically in django.db.query.Query.names_to_path) it needs to provide these cached properties too.

For additional bonus points, we also update TaggableManager's _get_mm_case_path_info and _get_gfk_case_path_info methods to make use of the new cached properties where available.

This (along with #791, incorporated here) gets the test suite passing again on the current Django main branch.

The old behaviour of assertQuerysetEqual automatically calling repr is deprecated in Django 3.2 https://docs.djangoproject.com/en/4.0/releases/3.2/#id3 and dropped in Django 4.1.
As of django/django@a697424 there are two new cached properties path_infos and reverse_path_infos which Django will use in preference to the get_path_info() and get_reverse_path_info() methods when not passing a filtered_relation argument. TaggableManager is patching these methods, and therefore it now needs to provide these cached properties to match Django's expectations.
These are introduced in django/django@a697424 to be used in preference to get_path_info / reverse_path_info when not passing filtered_relation.
@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #792 (c1954ac) into master (5f3d0df) will increase coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #792      +/-   ##
==========================================
+ Coverage   92.74%   92.91%   +0.17%     
==========================================
  Files           9        9              
  Lines         703      720      +17     
  Branches      136      140       +4     
==========================================
+ Hits          652      669      +17     
  Misses         34       34              
  Partials       17       17              
Impacted Files Coverage Δ
taggit/managers.py 96.12% <100.00%> (+0.19%) ⬆️

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 5f3d0df...c1954ac. Read the comment docs.

@rtpg
Copy link
Contributor

rtpg commented Feb 23, 2022

alright, looks good! Seems like the next steps here would be to actually prep a release with this 4.1 support

@rtpg rtpg merged commit 354c035 into jazzband:master Feb 23, 2022
@gasman
Copy link
Contributor Author

gasman commented Feb 23, 2022

@rtpg Thanks! I don't think there's any particular hurry to get a release out with this fix - at least until Django 4.1 is in release candidate stage, which is still a couple of months away (final release is scheduled for August). My motivation for fixing this now is to keep Wagtail's test suite passing against Django main so that we get early warning of any upcoming breaking changes - and for that purpose it's no problem for us to test against django-taggit master.

@gasman gasman mentioned this pull request Apr 28, 2022
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 this pull request may close these issues.

2 participants