API param to exclude notification types from response (#1341)

* Add exclude_types param to /api/v1/notifications

* Exclude notification types in web UI through exclude_types in the API

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