post_save_moderation breaks confirmation view

[nate-django@…]

I tried out the new moderation tools and I found that when a comment is deleted in the post_save signal handler post_save_moderation(), it breaks the comment confirmation view because the comment instance is gone and _get_pk_val() returns None. This causes the following traceback for the redirected request.

File "/var/lib/python-support/python2.5/django/core/handlers/base.py" in get_response
  86.                 response = callback(request, *callback_args, **callback_kwargs)
File "/var/lib/python-support/python2.5/django/contrib/comments/views/utils.py" in confirmed
  41.                 comment = comments.get_model().objects.get(pk=request.GET['c'])
File "/var/lib/python-support/python2.5/django/db/models/manager.py" in get
  93.         return self.get_query_set().get(*args, **kwargs)
File "/var/lib/python-support/python2.5/django/db/models/query.py" in get
  303.         clone = self.filter(*args, **kwargs)
File "/var/lib/python-support/python2.5/django/db/models/query.py" in filter
  489.         return self._filter_or_exclude(False, *args, **kwargs)
File "/var/lib/python-support/python2.5/django/db/models/query.py" in _filter_or_exclude
  507.             clone.query.add_q(Q(*args, **kwargs))
File "/var/lib/python-support/python2.5/django/db/models/sql/query.py" in add_q
  1258.                             can_reuse=used_aliases)
File "/var/lib/python-support/python2.5/django/db/models/sql/query.py" in add_filter
  1201.         self.where.add((alias, col, field, lookup_type, value), connector)
File "/var/lib/python-support/python2.5/django/db/models/sql/where.py" in add
  48.                 params = field.get_db_prep_lookup(lookup_type, value)
File "/var/lib/python-support/python2.5/django/db/models/fields/__init__.py" in get_db_prep_lookup
  202.             return [self.get_db_prep_value(value)]
File "/var/lib/python-support/python2.5/django/db/models/fields/__init__.py" in get_db_prep_value
  353.         return int(value)

Exception Type: ValueError at /comments/posted/
Exception Value: invalid literal for int() with base 10: 'None'

Shouldn't moderation.py connect to the comment_will_be_posted signal so post_comment() can handle the signal response?

[Thejaswi Puthraya]

The patch will delete the comment at the comment_was_posted signal handler and redirect to the comment_done or next page.

[Thejaswi Puthraya]

git-patch against the latest checkout

[nate-django@…]

The patch looks a lot like what I did to fix the issue for my site. My only problem with it is that I think the signal handler should return False so the bad comment never touches the database. But that causes post_comment() to return CommentPostBadRequest which does not have a template when DEBUG == False. So if a comment_will_be_posted signal handler returns False to refuse the comment, the user/client/spammer gets a 400 response and a blank page. I think the 400 is right, but I don't really like the blank page idea.

[Thejaswi Puthraya]

Actually, my confusion was also similar. Whether to allow the comment to be
posted and user given no indication of deletion or give the user
a warning of some kind.

But then, I decided to stick to the former (default) method that
comment-utils had been following for so long.

I believe the latter is very much possible because now the
comment_was_posted signal also passes the request as a kwarg.

Let's see what the devs think about this.

[Jacob]

If you make this change the comment moderators tests start failing. Someone will need to explain why before this can go in.

[nate-django@…]

Replying to jacob:

If you make this change the comment moderators tests start failing. Someone will need to explain why before this can go in.

The testcases create comments using the models. They do not go through the post_comment view which is where the comment_will_be_posted and comment_was_posted signals are dispatched from.

[Thejaswi Puthraya]

fixes the broken tests issue

[Jacob]

I believe this was a duplicate of #11113 and fixed in [10784]. Please reopen if I'm wrong.

[Jacob]

Milestone 1.1 deleted