From ecbcebb917da2171acd81c8edac6f93c30fefed2 Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Tue, 21 May 2019 07:28:49 -0400 Subject: [PATCH] Add `account_id` param to `GET /api/v1/notifications` (#10796) * Add `from_account` to notifications API this adds the ability to filter notifications by the account they originated from * passing a non-existent user should cause none to be returned * Fix codeclimate warnings * fix more codeclimate warnings * make requested changes: * use account id instead of user@domain * name the param `account_id` instead of `from_account` * Don't use `return` in a lambda --- .../api/v1/notifications_controller.rb | 6 +- app/models/notification.rb | 8 ++- .../api/v1/notifications_controller_spec.rb | 66 +++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index 3b492c5163391c..c91753ae7a925a 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -53,7 +53,7 @@ def paginated_notifications end def browserable_account_notifications - current_account.notifications.browserable(exclude_types) + current_account.notifications.browserable(exclude_types, from_account) end def target_statuses_from_notifications @@ -90,6 +90,10 @@ def exclude_types val end + def from_account + params[:account_id] + end + def pagination_params(core_params) params.slice(:limit, :exclude_types).permit(:limit, exclude_types: []).merge(core_params) end diff --git a/app/models/notification.rb b/app/models/notification.rb index 300269e24496c9..498673ff13d863 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -41,9 +41,13 @@ class Notification < ApplicationRecord validates :account_id, uniqueness: { scope: [:activity_type, :activity_id] } validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values } - scope :browserable, ->(exclude_types = []) { + scope :browserable, ->(exclude_types = [], account_id = nil) { types = TYPE_CLASS_MAP.values - activity_types_from_types(exclude_types + [:follow_request]) - where(activity_type: types) + if account_id.nil? + where(activity_type: types) + else + where(activity_type: types, from_account_id: account_id) + end } cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account, poll: [status: STATUS_INCLUDES] diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb index d0f82e79febbc8..db3f4b782be842 100644 --- a/spec/controllers/api/v1/notifications_controller_spec.rb +++ b/spec/controllers/api/v1/notifications_controller_spec.rb @@ -6,6 +6,7 @@ let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:other) { Fabricate(:user, account: Fabricate(:account, username: 'bob')) } + let(:third) { Fabricate(:user, account: Fabricate(:account, username: 'carol')) } before do allow(controller).to receive(:doorkeeper_token) { token } @@ -55,6 +56,7 @@ mentioning_status = PostStatusService.new.call(other.account, text: 'Hello @alice') @mention_from_status = mentioning_status.mentions.first @favourite = FavouriteService.new.call(other.account, first_status) + @second_favourite = FavouriteService.new.call(third.account, first_status) @follow = FollowService.new.call(other.account, 'alice') end @@ -84,6 +86,66 @@ end end + describe 'from specified user' do + before do + get :index, params: { account_id: third.account.id } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'includes favourite' do + expect(assigns(:notifications).map(&:activity)).to include(@second_favourite) + end + + it 'excludes favourite' do + expect(assigns(:notifications).map(&:activity)).to_not include(@favourite) + end + + it 'excludes mention' do + expect(assigns(:notifications).map(&:activity)).to_not include(@mention_from_status) + end + + it 'excludes reblog' do + expect(assigns(:notifications).map(&:activity)).to_not include(@reblog_of_first_status) + end + + it 'excludes follow' do + expect(assigns(:notifications).map(&:activity)).to_not include(@follow) + end + end + + describe 'from nonexistent user' do + before do + get :index, params: { account_id: 'foo' } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'excludes favourite' do + expect(assigns(:notifications).map(&:activity)).to_not include(@favourite) + end + + it 'excludes second favourite' do + expect(assigns(:notifications).map(&:activity)).to_not include(@second_favourite) + end + + it 'excludes mention' do + expect(assigns(:notifications).map(&:activity)).to_not include(@mention_from_status) + end + + it 'excludes reblog' do + expect(assigns(:notifications).map(&:activity)).to_not include(@reblog_of_first_status) + end + + it 'excludes follow' do + expect(assigns(:notifications).map(&:activity)).to_not include(@follow) + end + end + describe 'with excluded mentions' do before do get :index, params: { exclude_types: ['mention'] } @@ -105,6 +167,10 @@ expect(assigns(:notifications).map(&:activity)).to include(@favourite) end + it 'includes third favourite' do + expect(assigns(:notifications).map(&:activity)).to include(@second_favourite) + end + it 'includes follow' do expect(assigns(:notifications).map(&:activity)).to include(@follow) end