Opened 15 years ago
Closed 12 years ago
#11870 closed Bug (wontfix)
Moderation doesn't work with comments extended from BaseCommentAbstractModel
Reported by: | HM | Owned by: | nobody |
---|---|---|---|
Component: | contrib.comments | Version: | dev |
Severity: | Normal | Keywords: | moderation |
Cc: | brian@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
CommentFlag has a hardcoded foreign key to django.contrib.comments.models.Comment and thus the moderation-bits of django.contrib.comments cannot be used when inheriting directly from BaseCommentAbstractModel.
Either this should be mentioned in the documentation or CommentFlag should have a
import comments ... comment = models.ForeignKey(comments.get_model(), verbose_name=_('comment'), related_name="flags")
instead of
comment = models.ForeignKey(Comment, verbose_name=_('comment'), related_name="flags")
Attachments (3)
Change History (19)
by , 15 years ago
Attachment: | 11870.diff added |
---|
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 15 years ago
Version: | 1.1 → SVN |
---|
comment:3 by , 15 years ago
The patch uploaded by me doesn't work because it goes into a cyclical import and fails.
Sorry for uploading that patch without verifying. Underestimated this issue...
comment:5 by , 15 years ago
milestone: | → 1.2 |
---|
The latest patch works but it does late module loading ie the imports are called within an if...else statement. This is discouraged in PEP-8 and django's contributing guide but is the only way it can be solved. Every other method I could think of results in a recursive ImportError.
comment:6 by , 15 years ago
Needs tests: | set |
---|
Hm, I can't really think of an alternative, too. But a test of this behaviour would be good.
by , 15 years ago
Attachment: | 11870_3.diff added |
---|
git patch against the latest checkout with the required tests
comment:7 by , 15 years ago
milestone: | 1.2 |
---|
1.2 is feature-frozen, moving this feature request off the milestone.
comment:8 by , 14 years ago
Cc: | added |
---|
comment:9 by , 14 years ago
Patch needs improvement: | set |
---|
The last patch didn't run me: https://gist.github.com/f6284636f9edd130bd96
comment:10 by , 14 years ago
Jesus, of course the patch didn't run me, but it didn't run *for* me, too :)
comment:11 by , 14 years ago
Jannis, This patch works but the settings.py should have the COMMENTS_APP attribute set before the tests are run. The toggling of the COMMENTS_APP attribute in the CustomCommentTest class (present under comment_tests/tests/app_api_tests.py) does not help, because syncdb would've already taken place as soon as the test started and in the process, the evaluation of get_model function in the CommentFlag model.
The problem of setting the COMMENTS_APP to the custom comments explicitly before the tests are run will cause all the default comment tests to fail, unless there are some changes made the CommentTestCase class' setUp method.
What do you suggest? Do we require a test for this or modifying the comment tests?
comment:12 by , 14 years ago
As per a discussion with Jannis and Russell, this ticket needs to wait till LazyForeignKey or some equivalent.
comment:13 by , 13 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:16 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
django.contrib.comments
has been deprecated and is no longer supported, so I'm closing this ticket. We're encouraging users to transition to a custom solution, or to a hosted solution like Disqus.
The code itself has moved to https://github.com/django/django-contrib-comments; if you want to keep using it, you could move this bug over there.
git-patch against the latest checkout