Hide blocking accounts from blocked users (#10442)

* Revert "Add indication that you have been blocked in web UI (#10420)"

This reverts commit bd02ec6daa.

* Revert "Add `blocked_by` relationship to the REST API (#10373)"

This reverts commit 9745de883b.

* Hide blocking accounts from search results

* Filter blocking accouts from account followers

* Filter blocking accouts from account's following accounts

* Filter blocking accounts from “reblogged by” and “favourited by” lists

* Remove blocking account from URL search

* Return 410 on trying to fetch user data from a user who blocked us

* Return 410 in /api/v1/account/statuses for suspended or blocking accounts

* Fix status filtering when performing URL search

* Restore some React improvements

Restore some cleanup from bd02ec6daa

* Refactor by adding `without_blocking` scope
This commit is contained in:
ThibG 2019-04-01 20:06:13 +02:00 committed by Eugen Rochko
parent 67eb47e25f
commit 62bafa20a1
20 changed files with 136 additions and 62 deletions

View file

@ -25,7 +25,7 @@ class Api::V1::Accounts::FollowerAccountsController < Api::BaseController
end
def default_accounts
Account.includes(:active_relationships, :account_stat).references(:active_relationships)
Account.without_blocking(current_account).includes(:active_relationships, :account_stat).references(:active_relationships)
end
def paginated_follows

View file

@ -25,7 +25,7 @@ class Api::V1::Accounts::FollowingAccountsController < Api::BaseController
end
def default_accounts
Account.includes(:passive_relationships, :account_stat).references(:passive_relationships)
Account.without_blocking(current_account).includes(:passive_relationships, :account_stat).references(:passive_relationships)
end
def paginated_follows

View file

@ -3,6 +3,8 @@
class Api::V1::Accounts::StatusesController < Api::BaseController
before_action -> { authorize_if_got_token! :read, :'read:statuses' }
before_action :set_account
before_action :check_account_suspension
before_action :check_account_block
after_action :insert_pagination_headers
respond_to :json
@ -18,6 +20,14 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
@account = Account.find(params[:account_id])
end
def check_account_suspension
gone if @account.suspended?
end
def check_account_block
gone if current_account.present? && @account.blocking?(current_account)
end
def load_statuses
cached_account_statuses
end

View file

@ -10,6 +10,7 @@ class Api::V1::AccountsController < Api::BaseController
before_action :require_user!, except: [:show, :create]
before_action :set_account, except: [:create]
before_action :check_account_suspension, only: [:show]
before_action :check_account_block, only: [:show]
before_action :check_enabled_registrations, only: [:create]
respond_to :json
@ -75,6 +76,10 @@ class Api::V1::AccountsController < Api::BaseController
gone if @account.suspended?
end
def check_account_block
gone if current_account.present? && @account.blocking?(current_account)
end
def account_params
params.permit(:username, :email, :password, :agreement, :locale)
end

View file

@ -22,6 +22,7 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController
def default_accounts
Account
.without_blocking(current_account)
.includes(:favourites, :account_stat)
.references(:favourites)
.where(favourites: { status_id: @status.id })

View file

@ -21,7 +21,7 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController
end
def default_accounts
Account.includes(:statuses, :account_stat).references(:statuses)
Account.without_blocking(current_account).includes(:statuses, :account_stat).references(:statuses)
end
def paginated_statuses

View file

@ -111,7 +111,7 @@ class Header extends ImmutablePureComponent {
} else if (account.getIn(['relationship', 'requested'])) {
actionBtn = <Button className='logo-button' text={intl.formatMessage(messages.requested)} onClick={this.props.onFollow} />;
} else if (!account.getIn(['relationship', 'blocking'])) {
actionBtn = <Button disabled={account.getIn(['relationship', 'blocked_by'])} className={classNames('logo-button', { 'button--destructive': account.getIn(['relationship', 'following']) })} text={intl.formatMessage(account.getIn(['relationship', 'following']) ? messages.unfollow : messages.follow)} onClick={this.props.onFollow} />;
actionBtn = <Button className={classNames('logo-button', { 'button--destructive': account.getIn(['relationship', 'following']) })} text={intl.formatMessage(account.getIn(['relationship', 'following']) ? messages.unfollow : messages.follow)} onClick={this.props.onFollow} />;
} else if (account.getIn(['relationship', 'blocking'])) {
actionBtn = <Button className='logo-button' text={intl.formatMessage(messages.unblock, { name: account.get('username') })} onClick={this.props.onBlock} />;
}

View file

@ -14,17 +14,14 @@ import ImmutablePureComponent from 'react-immutable-pure-component';
import { FormattedMessage } from 'react-intl';
import { fetchAccountIdentityProofs } from '../../actions/identity_proofs';
const emptyList = ImmutableList();
const mapStateToProps = (state, { params: { accountId }, withReplies = false }) => {
const path = withReplies ? `${accountId}:with_replies` : accountId;
return {
statusIds: state.getIn(['timelines', `account:${path}`, 'items'], emptyList),
featuredStatusIds: withReplies ? ImmutableList() : state.getIn(['timelines', `account:${accountId}:pinned`, 'items'], emptyList),
statusIds: state.getIn(['timelines', `account:${path}`, 'items'], ImmutableList()),
featuredStatusIds: withReplies ? ImmutableList() : state.getIn(['timelines', `account:${accountId}:pinned`, 'items'], ImmutableList()),
isLoading: state.getIn(['timelines', `account:${path}`, 'isLoading']),
hasMore: state.getIn(['timelines', `account:${path}`, 'hasMore']),
blockedBy: state.getIn(['relationships', accountId, 'blocked_by'], false),
hasMore: state.getIn(['timelines', `account:${path}`, 'hasMore']),
};
};
@ -40,7 +37,6 @@ class AccountTimeline extends ImmutablePureComponent {
isLoading: PropTypes.bool,
hasMore: PropTypes.bool,
withReplies: PropTypes.bool,
blockedBy: PropTypes.bool,
};
componentWillMount () {
@ -48,11 +44,9 @@ class AccountTimeline extends ImmutablePureComponent {
this.props.dispatch(fetchAccount(accountId));
this.props.dispatch(fetchAccountIdentityProofs(accountId));
if (!withReplies) {
this.props.dispatch(expandAccountFeaturedTimeline(accountId));
}
this.props.dispatch(expandAccountTimeline(accountId, { withReplies }));
}
@ -60,11 +54,9 @@ class AccountTimeline extends ImmutablePureComponent {
if ((nextProps.params.accountId !== this.props.params.accountId && nextProps.params.accountId) || nextProps.withReplies !== this.props.withReplies) {
this.props.dispatch(fetchAccount(nextProps.params.accountId));
this.props.dispatch(fetchAccountIdentityProofs(nextProps.params.accountId));
if (!nextProps.withReplies) {
this.props.dispatch(expandAccountFeaturedTimeline(nextProps.params.accountId));
}
this.props.dispatch(expandAccountTimeline(nextProps.params.accountId, { withReplies: nextProps.params.withReplies }));
}
}
@ -74,7 +66,7 @@ class AccountTimeline extends ImmutablePureComponent {
}
render () {
const { shouldUpdateScroll, statusIds, featuredStatusIds, isLoading, hasMore, blockedBy } = this.props;
const { shouldUpdateScroll, statusIds, featuredStatusIds, isLoading, hasMore } = this.props;
if (!statusIds && isLoading) {
return (
@ -84,8 +76,6 @@ class AccountTimeline extends ImmutablePureComponent {
);
}
const emptyMessage = blockedBy ? <FormattedMessage id='empty_column.account_timeline_blocked' defaultMessage='You are blocked' /> : <FormattedMessage id='empty_column.account_timeline' defaultMessage='No toots here!' />;
return (
<Column>
<ColumnBackButton />
@ -94,13 +84,13 @@ class AccountTimeline extends ImmutablePureComponent {
prepend={<HeaderContainer accountId={this.props.params.accountId} />}
alwaysPrepend
scrollKey='account_timeline'
statusIds={blockedBy ? emptyList : statusIds}
statusIds={statusIds}
featuredStatusIds={featuredStatusIds}
isLoading={isLoading}
hasMore={hasMore}
onLoadMore={this.handleLoadMore}
shouldUpdateScroll={shouldUpdateScroll}
emptyMessage={emptyMessage}
emptyMessage={<FormattedMessage id='empty_column.account_timeline' defaultMessage='No toots here!' />}
/>
</Column>
);

View file

@ -20,7 +20,6 @@ import ScrollableList from '../../components/scrollable_list';
const mapStateToProps = (state, props) => ({
accountIds: state.getIn(['user_lists', 'followers', props.params.accountId, 'items']),
hasMore: !!state.getIn(['user_lists', 'followers', props.params.accountId, 'next']),
blockedBy: state.getIn(['relationships', props.params.accountId, 'blocked_by'], false),
});
export default @connect(mapStateToProps)
@ -32,7 +31,6 @@ class Followers extends ImmutablePureComponent {
shouldUpdateScroll: PropTypes.func,
accountIds: ImmutablePropTypes.list,
hasMore: PropTypes.bool,
blockedBy: PropTypes.bool,
};
componentWillMount () {
@ -52,7 +50,7 @@ class Followers extends ImmutablePureComponent {
}, 300, { leading: true });
render () {
const { shouldUpdateScroll, accountIds, hasMore, blockedBy } = this.props;
const { shouldUpdateScroll, accountIds, hasMore } = this.props;
if (!accountIds) {
return (
@ -62,7 +60,7 @@ class Followers extends ImmutablePureComponent {
);
}
const emptyMessage = blockedBy ? <FormattedMessage id='empty_column.account_timeline_blocked' defaultMessage='You are blocked' /> : <FormattedMessage id='account.followers.empty' defaultMessage='No one follows this user yet.' />;
const emptyMessage = <FormattedMessage id='account.followers.empty' defaultMessage='No one follows this user yet.' />;
return (
<Column>
@ -77,7 +75,7 @@ class Followers extends ImmutablePureComponent {
alwaysPrepend
emptyMessage={emptyMessage}
>
{blockedBy ? [] : accountIds.map(id =>
{accountIds.map(id =>
<AccountContainer key={id} id={id} withNote={false} />
)}
</ScrollableList>

View file

@ -20,7 +20,6 @@ import ScrollableList from '../../components/scrollable_list';
const mapStateToProps = (state, props) => ({
accountIds: state.getIn(['user_lists', 'following', props.params.accountId, 'items']),
hasMore: !!state.getIn(['user_lists', 'following', props.params.accountId, 'next']),
blockedBy: state.getIn(['relationships', props.params.accountId, 'blocked_by'], false),
});
export default @connect(mapStateToProps)
@ -32,7 +31,6 @@ class Following extends ImmutablePureComponent {
shouldUpdateScroll: PropTypes.func,
accountIds: ImmutablePropTypes.list,
hasMore: PropTypes.bool,
blockedBy: PropTypes.bool,
};
componentWillMount () {
@ -52,7 +50,7 @@ class Following extends ImmutablePureComponent {
}, 300, { leading: true });
render () {
const { shouldUpdateScroll, accountIds, hasMore, blockedBy } = this.props;
const { shouldUpdateScroll, accountIds, hasMore } = this.props;
if (!accountIds) {
return (
@ -62,7 +60,7 @@ class Following extends ImmutablePureComponent {
);
}
const emptyMessage = blockedBy ? <FormattedMessage id='empty_column.account_timeline_blocked' defaultMessage='You are blocked' /> : <FormattedMessage id='account.follows.empty' defaultMessage="This user doesn't follow anyone yet." />;
const emptyMessage = <FormattedMessage id='account.follows.empty' defaultMessage="This user doesn't follow anyone yet." />;
return (
<Column>
@ -77,7 +75,7 @@ class Following extends ImmutablePureComponent {
alwaysPrepend
emptyMessage={emptyMessage}
>
{blockedBy ? [] : accountIds.map(id =>
{accountIds.map(id =>
<AccountContainer key={id} id={id} withNote={false} />
)}
</ScrollableList>

View file

@ -99,9 +99,9 @@
}
}
&:active:not(:disabled),
&:focus:not(:disabled),
&:hover:not(:disabled) {
&:active,
&:focus,
&:hover {
background: lighten($ui-highlight-color, 10%);
svg path:last-child {

View file

@ -98,6 +98,7 @@ class Account < ApplicationRecord
scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) }
scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc')) }
scope :popular, -> { order('account_stats.followers_count desc') }
scope :without_blocking, ->(account) { account.nil? ? all : where.not(id: Block.where(target_account_id: account.id).pluck(&:account_id)) }
delegate :email,
:unconfirmed_email,

View file

@ -1,7 +1,7 @@
# frozen_string_literal: true
class AccountRelationshipsPresenter
attr_reader :following, :followed_by, :blocking, :blocked_by,
attr_reader :following, :followed_by, :blocking,
:muting, :requested, :domain_blocking,
:endorsed
@ -12,7 +12,6 @@ class AccountRelationshipsPresenter
@following = cached[:following].merge(Account.following_map(@uncached_account_ids, @current_account_id))
@followed_by = cached[:followed_by].merge(Account.followed_by_map(@uncached_account_ids, @current_account_id))
@blocking = cached[:blocking].merge(Account.blocking_map(@uncached_account_ids, @current_account_id))
@blocked_by = cached[:blocked_by].merge(Account.blocked_by_map(@uncached_account_ids, @current_account_id))
@muting = cached[:muting].merge(Account.muting_map(@uncached_account_ids, @current_account_id))
@requested = cached[:requested].merge(Account.requested_map(@uncached_account_ids, @current_account_id))
@domain_blocking = cached[:domain_blocking].merge(Account.domain_blocking_map(@uncached_account_ids, @current_account_id))
@ -23,7 +22,6 @@ class AccountRelationshipsPresenter
@following.merge!(options[:following_map] || {})
@followed_by.merge!(options[:followed_by_map] || {})
@blocking.merge!(options[:blocking_map] || {})
@blocked_by.merge!(options[:blocked_by_map] || {})
@muting.merge!(options[:muting_map] || {})
@requested.merge!(options[:requested_map] || {})
@domain_blocking.merge!(options[:domain_blocking_map] || {})
@ -39,7 +37,6 @@ class AccountRelationshipsPresenter
following: {},
followed_by: {},
blocking: {},
blocked_by: {},
muting: {},
requested: {},
domain_blocking: {},
@ -67,7 +64,6 @@ class AccountRelationshipsPresenter
following: { account_id => following[account_id] },
followed_by: { account_id => followed_by[account_id] },
blocking: { account_id => blocking[account_id] },
blocked_by: { account_id => blocked_by[account_id] },
muting: { account_id => muting[account_id] },
requested: { account_id => requested[account_id] },
domain_blocking: { account_id => domain_blocking[account_id] },

View file

@ -1,7 +1,7 @@
# frozen_string_literal: true
class REST::RelationshipSerializer < ActiveModel::Serializer
attributes :id, :following, :showing_reblogs, :followed_by, :blocking, :blocked_by,
attributes :id, :following, :showing_reblogs, :followed_by, :blocking,
:muting, :muting_notifications, :requested, :domain_blocking,
:endorsed
@ -27,10 +27,6 @@ class REST::RelationshipSerializer < ActiveModel::Serializer
instance_options[:relationships].blocking[object.id] || false
end
def blocked_by
instance_options[:relationships].blocked_by[object.id] || false
end
def muting
instance_options[:relationships].muting[object.id] ? true : false
end

View file

@ -10,7 +10,15 @@ class AccountSearchService < BaseService
@options = options
@account = account
search_service_results
results = search_service_results
unless account.nil?
account_ids = results.map(&:id)
blocked_by_map = Account.blocked_by_map(account_ids, account.id)
results.reject! { |item| blocked_by_map[item.id] }
end
results
end
private

View file

@ -12,6 +12,8 @@ class SearchService < BaseService
default_results.tap do |results|
if url_query?
results.merge!(url_resource_results) unless url_resource.nil?
results[:accounts].reject! { |item| item.blocking?(@account) }
results[:statuses].reject! { |status| StatusFilter.new(status, @account).filtered? }
elsif @query.present?
results[:accounts] = perform_accounts_search! if account_searchable?
results[:statuses] = perform_statuses_search! if full_text_searchable?

View file

@ -7,15 +7,40 @@ describe Api::V1::Accounts::FollowerAccountsController do
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') }
before do
Fabricate(:follow, target_account: user.account)
allow(controller).to receive(:doorkeeper_token) { token }
end
describe 'GET #index' do
let(:simon) { Fabricate(:account, username: 'simon') }
let(:lewis) { Fabricate(:account, username: 'lewis') }
before do
simon.follow!(lewis)
end
it 'returns http success' do
get :index, params: { account_id: user.account.id, limit: 1 }
get :index, params: { account_id: lewis.id, limit: 1 }
expect(response).to have_http_status(200)
end
it 'returns JSON with correct data' do
get :index, params: { account_id: lewis.id, limit: 1 }
json = body_as_json
expect(json).to be_a Enumerable
expect(json.first[:username]).to eq 'simon'
end
it 'does not return accounts blocking you' do
simon.block!(user.account)
get :index, params: { account_id: lewis.id, limit: 1 }
json = body_as_json
expect(json).to be_a Enumerable
expect(json.size).to eq 0
end
end
end

View file

@ -7,15 +7,40 @@ describe Api::V1::Accounts::FollowingAccountsController do
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') }
before do
Fabricate(:follow, account: user.account)
allow(controller).to receive(:doorkeeper_token) { token }
end
describe 'GET #index' do
let(:simon) { Fabricate(:account, username: 'simon') }
let(:lewis) { Fabricate(:account, username: 'lewis') }
before do
lewis.follow!(simon)
end
it 'returns http success' do
get :index, params: { account_id: user.account.id, limit: 1 }
get :index, params: { account_id: lewis.id, limit: 1 }
expect(response).to have_http_status(200)
end
it 'returns JSON with correct data' do
get :index, params: { account_id: lewis.id, limit: 1 }
json = body_as_json
expect(json).to be_a Enumerable
expect(json.first[:username]).to eq 'simon'
end
it 'does not return accounts blocking you' do
simon.block!(user.account)
get :index, params: { account_id: lewis.id, limit: 1 }
json = body_as_json
expect(json).to be_a Enumerable
expect(json.size).to eq 0
end
end
end

View file

@ -156,5 +156,22 @@ describe AccountSearchService, type: :service do
expect(results).to eq []
end
end
describe 'should not include accounts blocking the requester' do
let!(:blocked) { Fabricate(:account) }
let!(:blocker) { Fabricate(:account, username: 'exact') }
before do
blocker.block!(blocked)
end
it 'returns the fuzzy match first, and does not return suspended exacts' do
partial = Fabricate(:account, username: 'exactness')
results = subject.call('exact', blocked, limit: 10)
expect(results.size).to eq 1
expect(results).to eq [partial]
end
end
end
end

View file

@ -3,6 +3,8 @@
require 'rails_helper'
describe SearchService, type: :service do
let(:current_account) { Fabricate(:user).account }
subject { described_class.new }
describe '#call' do
@ -10,7 +12,7 @@ describe SearchService, type: :service do
it 'returns empty results without searching' do
allow(AccountSearchService).to receive(:new)
allow(Tag).to receive(:search_for)
results = subject.call('', nil, 10)
results = subject.call('', current_account, 10)
expect(results).to eq(empty_results)
expect(AccountSearchService).not_to have_received(:new)
@ -27,33 +29,33 @@ describe SearchService, type: :service do
it 'returns the empty results' do
service = double(call: nil)
allow(ResolveURLService).to receive(:new).and_return(service)
results = subject.call(@query, nil, 10)
results = subject.call(@query, current_account, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
expect(service).to have_received(:call).with(@query, on_behalf_of: current_account)
expect(results).to eq empty_results
end
end
context 'that finds an account' do
it 'includes the account in the results' do
account = Account.new
account = Fabricate(:account)
service = double(call: account)
allow(ResolveURLService).to receive(:new).and_return(service)
results = subject.call(@query, nil, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
results = subject.call(@query, current_account, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: current_account)
expect(results).to eq empty_results.merge(accounts: [account])
end
end
context 'that finds a status' do
it 'includes the status in the results' do
status = Status.new
status = Fabricate(:status)
service = double(call: status)
allow(ResolveURLService).to receive(:new).and_return(service)
results = subject.call(@query, nil, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
results = subject.call(@query, current_account, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: current_account)
expect(results).to eq empty_results.merge(statuses: [status])
end
end
@ -63,12 +65,12 @@ describe SearchService, type: :service do
context 'that matches an account' do
it 'includes the account in the results' do
query = 'username'
account = Account.new
account = Fabricate(:account)
service = double(call: [account])
allow(AccountSearchService).to receive(:new).and_return(service)
results = subject.call(query, nil, 10)
expect(service).to have_received(:call).with(query, nil, limit: 10, offset: 0, resolve: false)
results = subject.call(query, current_account, 10)
expect(service).to have_received(:call).with(query, current_account, limit: 10, offset: 0, resolve: false)
expect(results).to eq empty_results.merge(accounts: [account])
end
end
@ -79,7 +81,7 @@ describe SearchService, type: :service do
tag = Tag.new
allow(Tag).to receive(:search_for).with('tag', 10, 0).and_return([tag])
results = subject.call(query, nil, 10)
results = subject.call(query, current_account, 10)
expect(Tag).to have_received(:search_for).with('tag', 10, 0)
expect(results).to eq empty_results.merge(hashtags: [tag])
end
@ -87,7 +89,7 @@ describe SearchService, type: :service do
query = '@username'
allow(Tag).to receive(:search_for)
results = subject.call(query, nil, 10)
results = subject.call(query, current_account, 10)
expect(Tag).not_to have_received(:search_for)
expect(results).to eq empty_results
end