Opened 9 years ago

Closed 7 years ago

#17097 closed New feature (wontfix)

Permission to delete a comment should be based on a user's given permissions

Reported by: george@… Owned by: nobody
Component: contrib.comments Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no


The comments app in contrib decides that only superusers are allowed the privilege of deleting comments (line 31). Instead I think it should check the permissions of the user to see if they are allowed.

Attachments (4)

comments.patch (760 bytes) - added by george@… 9 years ago.
Patch file
comments.2.patch (747 bytes) - added by ghickman 9 years ago.
tests.patch (1.4 KB) - added by George Hickman <george@…> 9 years ago.
tests for the comments permissions
17097.patch (6.3 KB) - added by George Hickman <george@…> 9 years ago.
Full patch

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by george@…

Attachment: comments.patch added

Patch file

comment:1 Changed 9 years ago by Julien Phalip

Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedAccepted
Type: BugNew feature

I agree that it would make sense to use the delete permissions here. This would need some tests. Also, a note on the patch: ModelAdmin.has_delete_permission() should probably be used instead of hardcoding the permission name.

Finally, this would be backwards-incompatible and would require a mention in the release notes.

Changed 9 years ago by ghickman

Attachment: comments.2.patch added

Changed 9 years ago by George Hickman <george@…>

Attachment: tests.patch added

tests for the comments permissions

comment:2 Changed 9 years ago by George Hickman <george@…>

I've added a patch using ModelAdmin.has_delete_permission() in and a couple of pretty basic tests for user's permissions when set on a group. They'll need expanding though, as the patch isn't tested yet (bit short on time right now) but hopefully that's a start anyway.

Changed 9 years ago by George Hickman <george@…>

Attachment: 17097.patch added

Full patch

comment:3 Changed 9 years ago by George Hickman <george@…>

Added a full patch with the code and tests. I've focused on testing the permissions for a user and the actions available in that list.

comment:4 Changed 9 years ago by Julien Phalip

Patch needs improvement: set

Thanks a lot for your work on this patch!

However, I now think that we should aim for a more general fix. Ideally, if the user didn't have delete permissions, then the 'delete_selected' action wouldn't show up -- this applies not just to comments, but to any model. This general issue is addressed in #10609. I've just remembered about that ticket and I apologise for not letting you know about it earlier.

So, to summarise, I would prefer to fix #10609 first, and then clean up CommentsAdmin.get_actions(). Many thanks again!

comment:5 Changed 7 years ago by Jacob

Resolution: wontfix
Status: newclosed

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; if you want to keep using it, you could move this bug over there.

Note: See TracTickets for help on using tickets.
Back to Top