API: Don't return notifications target unless it's a post

app/controllers/api/v1/notifications_controller.rb

@@ -15,7 +15,7 @@ module Api
         notification = service.get_by_guid(params[:id])
 
         if notification
-          render json: NotificationPresenter.new(notification).as_api_json(true)
+          render json: NotificationPresenter.new(notification).as_api_json
         else
           render_error 404, "Notification with provided guid could not be found"
         end

app/presenters/notification_presenter.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 class NotificationPresenter < BasePresenter
-  def as_api_json(include_target=true)
+  def as_api_json
     data = base_hash
-    data = data.merge(target: target_json) if include_target && linked_object
+    data = data.merge(target: target_json) if target
     data
   end
 
@@ -20,8 +20,8 @@ class NotificationPresenter < BasePresenter
   end
 
   def target_json
-    json = {guid: linked_object.guid}
+    json = {guid: target.guid}
-    json[:author] = PersonPresenter.new(linked_object.author).as_api_json if linked_object.author
+    json[:author] = PersonPresenter.new(target.author).as_api_json if target.author
     json
   end
 
@@ -32,4 +32,9 @@ class NotificationPresenter < BasePresenter
   def type_as_json
     NotificationService::NOTIFICATIONS_REVERSE_JSON_TYPES[type]
   end
+
+  def target
+    return linked_object if linked_object&.is_a?(Post)
+    return linked_object.post if linked_object&.respond_to?(:post)
+  end
 end

lib/schemas/api_v1.json

@@ -213,7 +213,7 @@
           "items": { "$ref": "https://diaspora.software/api/v1/schema.json#/definitions/short_profile" }
         }
       },
-      "required": ["guid", "type", "read", "created_at", "target"],
+      "required": ["guid", "type", "read", "created_at"],
       "additionalProperties": false
     },
 

spec/presenters/notification_presenter_spec.rb

@@ -1,31 +1,58 @@
 # frozen_string_literal: true
 
 describe NotificationPresenter do
-  before do
+  it "makes json with target" do
-    @post = FactoryGirl.create(:status_message)
+    post = FactoryGirl.create(:status_message)
-    @notification = FactoryGirl.create(:notification, recipient: alice, target: @post)
+    notification = FactoryGirl.create(:notification, recipient: alice, target: post)
-  end
+    json = NotificationPresenter.new(notification).as_api_json
-
+    expect(json[:guid]).to eq(notification.guid)
-  it "makes json with target when requested" do
-    json = NotificationPresenter.new(@notification).as_api_json(true)
-    expect(json[:guid]).to eq(@notification.guid)
     expect(json[:type]).to eq("also_commented")
     expect(json[:read]).to be_falsey
-    expect(json[:created_at]).to eq(@notification.created_at)
+    expect(json[:created_at]).to eq(notification.created_at)
-    expect(json[:target][:guid]).to eq(@post.guid)
+    expect(json[:target][:guid]).to eq(post.guid)
     expect(json[:event_creators].length).to eq(1)
   end
 
-  it "makes json with without target" do
+  it "returns target on mentioned" do
-    json = NotificationPresenter.new(@notification).as_api_json(false)
-    expect(json.has_key?(:target)).to be_falsey
-  end
-
-  it "Makes target on mentioned" do
     mentioned_post = FactoryGirl.create(:status_message_in_aspect, author: alice.person, text: text_mentioning(bob))
     Notifications::MentionedInPost.notify(mentioned_post, [bob.id])
     notification = Notifications::MentionedInPost.last
-    json = NotificationPresenter.new(notification).as_api_json(true)
+    json = NotificationPresenter.new(notification).as_api_json
     expect(json[:target][:guid]).to eq(mentioned_post.guid)
   end
+
+  it "returns target on mentioned in comment" do
+    post = FactoryGirl.create(:status_message, public: true)
+    mentioned_comment = FactoryGirl.create(:comment, post: post, author: alice.person, text: text_mentioning(bob))
+    Notifications::MentionedInComment.notify(mentioned_comment, [bob.id])
+    notification = Notifications::MentionedInComment.last
+    json = NotificationPresenter.new(notification).as_api_json
+    expect(json[:target][:guid]).to eq(mentioned_comment.post.guid)
+  end
+
+  it "returns target on also_commented" do
+    post = FactoryGirl.create(:status_message)
+    bob.comment!(post, "cool")
+    comment2 = FactoryGirl.create(:comment, post: post)
+    Notifications::AlsoCommented.notify(comment2, [])
+    notification = Notifications::AlsoCommented.last
+    json = NotificationPresenter.new(notification).as_api_json
+    expect(json[:target][:guid]).to eq(post.guid)
+  end
+
+  it "returns no target on started_sharing" do
+    contact = FactoryGirl.create(:contact)
+    Notifications::StartedSharing.notify(contact, [bob.id])
+    notification = Notifications::StartedSharing.last
+    json = NotificationPresenter.new(notification).as_api_json
+    expect(json[:target]).to be_nil
+  end
+
+  it "returns no target on contacts_birthday" do
+    contact = FactoryGirl.create(:contact)
+    Notifications::ContactsBirthday.notify(contact, [bob.id])
+    notification = Notifications::ContactsBirthday.last
+    json = NotificationPresenter.new(notification).as_api_json
+    expect(json[:target]).to be_nil
+  end
 end