Comment templatetags should be independent of Comment Model

[Thejaswi Puthraya]

Hi,
I was going through the patch by carljm for #8630 [1] and decided to
give it a try. I ran into troubles but not with the patch. The reason
being comment templatetags reference 'is_public' [2] and
'is_removed' [3] fields of the Comment Model.

The idea for comments customization was to push essential fields onto
BaseCommentAbstractModel and then inherit from this model. This would
reduce the need to rewrite templatetags but I screwed this while
sending patches just before the merge into trunk.

I can think of two solutions to solve the problem. The first one being
http://dpaste.com/82448/ and the second one (http://dpaste.com/82449/)
to push the 'is_public' and 'is_removed' fields onto the
BaseCommentAbstract Model. Both these changes are backward-compatible.
I prefer the first method but would love to hear from you.

I have opened a ticket for this at #9303 [4]. Please do post your
thought either here or on this thread.

[1] http://code.djangoproject.com/ticket/8630
[2] http://code.djangoproject.com/browser/django/trunk/django/contrib/comments/templatetags/comments.py#L83
[3] http://code.djangoproject.com/browser/django/trunk/django/contrib/comments/templatetags/comments.py#L86
[4] http://code.djangoproject.com/ticket/9303

PS: I have made quite a few mistakes and I am really sorry about them
but if you believe I deserve to be spanked, please do ;-)

--
Cheers
Thejaswi Puthraya

[Malcolm Tredinnick]

On Sat, 2008-10-04 at 21:36 -0700, Thejaswi Puthraya wrote:
> Hi,
> I was going through the patch by carljm for #8630 [1] and decided to
> give it a try. I ran into troubles but not with the patch. The reason
> being comment templatetags reference 'is_public' [2] and
> 'is_removed' [3] fields of the Comment Model.
>
> The idea for comments customization was to push essential fields onto
> BaseCommentAbstractModel and then inherit from this model. This would
> reduce the need to rewrite templatetags but I screwed this while
> sending patches just before the merge into trunk.
>
> I can think of two solutions to solve the problem. The first one being
> http://dpaste.com/82448/ and the second one (http://dpaste.com/82449/)
> to push the 'is_public' and 'is_removed' fields onto the
> BaseCommentAbstract Model. Both these changes are backward-compatible.

The second approach does introduce an incompatibility. Anybody who has
created a model using that ABC would now have to alter their tables.
It's not part of the guaranteed stable API, so it's not impossible to
change, but does introduce a backwards-incompatible change.

For that reason I would slightly prefer the first approach at the
moment. But I want to think a bit more about what the common fields for
any comment customisations should be. I don't have a clear idea about
that in my head at the moment.

Regards,
Malcolm