Migrate from ModelAdmin to Snippets

[dkirkham]

Fixes #459 for Wagtail >= 5.2.

This fairly large PR removes Wagtail ModelAdmin from the wagtailmenus admin pages, replacing it with Wagtail Snippets.

My approach has been keep as much of the existing wagtailmenus functionality as possible, and absolutely minimise the number of changes to the existing test suite. If functionality changes are desired, these should be discussed and designed accordingly and prepared as separate PRs.

I have also only been targeting Wagtail 5.2 LTS, because earlier versions (particularly 4.1 LTS and 5.1) are supported by existing wagtailmenus releases, and are as a whole only in support for a couple more weeks anyway. That said, the tests appear to work for some earlier Wagtail releases, although I haven't yet validated whether the UI is fully functional. The tests also also work, excepting one HTML markup test, against the current github wagtail#main and that problem may just be a test setup issue.

Given the changes are significant, and I would encourage any users of this package to test it within their overall projects before it is merged or released.

There are a few more things left to do, certainly before release, and the details here may depend on any real-life testing:

• the exact scope of Wagtail versions it will work with is yet to be determined... it may be wider than the setup.py file currently allows, and if it can be made wider, it will ease the migration
• the detailed documentation will require some updates, again mostly relating to version compatibility.

[MrCordeiro]

Hey @dkirkham, sorry for the delay in my response.

I had planned to look at this during the weekend, but then got caught up with an unexpected tasks. I'll review your pull request first thing this Saturday. In the meanwhile, I also added @ababic as a reviewer. He's the original author and may offer a much more valuable insight than I could (if he is available, of course).

[MrCordeiro]

Well done, @dkirkham !

Code looks super tight. Couldn't find any issues. I've also trying migrating a few of my projects to this version and they have all worked without a hitch!

2 questions:

1. Because this would be a major version upgrade, I about releasing as a rc version for now and then making the official release on February. Wagtail 4.1 support goes up to Feb 1st, so I think it makes sense if we officially moved to Wagtail 5.2 together. What do you think?
2. Do you think it makes sense to add a "Migrating" section to the docs. Basically, all one needs to do is to delete the modeladmin contrib packages, but it would put users at ease.

[MrCordeiro]

You remembered to change the docs! Wonderful!

[MrCordeiro]

Comment: this might be a breaking change, but maybe it makes sense for use to rename this module snippets. Or, if it breaks existing projects, add a comment for future maintainers understand these are, for all intents and purposes, what wagtail now understands as snippets.

[dkirkham]

I was contemplating renaming this file to admin.py but wanted to keep the commit changeset more understandable. I can easily add this as an additional commit.

[ababic]

@dkirkham / @MrCordeiro i think we can class this whole peice of work as a breaking change, because anyone extending these classes currently will likely need to do some refactoring to make their overriden behaviour compatible with snippets.

I would suggest maybe wagtailadmin.py as the filename, just to distinguish it a little from admin.py - which is usually where you find modeladmin classes for the Django admin.

[dkirkham]

Hi @MrCordeiro, thanks for the feedback and testing. Responding to your questions:

1. Wagtail 5.2 LTS is out now and for another year, so the aligning timing of this wagtailmenus release with Wagtail 6 (Feb) doesn’t achieve much. What is probably more useful is to see if this new wagtailmenus release can support 5.2 LTS and the forthcoming Wagtail 6. We can test that soon as the RC is imminent.
2. Sure, while I updated one of the docs, I didn’t check through the detailed documentation to keep the PR more focussed. Would you like me to add these changes to this PR?

[ababic]

@dkirkham is there a Snippet template we can extend here instead that would already do some of this stuff? It would be nice to only override blocks where needed, if possible.

[dkirkham]

Hi @ababic, my intent was to override only blocks, but I couldn't find a way. The issue was finding a place for the mainmenu select: the only feasible area was within wagtailadmin/shared/header.html and only a limited context is passed there. If you can see a way, let me know.

If we removed the mainmenu select (which would change editor/admin workflow a bit) then I think we could avoid changing any templates, but I considered that sort of change out of scope for this PR.

[ababic]

Thanks for the explanation. Now that FieldPanel has the read_only option, I think we could get away with using that in the edit/create views to indicate the site, and perhaps only redirect straight to there in a single-site setup.

[vbabiy]

Any chance we can get this merged and released? Its blocking wagtail 6 upgrades.

[schlich]

Yes I will take a look

[dkirkham]

Sorry all, I've been a bit busy with other things over recent weeks. There is still some documentation to update, and now that Wagtail 6 is out, I'd like to see if we can get compatibility with that and 5.2 LTS. I'll be working on this later today.

[dkirkham]

I've just added Wagtail 6.0 support - there were only a couple of changes as I had expected. I still need to test this within an existing project of mine, which I'll do tomorrow.

@vbabiy, @schlich and others, have you tried this update in your projects, and have any issues arisen? If not, I think we are close to being able to do a release - just some doco to update.

[vbabiy]

I will give this a quick test and let you know.

[dkirkham]

I tested wagtailmenus in one of my projects this evening, using both Wagtail 5.2 and Wagtail 6.0.1, and haven't detected any issues.

I did however encounter an issue during installation, it seems installing from github didn't reliably remove and change all the package files. I found I had to:

• pip uninstall wagtailmenus
• pip install git+https://github.com/dkirkham/wagtailmenus.git@migrate-modeladmin-snippets

If wagtailmenus is the last dependence you have on Wagtail ModelAdmin, then you can also remove it:

• remove wagtail_modeladmin or wagtail.contrib.modeladmin from your settings/base.py
• pip uninstall wagtail_modeladmin

I'll start working on the documentation this Wednesday, unless someone else wants to start on it earlier.

[vbabiy]

Worked with no issues for me as well.

[schlich]

i'll take y'alls word on when this is ready to release and i can push the pypi button. @MrCordeiro if you have any reservations make 'em known!

[schlich]

If all we're waiting on is docs i'm gonna go ahead and merge, we can merge the docs when theyre ready

[schlich]

Made a follow-up issue for outstanding documentation ^^ clarifications welcome

[schlich]

Opened a PR for a release candidate at #480