Cannot delete attachments

[Jakob Kramer]

Attachments cannot be deleted without deleting the whole "medium."

[Christopher Allan Webber]

Gandaro, are you working on this? I'm switching the state to in_progress, but please unclaim if not the case.

[digital-dreamer]

Hi, I added this functionality, and it seems to work well, but the main repository on gitorious doesn't allow merge requests, so how exactly does one contribute to it? I am new to the project. The main page says to submit contributions via this ticket tracker. So, it is in my repository mediagoblin/digital-dreamer-mediagoblin on gitorious in the branch 465-delete-attachment. Please check it out and send me a message in case it still needs some improvements.

[digital-dreamer]

PS: I see you use patch files instead of merge requests, so I added it as a patch to this ticket's attachments.

[Christopher Allan Webber]

This looks useful... someone needs to test this patch, but it looks promising at least.

[Matt Molyneaux]

The above patch doesn't add any unit tests (unless I'm missing something?).

In particular, the delete method on MediaAttachmentFile and the media_confirm_delete_attachment view should be tested to make sure they actually do what is expected.

[digital-dreamer]

moggers87 - I forgot to write the unit tests. Here they are:

[Matt Molyneaux]

I've pushed the above patches (and a commit to fix whitespace errors!!!) to ​https://gitorious.org/mediagoblin/moggers87-mediagoblin/commits/issue-465>

Tests pass, but I haven't properly reviewed them yet.

[berkerpeksag]

Reviewing ​https://gitorious.org/mediagoblin/moggers87-mediagoblin/commit/e8c9c20fa07db417610d2aa8579331629d11dd26:

In mediagoblin/db/models.py:

• except OSError, error: should be except OSError as error:
• 'Deleted Attachment entry id "{0}"'.format(self.id): You could write {0} as {}.

In mediagoblin/tests/test_attachments.py:

• Please regroup imports like this:

# stdlib imports

# third party imports

# mediagoblin import

• class TestAttachments: -> class TestAttachments(object):
• assert this_storage.file_exists(attachment_path) == True: Drop == True
• attachment.delete();: Drop ;
• assert this_storage.file_exists(attachment_path) == False -> assert not this_storage.file_exists(attachment_path)

In mediagoblin/user_pages/views.py:

• Please do not mix tabs and spaces.
• if form.confirm.data is True: -> if form.confirm.data:
• if ((request.user.has_privilege(u'admin') and request.user.id != media.uploader)): -> if (request.user.has_privilege(u'admin') and request.user.id != media.uploader)