Fix #5376 - Ensure links have correct ID

This ensures that links to comments have the correct ID (the
ID of the Comment object) as well as fixing deletion on reports
and fixing a few other little things. I hope this fixes the #5376
issue, though cannot reproduce so unable to confirm.

diff --git a/mediagoblin/db/base.py b/mediagoblin/db/base.py
index 0f17a3a8d36fbe96f25be1240a93de249bce58d1..c59b0ebf732d5b8be67c8ff8cce44300239ddb4d 100644 (file)
--- a/mediagoblin/db/base.py
+++ b/mediagoblin/db/base.py
@@ -96,7 +96,7 @@ class GMGTableBase(object):
         # cause issues if it isn't. See #5382.
         # Import here to prevent cyclic imports.
         from mediagoblin.db.models import CollectionItem, GenericModelReference, \
-                                          Report, Notification
+                                          Report, Notification, Comment
         
         # Some of the models don't have an "id" field which means they can't be
         # used with GMR, these models won't be in collections because they
@@ -123,6 +123,12 @@ class GMGTableBase(object):
                 )
                 notifications.delete()
                 
+                # Delete this as a comment
+                comments = Comment.query.filter_by(
+                    comment_id=gmr.id
+                )
+                comments.delete()
+
                 # Set None on reports found
                 reports = Report.query.filter_by(
                     object_id=gmr.id

diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py
index 3f36f2279273b15b4d0b722d1129d9473d8c6a89..5393f6791d1ab33792c249ad47da739817d32568 100644 (file)
--- a/mediagoblin/db/models.py
+++ b/mediagoblin/db/models.py
@@ -609,7 +609,7 @@ class MediaEntry(Base, MediaEntryMixin, CommentingMixin):
         else:
             query = query.order_by(Comment.added.desc())
         
-        return FakeCursor(query, lambda c:c.comment())
+        return query
  
     def url_to_prev(self, urlgen):
         """get the next 'newer' entry by this user"""
@@ -778,7 +778,7 @@ class MediaEntry(Base, MediaEntryMixin, CommentingMixin):
 
         if show_comments:
             comments = [
-                comment.serialize(request) for comment in self.get_comments()]
+                l.comment().serialize(request) for l in self.get_comments()]
             total = len(comments)
             context["replies"] = {
                 "totalItems": total,
@@ -1008,17 +1008,6 @@ class TextComment(Base, TextCommentMixin, CommentingMixin):
                                               cascade="all, delete-orphan"))
     deletion_mode = Base.SOFT_DELETE
 
-    def soft_delete(self, *args, **kwargs):
-        # Find the GMR for this model.
-        gmr = GenericModelReference.query.filter_by(
-            obj_pk=self.id,
-            model_type=self.__tablename__
-        ).first()
-
-        # Delete the Comment object for this comment
-        Comment.query.filter_by(comment_id=gmr.id).delete()
-        return super(TextComment, self).soft_delete(*args, **kwargs)
-
     def serialize(self, request):
         """ Unserialize to python dictionary for API """
         target = self.get_reply_to()

diff --git a/mediagoblin/moderation/tools.py b/mediagoblin/moderation/tools.py
index 73afd05159c5b617626f6b5216ffaa2f6be77122..36d89d718ddb45b87be67b72d8cc6b63140e4891 100644 (file)
--- a/mediagoblin/moderation/tools.py
+++ b/mediagoblin/moderation/tools.py
@@ -69,7 +69,7 @@ def take_punitive_actions(request, form, report, user):
     if u'delete' in form.action_to_resolve.data and \
         report.is_comment_report():
             deleted_comment = report.obj()
-            Session.delete(deleted_comment)
+            deleted_comment.delete()
             form.resolution_content.data += \
                 _(u"\n{mod} deleted the comment.").format(
                     mod=request.user.username)

diff --git a/mediagoblin/notifications/__init__.py b/mediagoblin/notifications/__init__.py
index 3ed9ba79aa4536af673fedfe93523beb5b21a872..9d2f2b7835e86b686d316b9378960f4f9ec4b4c1 100644 (file)
--- a/mediagoblin/notifications/__init__.py
+++ b/mediagoblin/notifications/__init__.py
@@ -75,6 +75,11 @@ def mark_comment_notification_seen(comment_id, user):
         obj_pk=comment.id,
         model_type=comment.__tablename__
     ).first()
