From cd0995abf34634b5f90ed7417863fb76c32f460c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= 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 | 13 ++-- lib/schemas/api_v1.json | 2 +- .../presenters/notification_presenter_spec.rb | 61 +++++++++++++------ 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index d6662ccef..5fad84cfd 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/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 diff --git a/app/presenters/notification_presenter.rb b/app/presenters/notification_presenter.rb index 34407e2c7..10b5c6a3b 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 target data end @@ -20,8 +20,8 @@ class NotificationPresenter < BasePresenter end def target_json - json = {guid: linked_object.guid} - json[:author] = PersonPresenter.new(linked_object.author).as_api_json if linked_object.author + json = {guid: target.guid} + 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 diff --git a/lib/schemas/api_v1.json b/lib/schemas/api_v1.json index c4b7dbd9f..f7aa3f726 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 b981efe31..ce760aed8 100644 --- a/spec/presenters/notification_presenter_spec.rb +++ b/spec/presenters/notification_presenter_spec.rb @@ -1,31 +1,58 @@ # 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 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