django3.2 fixes for main test job (#16481)

* `static` is new name for `admin_static` template tag library

* escaping test fixes

* add obj arg to CollectionAddonInline.has_add_permission

* TestExceptionHandlerWithViewSet fix

* TranslationMultiDbTests fix

* re-enable main job with django32

* update blocklist tests with django's fancy quotes and dashes

.circleci/config.yml

@@ -681,7 +681,7 @@ workflows:
             parameters:
               djangoversion:
                 - django22
-#                - django32 # django3.2 tests still failing
+                - django32
       - reviewers-and-zadmin:
           matrix:
             parameters:

src/olympia/activity/tests/test_models.py

@@ -16,7 +16,6 @@ from olympia.addons.models import Addon, AddonUser
 from olympia.amo.tests import (
     addon_factory,
     TestCase,
-    DQUOTE_ESCAPED,
     user_factory,
     version_factory,
 )
@@ -300,7 +299,7 @@ class TestActivityLog(TestCase):
         log_expected = (
             'Yolo role changed to Owner for <a href="/en-US/'
             'firefox/addon/a3615/">Delicious &lt;script src='
-            f'{DQUOTE_ESCAPED}x.js{DQUOTE_ESCAPED}&gt;Bookmarks</a>.'
+            '&#34;x.js&#34;&gt;Bookmarks</a>.'
         )
         assert log.to_string() == log_expected
 

src/olympia/amo/tests/__init__.py

@@ -88,8 +88,6 @@ ES_INDEX_SUFFIXES = {key: timestamp_index('') for key in settings.ES_INDEXES.key
 
 # django2.2 encodes with the decimal code; django3.2 with the hex code.
 SQUOTE_ESCAPED = escape("'")
-# Unfortunately `escape` returns `"` rather than the code so workaround:
-DQUOTE_ESCAPED = '"' if 'x' in SQUOTE_ESCAPED else '"'
 
 
 def get_es_index_name(key):

src/olympia/api/tests/test_exceptions.py

@@ -23,7 +23,7 @@ test_exception = SimpleRouter()
 test_exception.register('testexcept', DummyViewSet, basename='test-exception')
 
 
-@override_settings(ROOT_URLCONF=test_exception.urls)
+@override_settings(ROOT_URLCONF=tuple(test_exception.urls))
 class TestExceptionHandlerWithViewSet(TestCase):
     # The test client connects to got_request_exception, so we need to mock it
     # otherwise it would immediately re-raise the exception.

src/olympia/bandwagon/admin.py

@@ -29,7 +29,7 @@ class CollectionAddonInline(admin.TabularInline):
         # remove an add-on from a collection (they can't delete a collection).
         return self.has_change_permission(request, obj=obj)
 
-    def has_add_permission(self, request):
+    def has_add_permission(self, request, obj=None):
         return CollectionAdmin(Collection, self.admin_site).has_add_permission(request)
 
 

src/olympia/blocklist/tests/test_admin.py

@@ -3,6 +3,7 @@ import json
 
 from unittest import mock
 
+import django
 from django.conf import settings
 from django.contrib.admin.models import LogEntry, ADDITION
 from django.contrib.contenttypes.models import ContentType
@@ -21,6 +22,17 @@ from olympia.amo.urlresolvers import reverse
 from ..models import Block, BlocklistSubmission
 
 
+IS_DJANGO_32 = django.VERSION[0] == 3
+# django3.2 uses fancy double quotes in its admin logging too
+FANCY_QUOTE_OR_DOUBLE_OPEN = '“' if IS_DJANGO_32 else '"'
+FANCY_QUOTE_OR_DOUBLE_CLOSE = '”' if IS_DJANGO_32 else '"'
+# And sometimes it's a named entity instead, because reasons.
+FANCY_QUOTE_OR_ENTITY_OPEN = '“' if IS_DJANGO_32 else '"'
+FANCY_QUOTE_OR_ENTITY_CLOSE = '”' if IS_DJANGO_32 else '"'
+# Now with a 50% dash length improvement!
+LONG_DASH = '—' if IS_DJANGO_32 else '-'
+
+
 class TestBlockAdmin(TestCase):
     def setUp(self):
         self.admin_home_url = reverse('admin:index')
@@ -293,8 +305,8 @@ class TestBlocklistSubmissionAdmin(TestCase):
         assert log == ActivityLog.objects.for_versions(deleted_addon_version).last()
         assert not ActivityLog.objects.for_versions(pending_version).exists()
         assert [msg.message for msg in response.context['messages']] == [
-            'The blocklist submission "No Sign-off: guid@; dfd; some reason" '
-            'was added successfully.'
+            f'The blocklist submission {FANCY_QUOTE_OR_DOUBLE_OPEN}No Sign-off: guid@; '
+            f'dfd; some reason{FANCY_QUOTE_OR_DOUBLE_CLOSE} was added successfully.'
         ]
 
         response = self.client.get(
@@ -769,8 +781,9 @@ class TestBlocklistSubmissionAdmin(TestCase):
 
         assert [msg.message for msg in response.context['messages']] == [
             'The blocklist submission '
-            '"No Sign-off: any@new, partial@existing, full@exist...; dfd; '
-            'some reason" was added successfully.'
+            f'{FANCY_QUOTE_OR_DOUBLE_OPEN}No Sign-off: any@new, partial@existing, '
+            f'full@exist...; dfd; some reason{FANCY_QUOTE_OR_DOUBLE_CLOSE} was added '
+            'successfully.'
         ]
 
         # This time the blocks are updated
@@ -1245,14 +1258,18 @@ class TestBlocklistSubmissionAdmin(TestCase):
         log_entry = LogEntry.objects.get()
         assert log_entry.user == user
         assert log_entry.object_id == str(mbs.id)
-        assert log_entry.change_message == json.dumps(
-            [{'changed': {'fields': ['url', 'reason']}}]
-        )
+        change_json = json.loads(log_entry.change_message)
+        # change_message fields are the Field names rather than the fields in django3.2
+        change_json[0]['changed']['fields'] = [
+            field.lower() for field in change_json[0]['changed']['fields']
+        ]
+        assert change_json == [{'changed': {'fields': ['url', 'reason']}}]
 
         response = self.client.get(multi_url, follow=True)
         assert (
-            b'Changed "Pending: guid@, invalid@, second@invalid; '
-            b'new.url; a new reason thats longer than 40 cha...' in response.content
+            f'Changed {FANCY_QUOTE_OR_ENTITY_OPEN}Pending: guid@, invalid@, '
+            'second@invalid; new.url; a new reason thats longer than 40 cha...'
+            in response.content.decode('utf-8')
         )
 
     def test_edit_page_with_blocklist_signoff(self):
@@ -1424,8 +1441,9 @@ class TestBlocklistSubmissionAdmin(TestCase):
 
         response = self.client.get(multi_url, follow=True)
         assert (
-            b'Changed "Approved: guid@, invalid@'
-            b'" - Sign-off Approval' in response.content
+            f'Changed {FANCY_QUOTE_OR_ENTITY_OPEN}Approved: guid@, invalid@'
+            f'{FANCY_QUOTE_OR_ENTITY_CLOSE} {LONG_DASH} Sign-off Approval'
+            in response.content.decode('utf-8')
         )
         assert b'not a Block!' not in response.content
 
@@ -1502,11 +1520,12 @@ class TestBlocklistSubmissionAdmin(TestCase):
         )
 
         response = self.client.get(multi_url, follow=True)
+        content = response.content.decode('utf-8')
         assert (
-            b'Changed "Rejected: guid@, invalid@'
-            b'" - Sign-off Rejection' in response.content
+            f'Changed {FANCY_QUOTE_OR_ENTITY_OPEN}Rejected: guid@, invalid@'
+            f'{FANCY_QUOTE_OR_ENTITY_CLOSE} {LONG_DASH} Sign-off Rejection' in content
         )
-        assert b'not a Block!' not in response.content
+        assert 'not a Block!' not in content
 
         # statuses didn't change
         addon.reload()

src/olympia/translations/tests/test_models.py

@@ -555,14 +555,23 @@ class TranslationTestCase(TestCase):
 
 class TranslationMultiDbTests(TransactionTestCase):
     fixtures = ['testapp/test_models.json']
+    # ConnectionHandler has changed in django3.2 - databases is now a @property
+    patch_property = (
+        'databases'
+        if isinstance(
+            getattr(django.db.utils.ConnectionHandler, 'databases'),
+            django.utils.functional.cached_property,
+        )
+        else 'settings'
+    )
 
     def setUp(self):
-        super(TranslationMultiDbTests, self).setUp()
+        super().setUp()
         translation.activate('en-US')
 
     def tearDown(self):
         self.cleanup_fake_connections()
-        super(TranslationMultiDbTests, self).tearDown()
+        super().tearDown()
 
     def reset_queries(self):
         # Django does a separate SQL query once per connection on MySQL, see
@@ -585,7 +594,7 @@ class TranslationMultiDbTests(TransactionTestCase):
         }
 
     def cleanup_fake_connections(self):
-        with patch.object(django.db.connections, 'databases', self.mocked_dbs):
+        with patch.object(django.db.connections, self.patch_property, self.mocked_dbs):
             for key in ('default', 'slave-1', 'slave-2'):
                 connections[key].close()
 
@@ -599,7 +608,7 @@ class TranslationMultiDbTests(TransactionTestCase):
     @override_settings(DEBUG=True)
     @patch('multidb.get_replica', lambda: 'slave-2')
     def test_translations_reading_from_multiple_db(self):
-        with patch.object(django.db.connections, 'databases', self.mocked_dbs):
+        with patch.object(django.db.connections, self.patch_property, self.mocked_dbs):
             # Make sure we are in a clean environnement.
             self.reset_queries()
 
@@ -612,7 +621,7 @@ class TranslationMultiDbTests(TransactionTestCase):
     @patch('multidb.get_replica', lambda: 'slave-2')
     @pytest.mark.xfail(reason='Needs django-queryset-transform patch to work')
     def test_translations_reading_from_multiple_db_using(self):
-        with patch.object(django.db.connections, 'databases', self.mocked_dbs):
+        with patch.object(django.db.connections, self.patch_property, self.mocked_dbs):
             # Make sure we are in a clean environnement.
             self.reset_queries()
 
@@ -624,7 +633,7 @@ class TranslationMultiDbTests(TransactionTestCase):
     @override_settings(DEBUG=True)
     @patch('multidb.get_replica', lambda: 'slave-2')
     def test_translations_reading_from_multiple_db_pinning(self):
-        with patch.object(django.db.connections, 'databases', self.mocked_dbs):
+        with patch.object(django.db.connections, self.patch_property, self.mocked_dbs):
             # Make sure we are in a clean environnement.
             self.reset_queries()
 

src/olympia/users/templates/admin/users/userprofile/change_form.html

@@ -1,5 +1,5 @@
 {% extends "admin/change_form.html" %}
-{% load i18n admin_static admin_list admin_urls %}
+{% load i18n static admin_list admin_urls %}
 
 {% block submit_buttons_bottom %}
     {{ block.super }}

src/olympia/users/tests/test_helpers.py

@@ -1,7 +1,6 @@
 # -*- coding: utf-8 -*-
 import pytest
 
-from olympia.amo.tests import SQUOTE_ESCAPED, DQUOTE_ESCAPED
 from olympia.users.models import UserProfile
 from olympia.users.templatetags.jinja_helpers import user_link, users_list
 
@@ -33,7 +32,7 @@ def test_user_link_xss():
     user = UserProfile(
         username='jconnor', display_name="""xss"'><iframe onload=alert(3)>""", pk=1
     )
-    html = f'xss{DQUOTE_ESCAPED}{SQUOTE_ESCAPED}&gt;&lt;iframe onload=alert(3)&gt;'
+    html = 'xss&#34;&#39;&gt;&lt;iframe onload=alert(3)&gt;'
     assert user_link(user) == '<a href="%s" title="%s">%s</a>' % (
         user.get_absolute_url(),
         html,