Opened 13 years ago
Last modified 5 years ago
#414 review enhancement
Ability to delete one's own comments.
Reported by: | Aleksej | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | programming | Keywords: | test, review |
Cc: | Parent Tickets: |
Description
There is currently no way to remove one's own comments.
Maybe the feature should be optional or customizable.
Change History (12)
comment:1 by , 12 years ago
Component: | component1 → programming |
---|
comment:2 by , 12 years ago
Owner: | removed |
---|---|
Status: | new → assigned |
comment:3 by , 12 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:4 by , 12 years ago
comment:5 by , 12 years ago
Update: this is done, but I need to update it to work with a recent MediaGoblin? version (my hacking instance is a bit out-of-date).
comment:6 by , 12 years ago
Keywords: | review added |
---|
comment:7 by , 12 years ago
Keywords: | review removed |
---|---|
Owner: | removed |
Status: | assigned → review |
comment:8 by , 12 years ago
Owner: | set to |
---|---|
Status: | review → in_progress |
Heya pythonsnake, sorry this took so long to review. This looks like a really good start and actually very close. But I'm passing it back to you, with the following comments:
- It looks like
mediagoblin/user_pages/comment_confirm_delete.html
wasn't checked in? The feature isn't working for me. - It's missing unit tests, those would be really helpful if you can find the time: both to confirm that a user can delete things, and that it fails appropriately if it wasn't the original author of the comment or an admin.
- I think the suggestion on the
/u/cwebber/m/foo-media/c/13/confirm-delete/
style URL was my suggestion. The one issue I have with this is that we're currently just pulling the comment id out and discarding the username and media slug bits. We should either also be checking against those to be sure or we should move this to a simpler URL like/c/13/confirm-delete/
probably? - There's more changes to the .comment_wrapper class CSS than I would expect (at most I'd expect making room for the X; I'm a bit confused why the background is black and etc).
Nonetheless this all looks very close. Thanks for working on it, I'd really love to see this get in! :)
comment:10 by , 11 years ago
Keywords: | test added |
---|
comment:11 by , 5 years ago
Keywords: | review added |
---|
comment:12 by , 5 years ago
Owner: | removed |
---|---|
Status: | in_progress → review |
or maybe even report comments!