diff options
| -rw-r--r-- | Gemfile | 1 | ||||
| -rw-r--r-- | Gemfile.lock | 5 | ||||
| -rw-r--r-- | app/assets/javascripts/components/actions/notifications.jsx | 12 | ||||
| -rw-r--r-- | app/controllers/api/v1/notifications_controller.rb | 10 | ||||
| -rw-r--r-- | app/models/notification.rb | 27 | ||||
| -rw-r--r-- | spec/controllers/api/v1/notifications_controller_spec.rb | 62 |
6 files changed, 100 insertions, 17 deletions
diff --git a/Gemfile b/Gemfile index 8810c83de..078ac5806 100644 --- a/Gemfile +++ b/Gemfile @@ -68,6 +68,7 @@ end group :test do gem 'faker' + gem 'rails-controller-testing' gem 'rspec-sidekiq' gem 'simplecov', require: false gem 'webmock' diff --git a/Gemfile.lock b/Gemfile.lock index 6e3865527..4fe8aa076 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -286,6 +286,10 @@ GEM bundler (>= 1.3.0, < 2.0) railties (= 5.0.2) sprockets-rails (>= 2.0.0) + rails-controller-testing (1.0.1) + actionpack (~> 5.x) + actionview (~> 5.x) + activesupport (~> 5.x) rails-dom-testing (2.0.2) activesupport (>= 4.2.0, < 6.0) nokogiri (~> 1.6) @@ -487,6 +491,7 @@ DEPENDENCIES rack-cors rack-timeout rails (~> 5.0.2) + rails-controller-testing rails-settings-cached rails_12factor react-rails diff --git a/app/assets/javascripts/components/actions/notifications.jsx b/app/assets/javascripts/components/actions/notifications.jsx index 980b7d63e..11e814e1f 100644 --- a/app/assets/javascripts/components/actions/notifications.jsx +++ b/app/assets/javascripts/components/actions/notifications.jsx @@ -61,6 +61,8 @@ export function refreshNotifications() { params.since_id = ids.first().get('id'); } + params.exclude_types = getState().getIn(['settings', 'notifications', 'shows']).filter(enabled => !enabled).keySeq().toJS(); + api(getState).get('/api/v1/notifications', { params }).then(response => { const next = getLinks(response).refs.find(link => link.rel === 'next'); @@ -105,11 +107,11 @@ export function expandNotifications() { dispatch(expandNotificationsRequest()); - api(getState).get(url, { - params: { - limit: 5 - } - }).then(response => { + const params = {}; + + params.exclude_types = getState().getIn(['settings', 'notifications', 'shows']).filter(enabled => !enabled).keySeq().toJS(); + + api(getState).get(url, params).then(response => { const next = getLinks(response).refs.find(link => link.rel === 'next'); dispatch(expandNotificationsSuccess(response.data, next ? next.uri : null)); diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index 71c054334..3cff29982 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -9,7 +9,7 @@ class Api::V1::NotificationsController < ApiController DEFAULT_NOTIFICATIONS_LIMIT = 15 def index - @notifications = Notification.where(account: current_account).browserable.paginate_by_max_id(limit_param(DEFAULT_NOTIFICATIONS_LIMIT), params[:max_id], params[:since_id]) + @notifications = Notification.where(account: current_account).browserable(exclude_types).paginate_by_max_id(limit_param(DEFAULT_NOTIFICATIONS_LIMIT), params[:max_id], params[:since_id]) @notifications = cache_collection(@notifications, Notification) statuses = @notifications.select { |n| !n.target_status.nil? }.map(&:target_status) @@ -32,7 +32,13 @@ class Api::V1::NotificationsController < ApiController private + def exclude_types + val = params.permit(exclude_types: [])[:exclude_types] || [] + val = [val] unless val.is_a?(Enumerable) + val + end + def pagination_params(core_params) - params.permit(:limit).merge(core_params) + params.permit(:limit, exclude_types: []).merge(core_params) end end diff --git a/app/models/notification.rb b/app/models/notification.rb index b7b474869..302d4382d 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -16,10 +16,17 @@ class Notification < ApplicationRecord validates :account_id, uniqueness: { scope: [:activity_type, :activity_id] } + TYPE_CLASS_MAP = { + mention: 'Mention', + reblog: 'Status', + follow: 'Follow', + follow_request: 'FollowRequest', + favourite: 'Favourite', + }.freeze + STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :media_attachments, :tags, mentions: :account]].freeze scope :cache_ids, -> { select(:id, :updated_at, :activity_type, :activity_id) } - scope :browserable, -> { where.not(activity_type: ['FollowRequest']) } cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account @@ -28,12 +35,7 @@ class Notification < ApplicationRecord end def type - case activity_type - when 'Status' - :reblog - else - activity_type.underscore.to_sym - end + @type ||= TYPE_CLASS_MAP.invert[activity_type].to_sym end def target_status @@ -50,6 +52,11 @@ class Notification < ApplicationRecord end class << self + def browserable(types = []) + types.concat([:follow_request]) + where.not(activity_type: activity_types_from_types(types)) + end + def reload_stale_associations!(cached_items) account_ids = cached_items.map(&:from_account_id).uniq accounts = Account.where(id: account_ids).map { |a| [a.id, a] }.to_h @@ -58,6 +65,12 @@ class Notification < ApplicationRecord item.from_account = accounts[item.from_account_id] end end + + private + + def activity_types_from_types(types) + types.map { |type| TYPE_CLASS_MAP[type.to_sym] }.compact + end end after_initialize :set_from_account diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb index e5f7eec73..c390d4f01 100644 --- a/spec/controllers/api/v1/notifications_controller_spec.rb +++ b/spec/controllers/api/v1/notifications_controller_spec.rb @@ -5,15 +5,71 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } let(:token) { double acceptable?: true, resource_owner_id: user.id } + let(:other) { Fabricate(:user, account: Fabricate(:account, username: 'bob')) } before do allow(controller).to receive(:doorkeeper_token) { token } end describe 'GET #index' do - it 'returns http success' do - get :index - expect(response).to have_http_status(:success) + before do + status = PostStatusService.new.call(user.account, 'Test') + @reblog = ReblogService.new.call(other.account, status) + @mention = PostStatusService.new.call(other.account, 'Hello @alice') + @favourite = FavouriteService.new.call(other.account, status) + @follow = FollowService.new.call(other.account, 'alice') + end + + describe 'with no options' do + before do + get :index + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'includes reblog' do + expect(assigns(:notifications).map(&:activity_id)).to include(@reblog.id) + end + + it 'includes mention' do + expect(assigns(:notifications).map(&:activity_id)).to include(@mention.mentions.first.id) + end + + it 'includes favourite' do + expect(assigns(:notifications).map(&:activity_id)).to include(@favourite.id) + end + + it 'includes follow' do + expect(assigns(:notifications).map(&:activity_id)).to include(@follow.id) + end + end + + describe 'with excluded mentions' do + before do + get :index, params: { exclude_types: ['mention'] } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'includes reblog' do + expect(assigns(:notifications).map(&:activity_id)).to include(@reblog.id) + end + + it 'excludes mention' do + expect(assigns(:notifications).map(&:activity_id)).to_not include(@mention.mentions.first.id) + end + + it 'includes favourite' do + expect(assigns(:notifications).map(&:activity_id)).to include(@favourite.id) + end + + it 'includes follow' do + expect(assigns(:notifications).map(&:activity_id)).to include(@follow.id) + end end end end |