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

Upgrade Wagtail to 6.1 #8517

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Upgrade Wagtail to 6.1 #8517

merged 1 commit into from
Jul 25, 2024

Conversation

willbarton
Copy link
Member

@willbarton willbarton commented Jul 22, 2024

This PR upgrades us to Wagtail 6.1. The required changes are minor.

One thing to note, we still have a RemovedInWagtail70Warning: The usage of WidgetWithScript hook is deprecated. Use external scripts instead. warning that will be fixed by wagtail/wagtail-autocomplete#180 when (if?) it gets merged.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)

@chosak
Copy link
Member

chosak commented Jul 23, 2024

The unittest failure in jobmanager can be reproduced locally by visiting http://localhost:8000/admin/snippets/jobmanager/region/. The issue seems to be this line. Here's a diff that fixes:

diff --git a/cfgov/jobmanager/models/django.py b/cfgov/jobmanager/models/django.py
index e83a6883c..4361f6867 100755
--- a/cfgov/jobmanager/models/django.py
+++ b/cfgov/jobmanager/models/django.py
@@ -92,6 +92,12 @@ class Region(ClusterableModel):
         InlinePanel("major_cities", label="Major cities"),
     ]
 
+    def states_in_region(self):
+        return ", ".join(str(state) for state in self.states.all())
+
+    def major_city_names(self):
+        return "; ".join(str(city) for city in self.major_cities.all())
+
 
 class State(models.Model):
     name = models.CharField(max_length=255)
diff --git a/cfgov/jobmanager/views.py b/cfgov/jobmanager/views.py
index 35e29f6d7..3b0afba27 100644
--- a/cfgov/jobmanager/views.py
+++ b/cfgov/jobmanager/views.py
@@ -48,13 +48,12 @@ class RegionViewSet(SnippetViewSet):
     icon = "site"
     menu_label = "Regions"
 
-    def states_in_region(self):
-        return ", ".join(str(state) for state in self.states.all())
-
-    def major_cities(self):
-        return "; ".join(str(city) for city in self.major_cities.all())
-
-    list_display = ["abbreviation", "name", states_in_region, major_cities]
+    list_display = [
+        "abbreviation",
+        "name",
+        "states_in_region",
+        "major_city_names",
+    ]
 
 
 class ServiceTypeViewSet(SnippetViewSet):

This was just changed by me as part of the 5.1 upgrade in #8075; I'm surprised this ever worked because according to the release notes specifying the callable directly was never supported.

@willbarton willbarton force-pushed the upgrade/wagtail61 branch 2 times, most recently from 4908db9 to 3a0d0c7 Compare July 23, 2024 14:56
Co-Authored-By: Andy Chosak <[email protected]>
@willbarton willbarton marked this pull request as ready for review July 25, 2024 13:12
Copy link
Member

@chosak chosak left a comment

Choose a reason for hiding this comment

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

LGTM!

@willbarton willbarton added this pull request to the merge queue Jul 25, 2024
Merged via the queue into main with commit 9880439 Jul 25, 2024
17 checks passed
@willbarton willbarton deleted the upgrade/wagtail61 branch July 25, 2024 20:53
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