+
+    # If there is no GMR, there is no notification
+    if comment_gmr == None:
+        return
+
     notification = Notification.query.filter_by(
         user_id=user.id,
         object_id=comment_gmr.id

diff --git a/mediagoblin/templates/mediagoblin/user_pages/media.html b/mediagoblin/templates/mediagoblin/user_pages/media.html
index 1a35414fd289a119bdbe58165115c44a53baaf2f..48a91ab77f771d04abc56729042a638ad31cf318 100644 (file)
--- a/mediagoblin/templates/mediagoblin/user_pages/media.html
+++ b/mediagoblin/templates/mediagoblin/user_pages/media.html
@@ -123,7 +123,8 @@
       {% endif %}
       <ul style="list-style:none">
       {% for comment in comments %}
-        {% set comment_author = comment.get_actor %}
+        {% set comment_object = comment.comment() %}
+        {% set comment_author = comment_object.get_actor %}
         <li id="comment-{{ comment.id }}"
           {%- if pagination.active_id == comment.id %}
             class="comment_wrapper comment_active">
@@ -143,15 +144,15 @@
                             user=media.get_actor.username,
                             media=media.slug_or_id) }}#comment"
                class="comment_whenlink">
-              <span title='{{- comment.created.strftime("%I:%M%p %Y-%m-%d") -}}'>
-                {%- trans formatted_time=timesince(comment.created) -%}
+              <span title='{{- comment_object.created.strftime("%I:%M%p %Y-%m-%d") -}}'>
+                {%- trans formatted_time=timesince(comment_object.created) -%}
                   {{ formatted_time }} ago
                 {%- endtrans -%}
               </span></a>:
           </div>
           <div class="comment_content">
             {% autoescape False -%}
-              {{ comment.content_html }}
+              {{ comment_object.content_html }}
             {%- endautoescape %}
           </div>
           <div>
@@ -159,7 +160,7 @@
                 <a href="{{ request.urlgen('mediagoblin.user_pages.media_home.report_comment',
                             user=media.get_actor.username,
                              media=media.slug_or_id,
-                             comment=comment.id) }}">
+                             comment=comment_object.id) }}">
                     {% trans %}Report{% endtrans %}</a>
             {% endif %}
           </div>

diff --git a/mediagoblin/tests/test_api.py b/mediagoblin/tests/test_api.py
index 33b9320860d45e53d5fa1eb6f9ed63ae1ee2b1b8..90873cb905284fd68f19e61070028890ef086897 100644 (file)
--- a/mediagoblin/tests/test_api.py
+++ b/mediagoblin/tests/test_api.py
@@ -317,7 +317,7 @@ class TestAPI(object):
 
         # Find the objects in the database
         media = MediaEntry.query.filter_by(public_id=data["object"]["id"]).first()
-        comment = media.get_comments()[0]
+        comment = media.get_comments()[0].comment()
 
         # Tests that it matches in the database
         assert comment.actor == self.user.id

diff --git a/mediagoblin/tests/test_misc.py b/mediagoblin/tests/test_misc.py
index 3423520986da3056a6ac0bbc03a973aa2659bb1b..5500a0d70ec9c5a81d7e4c396501c108d7e30885 100644 (file)
--- a/mediagoblin/tests/test_misc.py
+++ b/mediagoblin/tests/test_misc.py
@@ -64,13 +64,13 @@ def test_user_deletes_other_comments(test_app):
 
     usr_cnt1 = User.query.count()
     med_cnt1 = MediaEntry.query.count()
-    cmt_cnt1 = TextComment.query.count()
+    cmt_cnt1 = Comment.query.count()
 
     User.query.get(user_a.id).delete(commit=False)
 
     usr_cnt2 = User.query.count()
     med_cnt2 = MediaEntry.query.count()
-    cmt_cnt2 = TextComment.query.count()
+    cmt_cnt2 = Comment.query.count()
 
     # One user deleted
     assert usr_cnt2 == usr_cnt1 - 1
@@ -83,7 +83,7 @@ def test_user_deletes_other_comments(test_app):
 
     usr_cnt2 = User.query.count()
     med_cnt2 = MediaEntry.query.count()
-    cmt_cnt2 = TextComment.query.count()
+    cmt_cnt2 = Comment.query.count()
 
     # All users gone
     assert usr_cnt2 == usr_cnt1 - 2