Change Notification.object_id to be ID of Comemnt not TextComment

This shouldn't really effect much but it is a needed change for the future
this changes the Notification.object_id to be the ID of the Comment (the link
table to the comment object) rather than TextComment (the comment object itself).

This is needed as now comments can be other things, other than TextComment.

mediagoblin/db/migrations/versions/4066b9f8b84a_use_comment_link_ids_notifications.py

@@ -0,0 +1,72 @@
+"""use_comment_link_ids_notifications
+
+Revision ID: 4066b9f8b84a
+Revises: 8429e33fdf7
+Create Date: 2016-02-29 11:46:13.511318
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '4066b9f8b84a'
+down_revision = '8429e33fdf7'
+
+from alembic import op
+from sqlalchemy import MetaData
+from mediagoblin.db.migration_tools import inspect_table
+
+def upgrade():
+    """"
+    This replaces the Notification.obj with the ID of the Comment (i.e. comment
+    link) ID instead of the TextComment object.
+    """
+    db = op.get_bind()
+    metadata = MetaData(bind=db)
+    notification_table = inspect_table(metadata, "core__notifications")
+    comment_table = inspect_table(metadata, "core__comment_links")
+
+    # Get the notifications.
+    notifications = list(db.execute(notification_table.select()))
+
+    # Iterate through all the notifications
+    for notification in notifications:
+        # Lookup the Comment link object from the notification's ID
+        comment_link = db.execute(comment_table.select().where(
+            comment_table.c.comment_id == notification.object_id
+        )).first()
+
+        # Okay now we need to update the notification with the ID of the link
+        # rather than the ID of TextComment object.
+        db.execute(notification_table.update().values(
+            object_id=comment_link.id
+        ).where(
+            notification_table.c.id == notification.id
+        ))
+
+
+def downgrade():
+    """
+    This puts back the TextComment ID for the notification.object_id field
+    where we're using the Comment object (i.e. the comment link ID)
+    """
+    db = op.get_bind()
+    metadata = MetaData(bind=db)
+    notification_table = inspect_table(metadata, "core__notifications")
+    comment_table = inspect_table(metadata, "core__comment_links")
+
+    # Notificaitons
+    notifications = list(db.execute(notification_table.select()))
+
+    # Iterate through all the notifications
+    for notification in notifications:
+        # Lookup the Comment link object from the notification's ID
+        comment_link = db.execute(comment_table.select().where(
+            comment_table.c.id == notification.object_id
+        )).first()
+
+        # Update the notification with the TextComment (i.e. the comment object)
+        db.execute(notification_table.update().values(
+            object_id=comment_link.comment_id
+        ).where(
+            notification_table.c.id == notification.id
+        ))
+

mediagoblin/notifications/__init__.py

@@ -27,14 +27,23 @@ def trigger_notification(comment, media_entry, request):
     '''
     Send out notifications about a new comment.
     '''
+    # Verify we have the Comment object and not any other type e.g. TextComment
+    if not isinstance(comment, Comment):
+        raise ValueError("Must provide Comment to trigger_notification")
+
+    # Get the associated object associated to the Comment wrapper.
+    comment_object = comment.comment()
+
     subscriptions = CommentSubscription.query.filter_by(
         media_entry_id=media_entry.id).all()
 
     for subscription in subscriptions:
+        # Check the user wants to be notified, if not, skip.
         if not subscription.notify:
             continue
 
-        if comment.get_actor == subscription.user:
+        # If the subscriber is the current actor, don't bother.
+        if comment_object.get_actor == subscription.user:
             continue
 
         cn = Notification(
@@ -61,7 +70,7 @@ def mark_notification_seen(notification):
 
 
 def mark_comment_notification_seen(comment_id, user):
-    comment = Comment.query.get(comment_id).comment()
+    comment = Comment.query.get(comment_id)
     comment_gmr = GenericModelReference.query.filter_by(
         obj_pk=comment.id,
         model_type=comment.__tablename__

mediagoblin/notifications/tools.py

@@ -18,32 +18,38 @@ from mediagoblin.tools.template import render_template
 from mediagoblin.tools.translate import pass_to_ugettext as _
 from mediagoblin import mg_globals
 
-def generate_comment_message(user, comment, media, request):
+def generate_comment_message(user, comment, commentee, request):
     """
     Sends comment email to user when a comment is made on their media.
 
     Args:
     - user: the user object to whom the email is sent
