Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

Upgrade to Wagtail 5.2.x #558

Merged
merged 20 commits into from
Dec 21, 2023
Merged

Upgrade to Wagtail 5.2.x #558

merged 20 commits into from
Dec 21, 2023

Conversation

lparsons396
Copy link
Contributor

This PR upgrades Wagtail to version 5.2

See Monday ticket & Wagtail 5.2 release notes.

This branch builds on the Wagtail 5.1 upgrade #544

The upgrade considerations actualise many of the deprecation warnings is the 5.1 upgrade and so few changes have been necessary here. Upgrading Wagtail has also necessitated the upgrading of django-filter (22.1 -> 23.4) and django-modelcluster (6.0 -> 6.1). All packages have been checked against the package board and use the appropriate tag / branch as needed.

Copy link
Contributor

@nickmoreton nickmoreton left a comment

Choose a reason for hiding this comment

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

Hi @lparsons396 Thank for doing this. There's a few dependencies that could do with been bumped to latest releases before is merged.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@@ -16,7 +16,7 @@ jobs:

services:
postgres:
image: postgres:9.6
image: postgres:12.3
Copy link
Member

Choose a reason for hiding this comment

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

let's match this with the production version (which is 13.12 at the moment).

pyproject.toml Outdated
Django = "~3.2.20"
wagtail = "~5.0"
python = "^3.11"
Django = "4.2"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Django = "4.2"
Django = "~4.2"

as = "4.2" literally asks for version 4.2, not 4.2.x

@lparsons396
Copy link
Contributor Author

Thanks for your review @zerolab , let me know if there's anything else! :)

Copy link
Member

@zerolab zerolab left a comment

Choose a reason for hiding this comment

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

:shipit:

@zerolab zerolab changed the title Support/wagtail 52 Upgrade to Wagtail 5.2.x Dec 13, 2023
@zerolab
Copy link
Member

zerolab commented Dec 19, 2023

@helenb imho this is ready to go too

@helenb
Copy link
Member

helenb commented Dec 19, 2023

@helenb imho this is ready to go too

From @katdom13 on slack:

"I’m also encountering this error during staging deployment for 5.2:
TypeError: cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator.
I can replicate this. I reverted the change on staging for now and will investigate this along with the above tomorrow."

I will also need to wait to hear if 5.2 has been tested on staging.

@zerolab
Copy link
Member

zerolab commented Dec 19, 2023

TypeError: cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator.
@helenb @katdom13 the fix for that would be wagtail/wagtail.org@3410fbc

tbx/urls.py Outdated
Comment on lines 88 to 90
Page.serve = method_decorator(get_default_cache_control_decorator(), name="serve")(
Page.serve
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fixes the error on app startup

cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator.

image

@katdom13
Copy link
Collaborator

@helenb imho this is ready to go too

From @katdom13 on slack:

"I’m also encountering this error during staging deployment for 5.2: TypeError: cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator. I can replicate this. I reverted the change on staging for now and will investigate this along with the above tomorrow."

I will also need to wait to hear if 5.2 has been tested on staging.

Hi @helenb ,
Apologies, I reverted this yesterday on staging when I saw the errors, but failed to remove the tag here in this MR. Staging should still be in 5.1

@katdom13
Copy link
Collaborator

katdom13 commented Dec 20, 2023

Hi @zerolab ,

TypeError: cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator.
@helenb @katdom13 the fix for that would be wagtail/wagtail.org@3410fbc

I saw this just now and updated the fix accordingly. Could you please review the changes starting at:
bf06055?

Thanks!

@zerolab
Copy link
Member

zerolab commented Dec 20, 2023

@katdom13 they LGTM. Added another fix (e7d3bb5)

@helenb
Copy link
Member

helenb commented Dec 20, 2023

@katdom13 I've created this ticket for Anna to test this on staging so we can be clear when this is ready to be merged: https://torchbox.monday.com/boards/1192293412/pulses/1347671612

@helenb helenb merged commit 61e5439 into main Dec 21, 2023
3 checks passed
@helenb helenb deleted the support/wagtail-52 branch December 21, 2023 08:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants