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

Django main (4.1) compatibility #8028

Merged
merged 4 commits into from Feb 23, 2022
Merged

Django main (4.1) compatibility #8028

merged 4 commits into from Feb 23, 2022

Conversation

gasman
Copy link
Collaborator

@gasman gasman commented Feb 22, 2022

Get tests passing against current Django main branch. Incorporates #8026 and #8027, and pulls in fixes from django-taggit and django-modelcluster:

jazzband/django-taggit#791
jazzband/django-taggit#792
wagtail/django-modelcluster#154

along with an additional fix for django/django#15318, which was just merged today and feels like it's going to bite a lot of Django code in the wild (so I may well drop the django-developers mailing list a line tomorrow to ask if it's really a good idea to put that in a minor release with no deprecation path...)

Fixes a test failure against Django main.

In wagtail#2552, a fix was applied to ensure that the project-level MESSAGE_TAGS setting was ignored, allowing end-users to customise that setting for their own projects without it leaking into Wagtail admin styles.

Unfortunately, the test was flawed (or was broken in a Django regression at some point): in Django <=4.0, MESSAGE_TAGS was not affected by override_settings after the first request, which meant that unless the test was run in isolation, the custom classname that was supposed to flag up the problem never got applied, and the test always succeeded.

The change to SVG icons broke the intent of wagtail#2552, since it used message.level_tag for the icon's classname (and this picks up MESSAGE_TAGS customisations), but due to the broken test this went unnoticed.

django/django@24b3165 fixed the override_settings behaviour, making the test fail as it should have done long ago.

Here we adjust the test to not rely on override_settings (so that it does what it's supposed to do on all Django versions), fix a test that gets broken as a side effect (because it's unnecessarily checking message.level_tag), and fixes our SVG-icon-powered message include to bypass the MESSAGE_TAGS setting like the old implementation did.

Confusing? Yes.
As of django/django@7c4f396 the type="text/css" attribute is omitted.
@squash-labs
Copy link

squash-labs bot commented Feb 22, 2022

Manage this branch in Squash

Test this branch here: https://gasmanfixdjango40-o8voe.squash.io

Copy link
Contributor

@kaedroho kaedroho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants