Code

Opened 5 years ago

Closed 16 months ago

#11870 closed Bug (wontfix)

Moderation doesn't work with comments extended from BaseCommentAbstractModel

Reported by: HM Owned by: nobody
Component: contrib.comments Version: master
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)

11870.diff (1010 bytes) - added by thejaswi_puthraya 5 years ago.
git-patch against the latest checkout
11870_2.diff (2.1 KB) - added by thejaswi_puthraya 5 years ago.
git patch against the latest checkout
11870_3.diff (3.5 KB) - added by thejaswi_puthraya 5 years ago.
git patch against the latest checkout with the required tests

Download all attachments as: .zip

Change History (19)

Changed 5 years ago by thejaswi_puthraya

git-patch against the latest checkout

comment:1 Changed 5 years ago by thejaswi_puthraya

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by thejaswi_puthraya

  • Version changed from 1.1 to SVN

comment:3 Changed 5 years ago by thejaswi_puthraya

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:4 Changed 5 years ago by jezdez

Any progress on this?

Changed 5 years ago by thejaswi_puthraya

git patch against the latest checkout

comment:5 Changed 5 years ago by thejaswi_puthraya

  • milestone set to 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 Changed 5 years ago by jezdez

  • Needs tests set

Hm, I can't really think of an alternative, too. But a test of this behaviour would be good.

Changed 5 years ago by thejaswi_puthraya

git patch against the latest checkout with the required tests

comment:7 Changed 4 years ago by ubernostrum

  • milestone 1.2 deleted

1.2 is feature-frozen, moving this feature request off the milestone.

comment:8 Changed 4 years ago by unbracketed

  • Cc brian@… added

comment:9 Changed 4 years ago by jezdez

  • Patch needs improvement set

The last patch didn't run me: https://gist.github.com/f6284636f9edd130bd96

comment:10 Changed 4 years ago by jezdez

Jesus, of course the patch didn't run me, but it didn't run *for* me, too :)

comment:11 Changed 4 years ago by thejaswi_puthraya

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 Changed 4 years ago by thejaswi_puthraya

As per a discussion with Jannis and Russell, this ticket needs to wait till LazyForeignKey or some equivalent.

comment:13 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:14 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:15 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:16 Changed 16 months ago by jacob

  • Resolution set to wontfix
  • Status changed from new to 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.