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

Support Wagtail 5.2+, Wagtail 6+, Python 3.12 and Django 5 #387

Merged
merged 13 commits into from
Apr 12, 2024

Conversation

Morsey187
Copy link
Contributor

This PR adds support for:

  • Wagtail 6.0
  • Wagtail 5.2
  • Djangov5
  • Pyhon 3.12

There was one fix needed for Wagtail v6.0 to resolve an import error Query model moved to wagtail.contrib.search_promotions, and I've included commits from #385 for added Python and Django coverage.

Notes

You can test individual environments with tox using:

tox -e py310-django42-wagtail60

grapple/utils.py Outdated Show resolved Hide resolved
@@ -327,7 +327,6 @@ class Migration(migrations.Migration):
models.ImageField(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue:
My local build wants to remove this field, although, I'm not sure why as the field looks to exist still
https://github.com/wagtail/wagtail/blob/main/wagtail/images/models.py#L246

Would be great if this could be tested by someone else locally, to see if migration changes are needed and/or requested.

Copy link
Contributor

@jams2 jams2 Apr 10, 2024

Choose a reason for hiding this comment

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

Are you referring to the change below (line 330)? It's just removing the verbose name attribute, which seems to make sense as it matches the field name. If I run makemigrations against your branch I get the following output:

Migrations for 'testapp':
  testapp/migrations/0003_alter_blogpage_body_alter_customimage_file.py
    - Alter field body on blogpage
    - Alter field file on customimage

So it seems there's some changes that haven't been added to migrations. In the past, for projects with test apps in the past I've removed all the test app migrations and remade them fresh using the newest oldest supported versions of Django and Wagtail - you just need to run tox on all the envs to make sure the migrations don't fail on any particular version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed over slack, I've updated the migrations on django 4.2 and wagtail 5.2

tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated
py{311}-django{41,42}-wagtail{50,51}
py{311,312}-django{41,42}-wagtail{50,51,52}
py{311,312}-django{42,50}-wagtail60
py{38,39,310,311,312}-django{42,50}-wagtail{52,60}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add Python 3.12, Wagtail 6, and remove Wagtail 4 from PyPI classifiers https://github.com/torchbox/wagtail-grapple/blob/main/pyproject.toml#L17 ?

tests/requirements.txt Outdated Show resolved Hide resolved
tests/requirements.txt Outdated Show resolved Hide resolved
@zerolab zerolab merged commit 806dd04 into torchbox:main Apr 12, 2024
15 checks passed
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.

3 participants