-    - comment: the comment object referencing user's media
+    - comment: the comment wrapper object
-    - media: the media object the comment is about
+    - commentee: the object the comment is on
     - request: the request
     """
 
-    comment_url = request.urlgen(
+    # Get the comment object associated to the wrapper
-                    'mediagoblin.user_pages.media_home.view_comment',
+    comment_object = comment.comment()
-                    comment=comment.id,
-                    user=media.get_actor.username,
-                    media=media.slug_or_id,
-                    qualified=True) + '#comment'
 
-    comment_author = comment.get_actor.username
+    # Get the URL to the comment
+    comment_url = request.urlgen(
+        "mediagoblin.user_pages.media_home.view_comment",
+        comment=comment.id,
+        user=commentee.get_actor.username,
+        media=commentee.slug_or_id, 
+        qualified=True) + "#comment"
+
+    comment_author = comment.comment().get_actor.username
 
     rendered_email = render_template(
         request, 'mediagoblin/user_pages/comment_email.txt',
         {'username': user.username,
          'comment_author': comment_author,
-         'comment_content': comment.content,
+         'comment_content': comment_object.content,
-         'comment_url': comment_url})
+         'comment_url': comment_url
+        }
+    )
 
     return {
         'from': mg_globals.app_config['email_sender_address'],
@@ -52,4 +58,5 @@ def generate_comment_message(user, comment, media, request):
             comment_author=comment_author,
             instance_title=mg_globals.app_config['html_title']) \
                     + _('commented on your post'),
-        'body': rendered_email}
+        'body': rendered_email
+    }

mediagoblin/notifications/views.py

@@ -58,7 +58,7 @@ def mark_all_comment_notifications_seen(request):
     """
     for comment in get_notifications(request.user.id):
         mark_comment_notification_seen(
-            comment.obj().get_comment_link().id,
+            comment.id,
             request.user
         )
 

mediagoblin/templates/mediagoblin/fragments/header_notifications.html

@@ -4,9 +4,10 @@
     <h3>{% trans %}New comments{% endtrans %}</h3>
     <ul>
         {% for notification in  notifications %}
-        {% set comment = notification.obj() %}
+        {% set comment_wrapper = notification.obj() %}
-        {% set comment_author = comment.get_actor %}
+        {% set comment_object = comment_wrapper.comment() %}
-        {% set media = comment.get_reply_to() %}
+        {% set comment_author = comment_object.get_actor %}
+        {% set comment_target = comment_wrapper.target() %}
         <li class="comment_wrapper">
             <div class="comment_author">
                 <img src="{{ request.staticdirect('/images/icon_comment.png') }}" />
@@ -16,12 +17,12 @@
                     {{- comment_author.username -}}
                 </a>
                 <a href="{{ request.urlgen('mediagoblin.user_pages.media_home.view_comment',
-                            comment=comment.id,
+                            comment=comment_wrapper.id,
-                            user=media.get_actor.username,
+                            user=comment_target.get_actor.username,
-                            media=media.slug_or_id) }}#comment"
+                            media=comment_target.slug_or_id) }}#comment"
                     class="comment_whenlink">
-                    <span title='{{- comment.created.strftime("%I:%M%p %Y-%m-%d") -}}'>
+                    <span title='{{- comment_object.created.strftime("%I:%M%p %Y-%m-%d") -}}'>
-                    {%- trans formatted_time=timesince(comment.created) -%}
+                    {%- trans formatted_time=timesince(comment_object.created) -%}
                         {{ formatted_time }} ago
                     {%- endtrans -%}
                     </span>
@@ -29,7 +30,7 @@
             </div>
             <div class="comment_content">
                 {% autoescape False -%}
-                {{ comment.content_html }}
+                {{ comment_object.content_html }}
                 {%- endautoescape %}
             </div>
 

mediagoblin/tests/test_notifications.py

@@ -110,8 +110,8 @@ class TestNotifications:
 
         assert notification.seen == False
         assert notification.user_id == user.id
-        assert notification.obj().get_actor.id == self.test_user.id
+        assert notification.obj().comment().get_actor.id == self.test_user.id
-        assert notification.obj().content == u'Test comment #42'
+        assert notification.obj().comment().content == u'Test comment #42'
 
         if wants_email == True:
             # Why the `or' here?  In Werkzeug 0.11.0 and above
@@ -142,7 +142,7 @@ otherperson@example.com\n\nSGkgb3RoZXJwZXJzb24sCmNocmlzIGNvbW1lbnRlZCBvbiB5b3VyI
 
         # Save the ids temporarily because of DetachedInstanceError
         notification_id = notification.id
-        comment_id = notification.obj().get_comment_link().id
+        comment_id = notification.obj().id
 
         self.logout()
         self.login('otherperson', 'nosreprehto')

mediagoblin/user_pages/views.py

@@ -207,7 +207,7 @@ def media_post_comment(request, media):
         messages.add_message(
             request, messages.SUCCESS,
             _('Your comment has been posted!'))
-        trigger_notification(comment, media, request)
+        trigger_notification(link, media, request)
 
     return redirect_obj(request, media)