Skip to content

Fix 404 viewing form submissions in admin on Wagtail 7.x (#710)#717

Open
bradrice wants to merge 2 commits into
coderedcorp:mainfrom
bradrice:fix-form-submissions-404-formmixin
Open

Fix 404 viewing form submissions in admin on Wagtail 7.x (#710)#717
bradrice wants to merge 2 commits into
coderedcorp:mainfrom
bradrice:fix-form-submissions-404-formmixin

Conversation

@bradrice

Copy link
Copy Markdown

Fixes #710

Problem

On Wagtail 7.x, viewing form submissions in the Wagtail admin returns a 404. The Forms index lists the forms, but clicking through to a form's submissions (/admin/forms/submissions/<id>/) fails.

Wagtail 7.x hardened its admin submissions view to guard on the form type and use the new API:

form_page = get_object_or_404(Page, id=page_id).specific
if not isinstance(form_page, FormMixin):   # CodeRed pages fail this
    raise Http404
return form_page.serve_submissions_list_view(request, ...)
# ...and SubmissionsListView.get_base_queryset() now calls form_page.get_submissions()

CodeRed's CoderedFormMixin / form pages do not inherit Wagtail's wagtail.contrib.forms.models.FormMixin, so both the isinstance guard and the get_submissions() call fail.

Fix

Have CoderedFormPage and CoderedStreamFormPage inherit Wagtail's FormMixin as the last base:

class CoderedFormPage(CoderedFormMixin, CoderedWebPage, FormMixin): ...
class CoderedStreamFormPage(CoderedFormMixin, CoderedStreamFormMixin, CoderedWebPage, FormMixin): ...

Why last, and why on the page classes rather than CoderedFormMixin:

  • FormMixin is a fieldless plain mixinno migration (makemigrations --check reports No changes detected).
  • Placing it last in the MRO means it only supplies the two methods CodeRed lacks (get_submissions(), get_submissions_list_view_class()) and satisfies the isinstance check — it never shadows CodeRed's own methods.
  • Putting it on CoderedFormMixin instead would insert FormMixin ahead of wagtail_flexible_forms.StreamFormMixin in CoderedStreamFormPage's MRO, so FormMixin.get_form_fields() (which does self.form_fields.all()) would shadow the StreamField handling and break advanced forms ('StreamValue' object has no attribute 'all'). Inheriting on the page classes, last, avoids that.

Method resolution after the change:

Page isinstance(.., FormMixin) get_data_fields get_submissions
CoderedFormPage CoderedFormPage (unchanged) FormMixin
CoderedStreamFormPage StreamFormMixin (unchanged) FormMixin

Tests

Adds coderedcms/tests/test_forms.py covering the admin forms index and the submissions view for both the standard and advanced (StreamField) form pages. These tests fail without the fix (both submission views 404) and pass with it.

  • Full suite: 72 passed
  • ruff check / ruff format --check: clean
  • codespell: clean
  • makemigrations --check: No changes detected

Docs

Adds docs/releases/v6.0.1.rst (backwards-compatible bug fix → patch) and links it in the releases index.

…#710)

Wagtail 7.x guards its admin submissions view with
`isinstance(page, wagtail.contrib.forms.models.FormMixin)` and now calls
`page.get_submissions()`. CodeRed's form pages did not inherit Wagtail's
FormMixin, so viewing submissions for any CodeRed form 404'd.

Have CoderedFormPage and CoderedStreamFormPage inherit FormMixin as the
last base. FormMixin is a fieldless plain mixin, so no migration is needed;
placing it last means it only supplies the get_submissions() /
get_submissions_list_view_class() methods CodeRed lacks and satisfies the
isinstance check, without shadowing CodeRed's or wagtail-flexible-forms'
existing methods (notably StreamForm field handling).

Add a regression test covering the admin forms index and the submissions
view for both the standard and advanced (StreamField) form pages; these
fail without the fix. Add release notes for v6.0.1.

Fixes coderedcorp#710
Pre-existing spelling error flagged by codespell 2.4.3 in the Static
Analysis CI job; unrelated to the form-submissions fix but included so
the linters check passes on this PR.
@bradrice

Copy link
Copy Markdown
Author

Notes on the first CI run:

  • Unit tests pass on all matrix jobs (py3.10–3.13, basic + pro templates).
  • Linters / Spelling failed on a pre-existing codespell error in docs/getting_started/tutorial05.rst:106 ("pre-selected"), which is present on main and unrelated to this change (codespell 2.4.3 now flags it). I've included a one-line commit fixing that word so the linters job passes — happy to drop it into a separate PR if you'd prefer to keep this one strictly scoped.
  • Code Coverage failed in compare-codecov.ps1. This PR only adds code (a regression test covering the admin submissions path), so it shouldn't reduce coverage; this looks like the comparison flakiness the script itself notes, possibly amplified by the fork-PR baseline. Please re-run if needed.

The functional change is unaffected by either.

@bradrice

Copy link
Copy Markdown
Author

Update after the latest run (build 5158): unit tests, linters (spelling now fixed), and docs all pass. ✅

The only remaining red check is Code Coverage, and the log confirms it's not a coverage regression — compare-codecov.ps1 fails on the Invoke-WebRequest call that fetches the main baseline from the Azure DevOps API (it never reaches the comparison). That looks like a fork-PR API-access limitation rather than anything in this change. A maintainer re-run from within the repo (or a manual override) should clear it.

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.

Form submissions return 404 with Wagtail 7.x due to FormMixin isinstance check

1 participant