From 0dbafa17517a53cec85ee942d52ca50675da6a0c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonne=20Ha=C3=9F?= <me@jhass.eu>
Date: Sat, 8 Feb 2020 20:17:23 +0100
Subject: [PATCH] API: Don't return notifications target unless it's a post

---
 .../api/v1/notifications_controller.rb        |  2 +-
 app/presenters/notification_presenter.rb      |  4 +-
 lib/schemas/api_v1.json                       |  2 +-
 .../presenters/notification_presenter_spec.rb | 52 +++++++++++++------
 4 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb
index d6662ccefdf..5fad84cfd5d 100644
--- a/app/controllers/api/v1/notifications_controller.rb
+++ b/app/controllers/api/v1/notifications_controller.rb
@@ -15,7 +15,7 @@ def show
         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
diff --git a/app/presenters/notification_presenter.rb b/app/presenters/notification_presenter.rb
index 34407e2c795..91488996ca6 100644
--- a/app/presenters/notification_presenter.rb
+++ b/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 linked_object&.is_a?(Post)
     data
   end
 
diff --git a/lib/schemas/api_v1.json b/lib/schemas/api_v1.json
index c4b7dbd9f5e..f7aa3f72656 100644
--- a/lib/schemas/api_v1.json
+++ b/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
     },
 
diff --git a/spec/presenters/notification_presenter_spec.rb b/spec/presenters/notification_presenter_spec.rb
index b981efe31fc..d9bc93e4f93 100644
--- a/spec/presenters/notification_presenter_spec.rb
+++ b/spec/presenters/notification_presenter_spec.rb
@@ -1,31 +1,49 @@
 # frozen_string_literal: true
 
 describe NotificationPresenter do
-  before do
-    @post = FactoryGirl.create(:status_message)
-    @notification = FactoryGirl.create(:notification, recipient: alice, target: @post)
-  end
-
-  it "makes json with target when requested" do
-    json = NotificationPresenter.new(@notification).as_api_json(true)
-    expect(json[:guid]).to eq(@notification.guid)
+  it "makes json with target" do
+    post = FactoryGirl.create(:status_message)
+    notification = FactoryGirl.create(:notification, recipient: alice, target: post)
+    json = NotificationPresenter.new(notification).as_api_json
+    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[:target][:guid]).to eq(@post.guid)
+    expect(json[:created_at]).to eq(notification.created_at)
+    expect(json[:target][:guid]).to eq(post.guid)
     expect(json[:event_creators].length).to eq(1)
   end
 
-  it "makes json with without target" do
-    json = NotificationPresenter.new(@notification).as_api_json(false)
-    expect(json.has_key?(:target)).to be_falsey
-  end
-
-  it "Makes target on mentioned" do
+  it "returns 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 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