From: Sebastian Spaeth <Sebastian@SSpaeth.de>
Date: Thu, 15 Nov 2012 10:44:50 +0000 (+0100)
Subject: Implement MediaEntry().delete() (#540)
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=fdc34b8ba7465b528c7685be3f24c7d7d6d8748b;p=mediagoblin.git

Implement MediaEntry().delete() (#540)

Deleting a MediaEntry instance will automatically
delete all related comments and files/attachments. This moves
implementation logic out of views.py and allows to make use of this
functionality when e.g. deleting a User() account.

Whenever a MediaEntry entry is deleted, this will also sql-delete
the corresponding MediaFile entry.

Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
---

diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py
index ea915ae5..aeec8aea 100644
--- a/mediagoblin/db/models.py
+++ b/mediagoblin/db/models.py
@@ -18,7 +18,7 @@
 TODO: indexes on foreignkeys, where useful.
 """
 
-
+import logging
 import datetime
 import sys
 
@@ -34,6 +34,7 @@ from sqlalchemy.util import memoized_property
 from mediagoblin.db.extratypes import PathTupleWithSlashes, JSONEncoded
 from mediagoblin.db.base import Base, DictReadAttrProxy, Session
 from mediagoblin.db.mixin import UserMixin, MediaEntryMixin, MediaCommentMixin, CollectionMixin, CollectionItemMixin
+from mediagoblin.tools.files import delete_media_files
 
 # It's actually kind of annoying how sqlalchemy-migrate does this, if
 # I understand it right, but whatever.  Anyway, don't remove this :P
@@ -42,6 +43,8 @@ from mediagoblin.db.mixin import UserMixin, MediaEntryMixin, MediaCommentMixin,
 # this import-based meddling...
 from migrate import changeset
 
+_log = logging.getLogger(__name__)
+
 
 class User(Base, UserMixin):
     """
@@ -122,7 +125,6 @@ class MediaEntry(Base, MediaEntryMixin):
         )
 
     attachment_files_helper = relationship("MediaAttachmentFile",
-        cascade="all, delete-orphan",
         order_by="MediaAttachmentFile.created"
         )
     attachment_files = association_proxy("attachment_files_helper", "dict_view",
@@ -131,7 +133,7 @@ class MediaEntry(Base, MediaEntryMixin):
         )
 
     tags_helper = relationship("MediaTag",
-        cascade="all, delete-orphan"
+        cascade="all, delete-orphan" # should be automatically deleted
         )
     tags = association_proxy("tags_helper", "dict_view",
         creator=lambda v: MediaTag(name=v["name"], slug=v["slug"])
@@ -216,6 +218,37 @@ class MediaEntry(Base, MediaEntryMixin):
                 id=self.id,
                 title=safe_title)
 
+    def delete(self, del_orphan_tags=True, **kwargs):
+        """Delete MediaEntry and all related files/attachments/comments
+
+        This will *not* automatically delete unused collections, which
+        can remain empty...
+
+        :param del_orphan_tags: True/false if we delete unused Tags too
+        :param commit: True/False if this should end the db transaction"""
+        # User's CollectionItems are automatically deleted via "cascade".
+        # Delete all the associated comments
+        for comment in self.get_comments():
+            comment.delete(commit=False)
+
+        # Delete all related files/attachments
+        try:
+            delete_media_files(self)
+        except OSError, error:
+            # Returns list of files we failed to delete
+            _log.error('No such files from the user "{1}" to delete: '
+                       '{0}'.format(str(error), self.get_uploader))
+        _log.info('Deleted Media entry id "{0}"'.format(self.id))
+        # Related MediaTag's are automatically cleaned, but we might
+        # want to clean out unused Tag's too.
+        if del_orphan_tags:
+            # TODO: Import here due to cyclic imports!!!
+            #       This cries for refactoring
+            from mediagoblin.db.util import clean_orphan_tags
+            clean_orphan_tags(commit=False)
+        # pass through commit=False/True in kwargs
+        super(MediaEntry, self).delete(**kwargs)
+
 
 class FileKeynames(Base):
     """
diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py
index f115c3b8..1af04e7f 100644
--- a/mediagoblin/user_pages/views.py
+++ b/mediagoblin/user_pages/views.py
@@ -23,7 +23,6 @@ from mediagoblin.db.models import (MediaEntry, Collection, CollectionItem,
 from mediagoblin.tools.response import render_to_response, render_404, redirect
 from mediagoblin.tools.translate import pass_to_ugettext as _
 from mediagoblin.tools.pagination import Pagination
-from mediagoblin.tools.files import delete_media_files
 from mediagoblin.user_pages import forms as user_forms
 from mediagoblin.user_pages.lib import send_comment_email
 
@@ -268,21 +267,7 @@ def media_confirm_delete(request, media):
     if request.method == 'POST' and form.validate():
         if form.confirm.data is True:
             username = media.get_uploader.username
-
-            # Delete all the associated comments
-            for comment in media.get_comments():
-                comment.delete()
-
-            # Delete all files on the public storage
-            try:
-                delete_media_files(media)
-            except OSError, error:
-                _log.error('No such files from the user "{1}"'
-                           ' to delete: {0}'.format(str(error), username))
-                messages.add_message(request, messages.ERROR,
-                                     _('Some of the files with this entry seem'
-                                       ' to be missing.  Deleting anyway.'))
-
+            # Delete MediaEntry and all related files, comments etc.
             media.delete()
             messages.add_message(
                 request, messages.SUCCESS, _('You deleted the media.'))