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

Support/wagtail 51 #544

Merged
merged 9 commits into from
Dec 19, 2023
Merged

Support/wagtail 51 #544

merged 9 commits into from
Dec 19, 2023

Conversation

lparsons396
Copy link
Contributor

@lparsons396 lparsons396 commented Oct 11, 2023

This upgrades Wagtail to 5.1

Support ticket: https://torchbox.monday.com/boards/1130687432/pulses/1264168986
Wagtail 5.1 release notes: https://docs.wagtail.org/en/stable/releases/5.1.html#

Changes:

  • wagtail.contrib.modeladmin is soon to be deprecated and so the Wagtail_modeladmin package has been used to continue supporting its features

Note: The decision was made to use the modeladmin package instead of migrating to Snippets

Copy link
Contributor

@victoriachan victoriachan left a comment

Choose a reason for hiding this comment

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

I know we agreed to just use the ModelAdmin package for other projects. However as this is our own project, I'm wondering if we can update it to use snippets.

It looks like the model admin is only used for the Taxonomy section, with just the Service model. It was added in 2019, and the customisation is mainly to display the usage count. Usage count is now part of Wagtail core. I think we can ditch the whole customisation and just add Service as a standard snippet.

Or if have the time, we could also add custom SnippetViewSet, to load the Service admin page in the same URL as before, and perhaps displaying the usage count on the listing so it can be seen in one glance. It looks quite straightforward.

Another more compelling reason to ditch the existing modeladmin code is that the usage count is broken. E.g. click on a usage on this page /admin/taxonomy/service/ I didn't spot this earlier as this is a rarely used feature.

@lparsons396 lparsons396 force-pushed the support/wagtail-51 branch 2 times, most recently from 81e25fe to 2e2aeff Compare October 17, 2023 13:08
Copy link
Contributor

@victoriachan victoriachan left a comment

Choose a reason for hiding this comment

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

Looks good! Well done for adding the snippet admin group to map the Taxonomy menu item.

Just need to remove the get_admin_urls_for_registration() method from the snippet view class as it is a model admin method. Also I included some small nice-to-have customisation for the Services admin listing.

When I tested locally, the usage count is always 0 for the Service snippets, somehow, though the Service type is used in some blog posts. Even after running rebuild_references_index, they are 0. I'm not sure why. We might have to troubleshoot this on a separate ticket if this feature is required. Just something to note.

It shouldn't hold us back as the previous implementation is already broken and not showing usage anyway.

tbx/taxonomy/admin.py Outdated Show resolved Hide resolved
tbx/taxonomy/admin.py Show resolved Hide resolved
Copy link
Contributor

@victoriachan victoriachan left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you for your work!

@zerolab
Copy link
Member

zerolab commented Nov 23, 2023

Any chance we can get this in?

cc @helenb

@helenb
Copy link
Member

helenb commented Nov 23, 2023

@zerolab On the board it is with Anna for internal testing. I'll ask her where we are with it.

@helenb helenb merged commit c82fc20 into main Dec 19, 2023
2 checks passed
@helenb helenb deleted the support/wagtail-51 branch December 19, 2023 10:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants