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

Accepting a revision without a user causes a server error #7879

Closed
jlchilders11 opened this issue Jan 20, 2022 · 2 comments
Closed

Accepting a revision without a user causes a server error #7879

jlchilders11 opened this issue Jan 20, 2022 · 2 comments
Labels

Comments

@jlchilders11
Copy link

Found a bug? Please fill out the sections below. 👍

Issue Summary

Currently, if you create a revision using page.save_revision(submitted_for_moderation=True) and then attempt to accept the page in the moderation queue, a server error occurs. This is caused by the send_moderation_notifications method in wagtail.admin.mail creating a list with a none object rather than an empty list when there is no user on a revision.

Steps to Reproduce

  1. Start a new project with wagtail start myproject
  2. Enter the Django shell and import the page model.
  3. Execute page.save_revision(submitted_for_moderation=True)
  4. Run python manage.py runserver and login
  5. See your revision in the moderation queue, and attempt to accept the revision.
  6. Note the 500 error

Any other relevant information. For example, why do you consider this a bug and what did you expect to happen instead?

  • I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: (yes / no)

Technical details

  • Python version: 3.9.2
  • Django version: 3.2.9
  • Wagtail version: 2.14.2
  • Browser version: Firefox 96
@jlchilders11
Copy link
Author

@vsalvino Tagging the issue we discussed earlier.

@vsalvino
Copy link
Contributor

vsalvino commented Jan 20, 2022

@jlchilders11 if you get time to make the change we discussed, about handling None authors correctly, I can help shepherd it through and probably get it pushed out in the next patch, since it's a minimal change.

I think rather than using AnonymousUser (our temporary workaround), it would be better to just have an empty list [] and then skip over sending the notifications entirely in that case.

Another option would be to force revisions to have a valid user, which I think would be a much more disruptive change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants