Inheriting from comments breaks comment moderation

[Beetle_B]

I created a new kind of comment called TreeComment which (ultimately) subclasses Comment (in contrib.comments)

Essentially, I have a TreeCommentBase which subclasses Comment and is an abstract class. Then TreeComment subclasses TreeCommentBase. This is all in a custom app called treecomments.

Now in the moderation code, we have:

        signals.comment_will_be_posted.connect(self.pre_save_moderation, sender=comments.get_model())
        signals.comment_was_posted.connect(self.post_save_moderation, sender=comments.get_model())

which relies on get_model() which is defined in the init.py file in comments. This causes a wrong model to be sent (my model is TreeComment, not Comment).

If I replace it with treecomments.get_model() (and create that function under init.py - I needed it for something else anyway), then it all works.

Am I doing something wrong? Is there a more proper way of subclassing? For now I'll just subclass Moderator and customize to fix this...

[Karen Tracey]

This page: ​http://docs.djangoproject.com/en/dev/ref/contrib/comments/custom/

appears to document the correct way to get your own model returned by that comments.get_model() call.

[Beetle_B]

Thanks for pointing it out, but it still didn't work.

In my settings.py file, I put:

COMMENTS_APP = 'treecomments'

I had already created the get_model() in init.py for treecomments.

Yet, it doesn't work if I simply subclass CommentModerator and import moderator from django.contrib.comments.moderation. The funny thing is that if I simply copy and paste the moderation.py file into my models.py file, everything works fine: It correctly detects the new comments app. Could I be handling some imports incorrectly?

[Karen Tracey]

I cannot recreate any problem with the proper model being used in the above-noted moderation signal connections by following the documented method pointed to above. You haven't really provided enough information to recreate whatever you are seeing, and at this point the problem seems better suited to django-users than a ticket. As near as I can tell there is documented way to do what you are asking to do, and in my testing it works. There is also a test of comments.get_model() returning the correct model in the comment tests, and that is passing.

If you'd like to pursue this further please follow up on django-users with enough specifics of your models and settings to allow someone to recreate the issue, and also indicate how you are observing the wrong model being returned by comments.get_model(). I tested it by putting a breakpoint in my custom comment app get_model() and observing that it was hit twice when I entered:

from django.contrib.comments.moderation import moderator

from a manage.py shell. From the stack trace I could see it was hit once for each of those signal connection lines noted above.

[bjourne]

Reopening because I stumbled upon the exact same problem. Basically, it is almost like a race condition. When the django.contrib.comments.moderation module is imported, the Moderator class is instantiated which causes the connect() method to be called. The connect() method calls comments.get_model() which returns an instance of django.contrib.comments.models.Comment which causes the bug. It happens before python has evaluated your init.py file which contains the get_model() function definition. So the signal connection code has to be somehow delayed until after Python has evaluated init.py.

[Tobias McNulty]

A brief review tells me that you're subclassing the wrong thing. Like kmtracey said, please raise this issue on django-users, if you haven't already. Feel free to re-open the ticket if you've confirmed there that it's a bug.

​http://docs.djangoproject.com/en/dev/ref/contrib/comments/custom/#django.contrib.comments.get_model

[Phil Gyford]

I think there's something up here. I've created a testcase that's as simple as I can make it to demonstrate the situation I've come across that seems to cause this error.

This project is an extremely simple weblog, with comments that are extended from django.contrib.comments: ​http://github.com/philgyford/django-commentstest/

As it is, the moderation works fine - if enable_comments on an Entry is set to false, then posting a comment is prevented.

But if you add "from weblog.models import Entry" to either customcomments/forms.py or customcomments/models.py, then moderation is ignored, and comments can always be posted. (Why would you add this? Because you might want to extend the comment class to use something from the Entry model.)

Checking what's happening in django/contrib/comments/init.py's get_model() function, in the first case it's returning a CommentWithTitle class. But in the second case, when moderation isn't working, it's returning a standard Comment class.

I'm too new to Django to know how to proceed with this, and whether this really is a bug or it's something I've done, but after a couple of days of poking at this (and posting to django-users today), I'm stumped.

[Simon Meers]

Thank you for the example project philgyford. I have confirmed your reported (mis)behaviour.

When contrib.comments attempts to import your custom comments app and check for its get_model attribute, for example, your custom __init__.py imports the custom models.py/forms.py -- if either of those import something from weblog.models, you're then back to importing moderation, so the custom comments app imports with only ['__builtins__', '__doc__', '__file__', '__name__', '__package__', '__path__'] -- no get_model or get_form. Which means the moderator defaults to the standard Comment model, etc.

For now I'll fix this by adding documentation warning developers to avoid cyclic imports involving moderation when developing custom comment apps. When smarter registration mechanisms (e.g. startup.py) land, cyclic moderation-registration imports should be quite easily avoided.

[Simon Meers]

(In [14810]) Fixed #12812 -- added warning about cyclic imports in contrib.comments. Thanks to Beetle_B, bjourne and philgyford for the reports, and Russ for the wording.

[Simon Meers]

(In [14811]) [1.2.X] Fixed #12812 -- added warning about cyclic imports in contrib.comments. Thanks to Beetle_B, bjourne and philgyford for the reports, and Russ for the wording.

Backport of r14810 from trunk.