Improve the comments framework customizability

git-patch against the latest trunk checkout

Yes, it is a feature. The comments work fine without this patch, so it's not a bug fix. It's adding new functionality.

Replying to mtredinnick:

Yes, it is a feature. The comments work fine without this patch, so it's not a bug fix. It's adding new functionality.

But we have no way to customize comments without the patch :(

You're not understanding how those methods are intended to be called: an app that wants to provide custom comments provides its own app; you'd call, for example, get_comments_app().get_form() -- this would return COMMENTS_APP.get_form().

Either way, though, COMMENTS_APP is a hook designed for the future; the goal of comments in 1.0 is to *work*. Extensibility comes next.

In that case, the template tags in django.contrib.comments need fixing to use get_comments_app(). Currently the template tags are hardcoded to call comments.get_form() etc, which means there is no usable customization hook in contrib.comments.

A patch to use get_comment_app in templatetags, so customization can work (but I actually think Thejaswi's patch is a better approach)

Thejaswi's original patch, in svn diff style

Carljm, I actually wanted to sneak this patch before 1.0 but Malcolm and Jacob have a valid point that comments currently work and the extensibility can be taken care of post-1.0. This patch might also require documenting the process of writing custom comments. So a patch just addresses the issue is not sufficient, docs are also vital.

Anyways, let this ticket rest till 1.0 is released :)

Replying to thejaswi_puthraya:

Anyways, let this ticket rest till 1.0 is released :)

Sure - I'm not advocating for this to happen pre-1.0. Just needed to do custom comments for my own project now anyway, so thought I might as well update things here while I was at it. I'm happy to help work on docs when the time comes.

admin.py needs to not register the Comment model if it's been replaced with a custom model

Also removed the checks in get_comment_app() so it's possible to override only part of the functions and use the default for the rest.

I'm ready to write draft docs and put together the patch for this, whenever it gets blessed with Accepted (though there may be a design decision needed first?)

That's a valid issue and needs to be adressed.

consolidated patch

consolidated patch, with docs

Ok, this patch is ready for a looking-over by someone who knows what they're doing. This is my first time trying to actually write any Django docs, so feedback and suggestions are welcome.

A couple other notes on decisions I made in the patch:

- I took Thejaswi's approach of modifying contrib.comments' get_model() etc functions to be smart about delegating to a custom COMMENTS_APP. To me, this seems cleanest, as no other code (ie the templatetags) has to duplicate the get_comment_app() song and dance, it all happens in one place.

- I also incorporated amir's suggestion to make all the customization functions optional, so you can customize what you like and leave the rest.

- No tests yet. I'm not clear where to go with this, as contrib.comments has no tests at all currently. Should I be writing tests for the whole app as part of this ticket? (Seems like maybe that should be its own ticket). Should I just introduce some simple tests for the customizability, and leave everything else untested?

Thoughts and feedback welcome.

Oops - of course the comments app has tests, they're just with the rest of the Django tests, not with the app. Off to add some tests for customization...

consolidated patch with docs and tests

#9562 was a duplicate.

git-patch that has tests, docs and the patch. Slight change from Carljm's patch.

The latest patch is based on carljm's patch with a few changes. Docs are courtesy of carljm. Thanks carljm.

updated patch with minor typo fixes

Thanks Thejaswi. It'd be nice if you could summarize your changes to the patch to make it easier to look over.

I attached an updated version that fixes a typo your latest patch introduced in contrib/comments/init.py in get_delete_url(), and uses forward slashes instead of dot notation to refer to file locations in docs/ref/contrib/comments/custom.txt.

this time with all the files included :-)

(In [9889]) Refactored CommentForm?.get_comment_object into a handful of separete methods to make it easier for subclasses to provide custom models and data. Refs #8630.

(In [9890]) Fixed #8630: finished the custom comment app API that was left out of 1.0. This means it's now possible to override any of the models, forms, or views used by the comment app; see the new custom comment app docs for details and an example. Thanks to Thejaswi Puthraya for the original patch, and to carljm for docs and tests.

Greetings,

I'm trying to take advantage of the newest version of the django's customizable comments framework. I am following all the documentation when I try and implement a simple comment system that has an additional field. I tried initially with a BooleanField?, but have since resorted to trying to simply follow the example from the documentation provided.

When I run it, I get the following error message: Caught an exception while rendering: Cannot resolve keyword 'is_public' into field. Choices are: content_type, id, object_pk, site, title

I suspect that this is a bug or either mistaken documentation, as I'm assuming if I follow the example online character for character it should work.

A couple of things I've found by digging through the code. If we extend the BaseCommentAbstractModel?, how can we receive the Comment model's fields? Why does line 84 of templatetags/comments.py contain is_public = true. This field will only exist if we have extended the Comment model but this isn't an option as we are supposed to extend the BaseCommentAbstractModel?.

Also the unit testing doesn't seem to test the rendering which is where the problems seem to be occurring.

Maybe I'm completely off here, this is my first time posting any comments so if I have broken any posting rules please let me know.

Thanks for your work on this portion.

Josh

This is valid, but should get its own ticket. Created #10548.

Hi Josh,

Thanks for your comments. They have been invaluable and they have helped us locate a bug elsewhere.

Replying to joshvanwyck:

Greetings, I'm trying to take advantage of the newest version of the django's customizable comments framework. I am following all the documentation when I try and implement a simple comment system that has an additional field. I tried initially with a BooleanField?, but have since resorted to trying to simply follow the example from the documentation provided. When I run it, I get the following error message: Caught an exception while rendering: Cannot resolve keyword 'is_public' into field. Choices are: content_type, id, object_pk, site, title I suspect that this is a bug or either mistaken documentation, as I'm assuming if I follow the example online character for character it should work.

Ideally you should be subclassing from the Comment Model. But if your requirements are different, then BaseCommentAbstractModel? should do. This problem was fixed in [9891] but there is a small typo in that.

Will reopen that ticket and get it fixed. Also I will open a ticket for patching the documentation that makes it very clear.

A couple of things I've found by digging through the code. If we extend the BaseCommentAbstractModel?, how can we receive the Comment model's fields? Why does line 84 of templatetags/comments.py contain is_public = true. This field will only exist if we have extended the Comment model but this isn't an option as we are supposed to extend the BaseCommentAbstractModel?. Also the unit testing doesn't seem to test the rendering which is where the problems seem to be occurring. Maybe I'm completely off here, this is my first time posting any comments so if I have broken any posting rules please let me know. Thanks for your work on this portion. Josh