Fix for COMMENTS_BANNED_USERS_GROUP exploding the ORM when using the templatetag.

[Brett Hoerner <bretthoerner@…>]

Using Django comments with COMMENTS_BANNED_USERS_GROUP drops you into the following branch:

if not self.free and settings.COMMENTS_BANNED_USERS_GROUP:
    kwargs['select'] = {'is_hidden': 'user_id IN (SELECT user_id FROM auth_user_groups WHERE group_id = %s)' % settings.COMMENTS_BANNED_USERS_GROUP}
comment_list = get_list_function(**kwargs).order_by(self.ordering + 'submit_date').select_related()

Which in turn shoves the kwarg 'select' into a filter function, thus making the ORM puke violently. A patch is attached:

comment_list = get_list_function(**kwargs).order_by(self.ordering + 'submit_date').select_related()
if not self.free and settings.COMMENTS_BANNED_USERS_GROUP:
    comment_list = comment_list.extra(select={'is_hidden': 'user_id IN (SELECT user_id FROM auth_user_groups WHERE group_id = %s)' % settings.COMMENTS_BANNED_USERS_GROUP})

Also, the Comment/FreeComment models' get_absolute_url() methods can cause a site to fail loudly if an object is erased and latest comments are still referred to. An example is a "latest comments" rail showing comments on different objects. If one of the objects is removed the following code will cause a rendering error, instead of failing silently. (get_content_object returns None and then get_absolute_url explodes)

def get_absolute_url(self):
    return self.get_content_object().get_absolute_url() + "#c" + str(self.id)

This is a problem because non-developers (template creators and/or admin users) can bring a site to a halt using comments (I believe Django prefers templates to fail silently instead of loudly when they can), I recommend a safe/harmless change such as this:

def get_absolute_url(self):
    try:
        return self.get_content_object().get_absolute_url() + "#c" + str(self.id)
    except AttributeError:
        return ""

[Brett Hoerner <bretthoerner@…>]

get_absolute_url() patch

[Brett Hoerner <bretthoerner@…>]

COMMENTS_BANNED_USERS_GROUP / ORM patch

[Jeremy Dunck <jdunck@…>]

We found one more issue while using comments with 0.96 and settings.COMMENTS_BANNED_USERS_GROUP / settings.COMMENTS_SKETCHY_USERS_GROUP.

I'm attaching a combined patch.

To be clear, the bugs reported here aren't obvious to other people using the comments framework because the framework short-circuits around the problem areas if settings does not include a truth-y related *_USERS_GROUP setting.

There don't seem to be any tests on the comments app, or I'd add tests to cover this particular edge case.

[Jeremy Dunck <jdunck@…>]

Supercedes previous 2 patches.

[Malcolm Tredinnick]

On balance, I think the handling for get_absolute_url() is incorrect: the function is now (after patch applcation) returning the wrong answer in the 'except' case, because no URL is valid. A more correct solution is to not call get_absolute_url on non-existent objects and so the callers need to be examined. However, given that all of this code is slated for replacement prior to 1.0, it's not worth worrying about this at the moment. I'll apply the patch as a stop-gap until the comments rewrite.

[Malcolm Tredinnick]

(In [5848]) Fixed #4189 -- Fixed crashes in a couple of corner cases in the comments app. Not a perfect fix (see ticket), but it will do as a holdover until the new comments framework is in place.

[Jacob]

Milestone 1.0 beta deleted