diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb new file mode 100644 index 000000000..c2d806b38 --- /dev/null +++ b/app/controllers/api/v1/notifications_controller.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Api + module V1 + class NotificationsController < Api::V1::BaseController + before_action except: %i[update] do + require_access_token %w[read] + end + + before_action only: %i[update] do + require_access_token %w[write] + end + + rescue_from ActiveRecord::RecordNotFound do + render json: I18n.t("api.endpoint_errors.notifications.not_found"), status: :not_found + end + + def show + notification = service.get_by_guid(params[:id]) + + if notification + render json: NotificationPresenter.new(notification).as_api_json(true) + else + render json: I18n.t("api.endpoint_errors.notifications.not_found"), status: :not_found + end + end + + def index + after_date = Date.iso8601(params[:only_after]) if params.has_key?(:only_after) + notifications = service.index(params[:only_unread], after_date) + render json: notifications.map {|note| NotificationPresenter.new(note, default_serializer_options).as_api_json } + rescue ArgumentError + render json: I18n.t("api.endpoint_errors.notifications.cant_process"), status: :unprocessable_entity + end + + def update + read = ActiveModel::Type::Boolean.new.cast(params.require(:read)) + if service.update_status_by_guid(params[:id], read) + head :no_content + else + render json: I18n.t("api.endpoint_errors.notifications.cant_process"), status: :unprocessable_entity + end + rescue ActionController::ParameterMissing + render json: I18n.t("api.endpoint_errors.notifications.cant_process"), status: :unprocessable_entity + end + + private + + def service + @service ||= NotificationService.new(current_user) + end + end + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 3f8fe719e..c70422e0e 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -5,6 +5,8 @@ # the COPYRIGHT file. # class Notification < ApplicationRecord + include Diaspora::Fields::Guid + belongs_to :recipient, class_name: "User" has_many :notification_actors, dependent: :delete_all has_many :actors, class_name: "Person", through: :notification_actors, source: :person diff --git a/app/presenters/notification_presenter.rb b/app/presenters/notification_presenter.rb new file mode 100644 index 000000000..445a98867 --- /dev/null +++ b/app/presenters/notification_presenter.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class NotificationPresenter < BasePresenter + def as_api_json(include_target=true) + data = base_hash + data = data.merge(target: target_json) if include_target + data + end + + private + + def base_hash + { + guid: guid, + type: type_as_json, + read: !unread, + created_at: created_at, + event_creators: creators_json + } + end + + def target_json + { + guid: target.guid, + author: PersonPresenter.new(target.author).as_api_json + } + end + + def creators_json + actors.map {|actor| PersonPresenter.new(actor).as_api_json } + end + + def type_as_json + NotificationService::NOTIFICATIONS_REVERSE_JSON_TYPES[type] + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 92fd979e8..71905e19f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -11,6 +11,45 @@ class NotificationService Contact => [Notifications::StartedSharing] }.freeze + NOTIFICATIONS_JSON_TYPES = { + "also_commented" => "Notifications::AlsoCommented", + "comment_on_post" => "Notifications::CommentOnPost", + "liked" => "Notifications::Liked", + "mentioned" => "Notifications::MentionedInPost", + "mentioned_in_comment" => "Notifications::MentionedInComment", + "reshared" => "Notifications::Reshared", + "started_sharing" => "Notifications::StartedSharing", + "contacts_birthday" => "Notifications::ContactsBirthday" + }.freeze + + NOTIFICATIONS_REVERSE_JSON_TYPES = NOTIFICATIONS_JSON_TYPES.invert.freeze + + def initialize(user=nil) + @user = user + end + + def index(unread_only=nil, only_after=nil) + query_string = "recipient_id = ? " + query_string += "AND unread = true " if unread_only + where_clause = [query_string, @user.id] + if only_after + query_string += " AND created_at >= ?" + where_clause = [query_string, @user.id, only_after] + end + Notification.where(where_clause).includes(:target, actors: :profile) + end + + def get_by_guid(guid) + Notification.where(recipient_id: @user.id, guid: guid).first + end + + def update_status_by_guid(guid, is_read_status) + notification = get_by_guid(guid) + raise ActiveRecord::RecordNotFound unless notification + notification.set_read_state(is_read_status) + true + end + def notify(object, recipient_user_ids) notification_types(object).each {|type| type.notify(object, recipient_user_ids) } end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 9b72c8694..7de2fcd9d 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -974,6 +974,9 @@ en: no_like: "Like doesn’t exist" interactions: cant_subscribe: "Can't subscribe to this post" + notifications: + not_found: "Notification with provided guid could not be found" + cant_process: "Couldn't process the notifications request" posts: post_not_found: "Post with provided guid could not be found" failed_create: "Failed to create the post" diff --git a/config/routes.rb b/config/routes.rb index c64d0cd33..6dc858556 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -239,7 +239,7 @@ Rails.application.routes.draw do resources :conversations, only: %i[show index create destroy] do resources :messages, only: %i[index create] end - + resources :notifications, only: %i[index show update] resources :tag_followings, only: %i[index create destroy] get "streams/activity" => "streams#activity", :as => "activity_stream" get "streams/main" => "streams#multi", :as => "stream" diff --git a/db/migrate/20181127012722_add_guid_to_notifications.rb b/db/migrate/20181127012722_add_guid_to_notifications.rb new file mode 100644 index 000000000..8581173f2 --- /dev/null +++ b/db/migrate/20181127012722_add_guid_to_notifications.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddGuidToNotifications < ActiveRecord::Migration[5.1] + def change + add_column :notifications, :guid, :string + add_index :notifications, :guid, name: :index_notifications_on_guid, length: 191, unique: true + end +end diff --git a/spec/integration/api/notifications_controller_spec.rb b/spec/integration/api/notifications_controller_spec.rb new file mode 100644 index 000000000..e8c52f38e --- /dev/null +++ b/spec/integration/api/notifications_controller_spec.rb @@ -0,0 +1,208 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe Api::V1::NotificationsController do + let(:auth) { FactoryGirl.create(:auth_with_read_and_write) } + let(:auth_read_only) { FactoryGirl.create(:auth_with_read) } + let!(:access_token) { auth.create_access_token.to_s } + let!(:access_token_read_only) { auth_read_only.create_access_token.to_s } + + before do + @post = auth.user.post( + :status_message, + text: "This is a status message", + public: true, + to: "all" + ) + @notification = FactoryGirl.create(:notification, recipient: auth.user, target: @post) + end + + describe "#index" do + context "success" do + it "with proper credentials and no flags" do + get( + api_v1_notifications_path, + params: {access_token: access_token} + ) + expect(response.status).to eq(200) + notification = JSON.parse(response.body) + expect(notification.length).to eq(1) + confirm_notification_format(notification[0], @notification, "also_commented", nil) + end + + it "with proper credentials and unread only" do + get( + api_v1_notifications_path, + params: {only_unread: true, access_token: access_token} + ) + expect(response.status).to eq(200) + notification = JSON.parse(response.body) + expect(notification.length).to eq(1) + @notification.set_read_state(true) + get( + api_v1_notifications_path, + params: {only_unread: true, access_token: access_token} + ) + expect(response.status).to eq(200) + notification = JSON.parse(response.body) + expect(notification.length).to eq(0) + end + + it "with proper credentials and after certain date" do + get( + api_v1_notifications_path, + params: {only_after: (Date.current - 1.day).iso8601, access_token: access_token} + ) + expect(response.status).to eq(200) + notification = JSON.parse(response.body) + expect(notification.length).to eq(1) + @notification.set_read_state(true) + get( + api_v1_notifications_path, + params: {only_after: (Date.current + 1.day).iso8601, access_token: access_token} + ) + expect(response.status).to eq(200) + notification = JSON.parse(response.body) + expect(notification.length).to eq(0) + end + end + + context "fails" do + it "with bad date format" do + get( + api_v1_notifications_path, + params: {only_after: "January 1, 2018", access_token: access_token} + ) + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.notifications.cant_process")) + end + + it "with improper credentials" do + get( + api_v1_notifications_path, + params: {access_token: "999_999_999"} + ) + expect(response.status).to eq(401) + end + end + end + + describe "#show" do + context "success" do + it "with proper credentials and flags" do + get( + api_v1_notification_path(@notification.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(200) + notification = JSON.parse(response.body) + confirm_notification_format(notification, @notification, "also_commented", @post) + end + end + + context "fails" do + it "with proper invalid GUID" do + get( + api_v1_notification_path("999_999_999"), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.notifications.not_found")) + end + + it "on someone else's notification" do + alice_post = alice.post( + :status_message, + text: "This is a status message", + public: true, + to: "all" + ) + alice_notification = FactoryGirl.create(:notification, recipient: alice, target: alice_post) + + get( + api_v1_notification_path(alice_notification.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.notifications.not_found")) + end + + it "with improper credentials" do + get( + api_v1_notification_path(@notification.guid), + params: {access_token: "999_999_999"} + ) + expect(response.status).to eq(401) + end + end + end + + describe "#update" do + context "success" do + it "with proper credentials and flags" do + patch( + api_v1_notification_path(@notification.guid), + params: {read: true, access_token: access_token} + ) + expect(response.status).to eq(204) + expect(@notification.reload.unread).to be(false) + patch( + api_v1_notification_path(@notification.guid), + params: {read: false, access_token: access_token} + ) + expect(response.status).to eq(204) + expect(@notification.reload.unread).to be(true) + end + end + + context "fails" do + it "with proper invalid GUID" do + patch( + api_v1_notification_path("999_999_999"), + params: {access_token: access_token} + ) + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.notifications.cant_process")) + end + + it "with proper missing read field" do + patch( + api_v1_notification_path(@notification.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.notifications.cant_process")) + end + + it "with read only credentials" do + patch( + api_v1_notification_path(@notification.guid), + params: {access_token: access_token_read_only} + ) + expect(response.status).to eq(403) + end + + it "with improper credentials" do + patch( + api_v1_notification_path(@notification.guid), + params: {access_token: "999_999_999"} + ) + expect(response.status).to eq(401) + end + end + end + + private + + # rubocop:disable Metrics/AbcSize + def confirm_notification_format(notification, ref_notification, expected_type, target) + expect(notification["guid"]).to eq(ref_notification.guid) + expect(notification["type"]).to eq(expected_type) if expected_type + expect(notification["read"]).to eq(!ref_notification.unread) + expect(notification.has_key?("created_at")).to be_truthy + expect(notification["target"]["guid"]).to eq(target.guid) if target + expect(notification["event_creators"].length).to eq(ref_notification.actors.length) + end + # rubocop:enable Metrics/AbcSize +end diff --git a/spec/presenters/notification_presenter_spec.rb b/spec/presenters/notification_presenter_spec.rb new file mode 100644 index 000000000..0902e420a --- /dev/null +++ b/spec/presenters/notification_presenter_spec.rb @@ -0,0 +1,23 @@ +# 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) + 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[: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 +end diff --git a/spec/services/conversation_service.rb b/spec/services/conversation_service_spec.rb similarity index 100% rename from spec/services/conversation_service.rb rename to spec/services/conversation_service_spec.rb diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index e690b8de8..59c72b74f 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -73,4 +73,61 @@ describe NotificationService do end end end + + describe "query methods" do + before do + @post = alice.post( + :status_message, + text: "This is a status message", + public: true, + to: "all" + ) + @notification = FactoryGirl.create(:notification, recipient: alice, target: @post) + @service = NotificationService.new(alice) + end + + describe "#index" do + it "gets all" do + notifications = @service.index + expect(notifications.length).to eq(1) + end + + it "gets unread only" do + notifications = @service.index(true) + expect(notifications.length).to eq(1) + @notification.set_read_state(true) + notifications = @service.index(true) + expect(notifications.length).to eq(0) + end + + it "gets only after" do + notifications = @service.index(nil, (Time.current - 1.day)) + expect(notifications.length).to eq(1) + @notification.set_read_state(true) + notifications = @service.index(nil, (Time.current + 1.day)) + expect(notifications.length).to eq(0) + end + + it "combined filtering" do + notifications = @service.index(true, (Time.current - 1.day)) + expect(notifications.length).to eq(1) + end + end + + describe "#show" do + it "succeeds with valid GUID" do + notification = @service.get_by_guid(@notification.guid) + expect(notification).not_to be_nil + end + end + + describe "#update" do + it "succeeds with valid GUID" do + expect(@service.update_status_by_guid(@notification.guid, true)).to be_truthy + expect(@notification.reload.unread).to eq(false) + expect(@service.update_status_by_guid(@notification.guid, false)).to be_truthy + expect(@notification.reload.unread).to eq(true) + end + end + end end