Skip to content

Commit

Permalink
API param to exclude notification types from response (mastodon#1341)
Browse files Browse the repository at this point in the history
* Add exclude_types param to /api/v1/notifications

* Exclude notification types in web UI through exclude_types in the API
  • Loading branch information
Gargron authored Apr 10, 2017
1 parent 0687ab8 commit 2810013
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 17 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ end

group :test do
gem 'faker'
gem 'rails-controller-testing'
gem 'rspec-sidekiq'
gem 'simplecov', require: false
gem 'webmock'
Expand Down
5 changes: 5 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -487,6 +491,7 @@ DEPENDENCIES
rack-cors
rack-timeout
rails (~> 5.0.2)
rails-controller-testing
rails-settings-cached
rails_12factor
react-rails
Expand Down
12 changes: 7 additions & 5 deletions app/assets/javascripts/components/actions/notifications.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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));
Expand Down
10 changes: 8 additions & 2 deletions app/controllers/api/v1/notifications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -32,7 +32,13 @@ def clear

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
27 changes: 20 additions & 7 deletions app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -28,12 +35,7 @@ def activity(eager_loaded = true)
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
Expand All @@ -50,6 +52,11 @@ def browserable?
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
Expand All @@ -58,6 +65,12 @@ def reload_stale_associations!(cached_items)
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
Expand Down
62 changes: 59 additions & 3 deletions spec/controllers/api/v1/notifications_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,71 @@

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

0 comments on commit 2810013

Please sign in to comment.