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.

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

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()

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)

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

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>

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

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