Merge ~lvoytek/ubuntu/+source/python-django-modelcluster:fix-django4-compatibility into ubuntu/+source/python-django-modelcluster:ubuntu/devel

[Lena Voytek]

Fix compatibility with Django 4 by updating a test file

PPA building against 4.2: https://launchpad.net/~lvoytek/+archive/ubuntu/django-4-mantic
PPA building against 3.2 (currently in Ubuntu): https://launchpad.net/~lvoytek/+archive/ubuntu/django3-mantic

[Lucas Kanashiro (lucaskanashiro)]

Thanks for this MP Lena! Your patch indeed fixes the test failure, I checked the upstream code and it should also fail with Django 4:

https://github.com/wagtail/django-modelcluster/blame/main/tests/tests/test_cluster_form.py#L261

In the setup.py, there is no safeguard to avoid running it with Django 4:

https://github.com/wagtail/django-modelcluster/blob/main/setup.py#L21

So I think you can submit a PR already anticipating that. WDYT?

review: Needs Information

[Lena Voytek (lvoytek)]

I think that's reasonable. Though I looked at the open PRs upstream and actually found one that fixes the same issue I found in a similar way, https://github.com/wagtail/django-modelcluster/pull/174, albeit with a lot of extra changes.

[Lena Voytek (lvoytek)]

For us though, python-django-modelcluster will have to be compatible with 4.2 since Django 3.2 will no longer be available once the transition happens.

[Lucas Kanashiro (lucaskanashiro)]

Thanks for referring to this PR. I just tried to apply part of the changes in that PR (changes inside the test/ directory only) and they still work fine with Django 3.2 in the archive, and I assume it should work with Django 4.2 from your PPA as well. Could you please try that out?

I understand it was not merged upstream yet but the changes look reasonable to me. I think we should at least try to follow that, in this case we can use the proposed patch with some modifications. Moreover, the patch seems to also make sure the tests pass with Django master branch (>= 5.0), so this can avoid a similar issue when we perform the next transition. WDYT?

824a042... by Lena Voytek on 2023-08-03

* debian/patches/fix_test_suite_django_4plus.patch: Fix test suite to
  maintain compatibility with Django 4.2+ (LP: #2022089)

f9dd908... by Lena Voytek on 2023-08-03

changelog

[Lena Voytek (lvoytek)]

Sounds good to me, I turned my patch into a backport containing the test directory fixes

[Lucas Kanashiro (lucaskanashiro)]

Thanks Lena! That looks good to me now. I am sponsoring the upload for you:

Uploading python-django-modelcluster_6.0-2ubuntu1.dsc
Uploading python-django-modelcluster_6.0-2ubuntu1.debian.tar.xz
Uploading python-django-modelcluster_6.0-2ubuntu1_source.changes

review: Approve

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: lucaskanashiro, lvoytek
Uploaders: lucaskanashiro
MP auto-approved

review: Approve

diff --git a/debian/changelog b/debian/changelog
index a1ea184..c7b9e6e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+python-django-modelcluster (6.0-2ubuntu1) mantic; urgency=medium
+
+  * d/p/fix_test_suite_django_4plus.patch: Fix test suite to maintain
+    compatibility with Django 4.2+ (LP: #2022089)
+
+ -- Lena Voytek <[e-mail]>  Tue, 01 Aug 2023 15:52:22 -0700
+
 python-django-modelcluster (6.0-2) unstable; urgency=medium
 
   * Team upload
diff --git a/debian/control b/debian/control
index 513e8bf..ebf67cb 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: python-django-modelcluster
 Section: python
 Priority: optional
-Maintainer: Debian Python Team <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian Python Team <[e-mail]>
 Uploaders:
  Michael Fladischer <[e-mail]>,
 Build-Depends:
diff --git a/debian/patches/fix_test_suite_django_4plus.patch b/debian/patches/fix_test_suite_django_4plus.patch
new file mode 100644
index 0000000..e941441
--- /dev/null
+++ b/debian/patches/fix_test_suite_django_4plus.patch
@@ -0,0 +1,59 @@
+Description: Fix build against Django 4.2+ by updating test suite
+ When building against Django 4, the test_formfield_callback test fails due to
+ an AssertionError. This is caused by the formfield_callback variable being
+ missing from the BandFormWithFFC Meta subclass. In previous Django versions
+ this was fine since formfield_callback exists in the BandFormWithFFC class
+ itself. The issue is fixed by also adding the field to the Meta class in
+ Django 4.2 and above. Meanwhile, for Django 5.0, an addition to test_tag is
+ needed when checking html generation. In the new version, Django adds the
+ aria-describedby attribute for accessibility. Adding this to the test html
+ fixes the issue.
+Author: irtazaakram <[e-mail]>
+Origin: backport, https://github.com/wagtail/django-modelcluster/pull/174/commits/4e0daeb0d5b3f7ab77c2edac4b7cc841fa2ed603
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/mantic/+source/python-django-modelcluster/+bug/2022089
+Last-Update: 2023-08-02
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/tests/tests/test_cluster_form.py
++++ b/tests/tests/test_cluster_form.py
+@@ -272,6 +272,9 @@
+                 fields = ['name']
+                 formsets = ['members', 'albums']
+ 
++                if DJANGO_VERSION >= (4, 2):
++                    formfield_callback = formfield_for_dbfield
++
+         form = BandFormWithFFC()
+         self.assertEqual(Textarea, type(form['name'].field.widget))
+         self.assertEqual(Textarea, type(form.formsets['members'].forms[0]['name'].field.widget))
+--- a/tests/tests/test_tag.py
++++ b/tests/tests/test_tag.py
+@@ -2,11 +2,12 @@
+ 
+ import unittest
+ 
++from django import VERSION as DJANGO_VERSION
+ from django.test import TestCase, override_settings
+ from taggit import VERSION as TAGGIT_VERSION
+ from taggit.models import Tag
+-from modelcluster.forms import ClusterForm
+ 
++from modelcluster.forms import ClusterForm
+ from tests.models import NonClusterPlace, Place, TaggedPlace
+ 
+ 
+@@ -148,7 +149,13 @@
+         mission_burrito.tags.add('burrito', 'mexican')
+         form = PlaceForm(instance=mission_burrito)
+         form_html = form.as_p()
+-        self.assertInHTML('<input type="text" name="tags" value="burrito, mexican" id="id_tags">', form_html)
++        html = '<input type="text" name="tags" value="burrito, mexican" id="id_tags">'
++
++        if DJANGO_VERSION >= (5, 0):
++            # https://docs.djangoproject.com/en/dev/releases/5.0/#forms
++            html = '<input type="text" name="tags" value="burrito, mexican" aria-describedby="id_tags_helptext" id="id_tags">'
++
++        self.assertInHTML(html, form_html)
+ 
+     @override_settings(TAGGIT_CASE_INSENSITIVE=True)
+     def test_case_insensitive_tags(self):
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..ac95d12
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1 @@
+fix_test_suite_django_4plus.patch