From 3b3bdc7293493735a2169d3377a5a5b7d9006497 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 31 Dec 2019 00:55:32 +0100 Subject: [PATCH] Hide blocked users from more places (#12733) * Hide blocked, muted, and blocked-by users from toot favourite lists * Hide blocked, muted, and blocked-by users from toot reblog lists * Hide blocked, muted, and blocked-by users from followers/following (API) * Fix tests * Hide blocked, muted, and blocked-by users from followers/following on public pages --- .../accounts/follower_accounts_controller.rb | 4 ++- .../accounts/following_accounts_controller.rb | 4 ++- .../favourited_by_accounts_controller.rb | 4 ++- .../reblogged_by_accounts_controller.rb | 4 ++- .../follower_accounts_controller.rb | 6 ++++- .../following_accounts_controller.rb | 6 ++++- .../follower_accounts_controller_spec.rb | 27 ++++++++++++++++--- .../following_accounts_controller_spec.rb | 27 ++++++++++++++++--- .../favourited_by_accounts_controller_spec.rb | 20 ++++++++++++-- .../reblogged_by_accounts_controller_spec.rb | 20 ++++++++++++-- .../follower_accounts_controller_spec.rb | 12 +++++++++ .../following_accounts_controller_spec.rb | 12 +++++++++ 12 files changed, 128 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v1/accounts/follower_accounts_controller.rb b/app/controllers/api/v1/accounts/follower_accounts_controller.rb index 950e9acf0e..e360b8a929 100644 --- a/app/controllers/api/v1/accounts/follower_accounts_controller.rb +++ b/app/controllers/api/v1/accounts/follower_accounts_controller.rb @@ -21,7 +21,9 @@ class Api::V1::Accounts::FollowerAccountsController < Api::BaseController def load_accounts return [] if hide_results? - default_accounts.merge(paginated_follows).to_a + scope = default_accounts + scope = scope.where.not(id: current_account.excluded_from_timeline_account_ids) unless current_account.nil? + scope.merge(paginated_follows).to_a end def hide_results? diff --git a/app/controllers/api/v1/accounts/following_accounts_controller.rb b/app/controllers/api/v1/accounts/following_accounts_controller.rb index b1433af5e4..a405b365f2 100644 --- a/app/controllers/api/v1/accounts/following_accounts_controller.rb +++ b/app/controllers/api/v1/accounts/following_accounts_controller.rb @@ -21,7 +21,9 @@ class Api::V1::Accounts::FollowingAccountsController < Api::BaseController def load_accounts return [] if hide_results? - default_accounts.merge(paginated_follows).to_a + scope = default_accounts + scope = scope.where.not(id: current_account.excluded_from_timeline_account_ids) unless current_account.nil? + scope.merge(paginated_follows).to_a end def hide_results? diff --git a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb index 657e578319..99eff360ec 100644 --- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb @@ -17,7 +17,9 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController private def load_accounts - default_accounts.merge(paginated_favourites).to_a + scope = default_accounts + scope = scope.where.not(id: current_account.excluded_from_timeline_account_ids) unless current_account.nil? + scope.merge(paginated_favourites).to_a end def default_accounts diff --git a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb index 6851099f66..cc285ad231 100644 --- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb @@ -17,7 +17,9 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController private def load_accounts - default_accounts.merge(paginated_statuses).to_a + scope = default_accounts + scope = scope.where.not(id: current_account.excluded_from_timeline_account_ids) unless current_account.nil? + scope.merge(paginated_statuses).to_a end def default_accounts diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index ef183eca7f..7103749ade 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -36,7 +36,11 @@ class FollowerAccountsController < ApplicationController private def follows - @follows ||= Follow.where(target_account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:account) + return @follows if defined?(@follows) + + scope = Follow.where(target_account: @account) + scope = scope.where.not(account_id: current_account.excluded_from_timeline_account_ids) if user_signed_in? + @follows = scope.recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:account) end def page_requested? diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index fb1eaaa377..6c8fb84d86 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -36,7 +36,11 @@ class FollowingAccountsController < ApplicationController private def follows - @follows ||= Follow.where(account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:target_account) + return @follows if defined?(@follows) + + scope = Follow.where(account: @account) + scope = scope.where.not(target_account_id: current_account.excluded_from_timeline_account_ids) if user_signed_in? + @follows = scope.recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:target_account) end def page_requested? diff --git a/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb b/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb index 75e0570e92..54587187ff 100644 --- a/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb @@ -3,19 +3,38 @@ require 'rails_helper' describe Api::V1::Accounts::FollowerAccountsController do render_views - let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') } + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') } + let(:account) { Fabricate(:account) } + let(:alice) { Fabricate(:account) } + let(:bob) { Fabricate(:account) } before do - Fabricate(:follow, target_account: user.account) + alice.follow!(account) + bob.follow!(account) allow(controller).to receive(:doorkeeper_token) { token } end describe 'GET #index' do it 'returns http success' do - get :index, params: { account_id: user.account.id, limit: 1 } + get :index, params: { account_id: account.id, limit: 2 } expect(response).to have_http_status(200) end + + it 'returns accounts following the given account' do + get :index, params: { account_id: account.id, limit: 2 } + + expect(body_as_json.size).to eq 2 + expect([body_as_json[0][:id], body_as_json[1][:id]]).to match_array([alice.id.to_s, bob.id.to_s]) + end + + it 'does not return blocked users' do + user.account.block!(bob) + get :index, params: { account_id: account.id, limit: 2 } + + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq alice.id.to_s + end end end diff --git a/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb b/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb index 7f7105ad3a..a580a73684 100644 --- a/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb @@ -3,19 +3,38 @@ require 'rails_helper' describe Api::V1::Accounts::FollowingAccountsController do render_views - let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') } + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') } + let(:account) { Fabricate(:account) } + let(:alice) { Fabricate(:account) } + let(:bob) { Fabricate(:account) } before do - Fabricate(:follow, account: user.account) + account.follow!(alice) + account.follow!(bob) allow(controller).to receive(:doorkeeper_token) { token } end describe 'GET #index' do it 'returns http success' do - get :index, params: { account_id: user.account.id, limit: 1 } + get :index, params: { account_id: account.id, limit: 2 } expect(response).to have_http_status(200) end + + it 'returns accounts followed by the given account' do + get :index, params: { account_id: account.id, limit: 2 } + + expect(body_as_json.size).to eq 2 + expect([body_as_json[0][:id], body_as_json[1][:id]]).to match_array([alice.id.to_s, bob.id.to_s]) + end + + it 'does not return blocked users' do + user.account.block!(bob) + get :index, params: { account_id: account.id, limit: 2 } + + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq alice.id.to_s + end end end diff --git a/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb b/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb index 40f75c700f..f053ae5738 100644 --- a/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb @@ -6,6 +6,8 @@ RSpec.describe Api::V1::Statuses::FavouritedByAccountsController, type: :control let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, application: app, scopes: 'read:accounts') } + let(:alice) { Fabricate(:account) } + let(:bob) { Fabricate(:account) } context 'with an oauth token' do before do @@ -16,14 +18,28 @@ RSpec.describe Api::V1::Statuses::FavouritedByAccountsController, type: :control let(:status) { Fabricate(:status, account: user.account) } before do - Fabricate(:favourite, status: status) + Favourite.create!(account: alice, status: status) + Favourite.create!(account: bob, status: status) end it 'returns http success' do - get :index, params: { status_id: status.id, limit: 1 } + get :index, params: { status_id: status.id, limit: 2 } expect(response).to have_http_status(200) expect(response.headers['Link'].links.size).to eq(2) end + + it 'returns accounts who favorited the status' do + get :index, params: { status_id: status.id, limit: 2 } + expect(body_as_json.size).to eq 2 + expect([body_as_json[0][:id], body_as_json[1][:id]]).to match_array([alice.id.to_s, bob.id.to_s]) + end + + it 'does not return blocked users' do + user.account.block!(bob) + get :index, params: { status_id: status.id, limit: 2 } + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq alice.id.to_s + end end end diff --git a/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb b/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb index d758786dc9..60908b7b38 100644 --- a/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb @@ -6,6 +6,8 @@ RSpec.describe Api::V1::Statuses::RebloggedByAccountsController, type: :controll let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, application: app, scopes: 'read:accounts') } + let(:alice) { Fabricate(:account) } + let(:bob) { Fabricate(:account) } context 'with an oauth token' do before do @@ -16,14 +18,28 @@ RSpec.describe Api::V1::Statuses::RebloggedByAccountsController, type: :controll let(:status) { Fabricate(:status, account: user.account) } before do - Fabricate(:status, reblog_of_id: status.id) + Fabricate(:status, account: alice, reblog_of_id: status.id) + Fabricate(:status, account: bob, reblog_of_id: status.id) end it 'returns http success' do - get :index, params: { status_id: status.id, limit: 1 } + get :index, params: { status_id: status.id, limit: 2 } expect(response).to have_http_status(200) expect(response.headers['Link'].links.size).to eq(2) end + + it 'returns accounts who reblogged the status' do + get :index, params: { status_id: status.id, limit: 2 } + expect(body_as_json.size).to eq 2 + expect([body_as_json[0][:id], body_as_json[1][:id]]).to match_array([alice.id.to_s, bob.id.to_s]) + end + + it 'does not return blocked users' do + user.account.block!(bob) + get :index, params: { status_id: status.id, limit: 2 } + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq alice.id.to_s + end end end diff --git a/spec/controllers/follower_accounts_controller_spec.rb b/spec/controllers/follower_accounts_controller_spec.rb index 83032ab646..34a0cf3f48 100644 --- a/spec/controllers/follower_accounts_controller_spec.rb +++ b/spec/controllers/follower_accounts_controller_spec.rb @@ -22,6 +22,18 @@ describe FollowerAccountsController do expect(assigned[0]).to eq follow1 expect(assigned[1]).to eq follow0 end + + it 'does not assign blocked users' do + user = Fabricate(:user) + user.account.block!(follower0) + sign_in(user) + + expect(response).to have_http_status(200) + + assigned = assigns(:follows).to_a + expect(assigned.size).to eq 1 + expect(assigned[0]).to eq follow1 + end end context 'when format is json' do diff --git a/spec/controllers/following_accounts_controller_spec.rb b/spec/controllers/following_accounts_controller_spec.rb index d5e4ee587f..e9a1f597df 100644 --- a/spec/controllers/following_accounts_controller_spec.rb +++ b/spec/controllers/following_accounts_controller_spec.rb @@ -22,6 +22,18 @@ describe FollowingAccountsController do expect(assigned[0]).to eq follow1 expect(assigned[1]).to eq follow0 end + + it 'does not assign blocked users' do + user = Fabricate(:user) + user.account.block!(followee0) + sign_in(user) + + expect(response).to have_http_status(200) + + assigned = assigns(:follows).to_a + expect(assigned.size).to eq 1 + expect(assigned[0]).to eq follow1 + end end context 'when format is json' do