Opened 12 years ago

Last modified 4 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:


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 Christopher Allan Webber, 12 years ago

Component: component1programming

comment:2 by ShawnRisk, 12 years ago

Owner: somebody removed
Status: newassigned

comment:3 by pythonsnake, 11 years ago

Owner: set to pythonsnake
Status: newassigned

comment:4 by pythonsnake, 11 years ago

or maybe even report comments!

comment:5 by pythonsnake, 11 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:7 by Christopher Allan Webber, 11 years ago

Keywords: review removed
Owner: pythonsnake removed
Status: assignedreview

comment:8 by Christopher Allan Webber, 11 years ago

Owner: set to pythonsnake
Status: reviewin_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:9 by pythonsnake, 11 years ago

Working on it.

comment:10 by ShawnRisk, 10 years ago

Keywords: test added

comment:11 by ShawnRisk, 4 years ago

Keywords: review added

comment:12 by ShawnRisk, 4 years ago

Owner: pythonsnake removed
Status: in_progressreview
Note: See TracTickets for help on using tickets.