From 67b3b62b98b89f24097a2757e42bc94f1bce123c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 7 Apr 2019 04:59:13 +0200 Subject: Improve blocked view of profiles (#10491) * Revert "Fix filtering of favourited_by, reblogged_by, followers and following (#10447)" This reverts commit 120544067fcca4bf6e71ba1ffb276c451c17c656. * Revert "Hide blocking accounts from blocked users (#10442)" This reverts commit 62bafa20a112ccdddaedb25723fc819dbbcd8e9a. * Improve blocked view of profiles - Change "You are blocked" to "Profile unavailable" - Hide following/followers in API when blocked - Disable follow button and show "Profile unavailable" on public profile as well --- .../accounts/follower_accounts_controller_spec.rb | 29 ++------------------- .../accounts/following_accounts_controller_spec.rb | 29 ++------------------- spec/services/account_search_service_spec.rb | 17 ------------ spec/services/search_service_spec.rb | 30 ++++++++++------------ 4 files changed, 18 insertions(+), 87 deletions(-) (limited to 'spec') 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 42a18cdc3..75e0570e9 100644 --- a/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb @@ -7,40 +7,15 @@ 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: lewis.id, limit: 1 } + get :index, params: { account_id: user.account.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 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 911b381fe..7f7105ad3 100644 --- a/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb @@ -7,40 +7,15 @@ 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: lewis.id, limit: 1 } + get :index, params: { account_id: user.account.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 diff --git a/spec/services/account_search_service_spec.rb b/spec/services/account_search_service_spec.rb index 40ef4b84a..7b071b378 100644 --- a/spec/services/account_search_service_spec.rb +++ b/spec/services/account_search_service_spec.rb @@ -156,22 +156,5 @@ 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 diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb index 900533e71..d064cd9b8 100644 --- a/spec/services/search_service_spec.rb +++ b/spec/services/search_service_spec.rb @@ -3,8 +3,6 @@ require 'rails_helper' describe SearchService, type: :service do - let(:current_account) { Fabricate(:user).account } - subject { described_class.new } describe '#call' do @@ -12,7 +10,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('', current_account, 10) + results = subject.call('', nil, 10) expect(results).to eq(empty_results) expect(AccountSearchService).not_to have_received(:new) @@ -29,33 +27,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, current_account, 10) + results = subject.call(@query, nil, 10) - expect(service).to have_received(:call).with(@query, on_behalf_of: current_account) + expect(service).to have_received(:call).with(@query, on_behalf_of: nil) expect(results).to eq empty_results end end context 'that finds an account' do it 'includes the account in the results' do - account = Fabricate(:account) + account = Account.new service = double(call: account) allow(ResolveURLService).to receive(:new).and_return(service) - results = subject.call(@query, current_account, 10) - expect(service).to have_received(:call).with(@query, on_behalf_of: current_account) + results = subject.call(@query, nil, 10) + expect(service).to have_received(:call).with(@query, on_behalf_of: nil) 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 = Fabricate(:status) + status = Status.new service = double(call: status) allow(ResolveURLService).to receive(:new).and_return(service) - results = subject.call(@query, current_account, 10) - expect(service).to have_received(:call).with(@query, on_behalf_of: current_account) + results = subject.call(@query, nil, 10) + expect(service).to have_received(:call).with(@query, on_behalf_of: nil) expect(results).to eq empty_results.merge(statuses: [status]) end end @@ -65,12 +63,12 @@ describe SearchService, type: :service do context 'that matches an account' do it 'includes the account in the results' do query = 'username' - account = Fabricate(:account) + account = Account.new service = double(call: [account]) allow(AccountSearchService).to receive(:new).and_return(service) - results = subject.call(query, current_account, 10) - expect(service).to have_received(:call).with(query, current_account, limit: 10, offset: 0, resolve: false) + results = subject.call(query, nil, 10) + expect(service).to have_received(:call).with(query, nil, limit: 10, offset: 0, resolve: false) expect(results).to eq empty_results.merge(accounts: [account]) end end @@ -81,7 +79,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, current_account, 10) + results = subject.call(query, nil, 10) expect(Tag).to have_received(:search_for).with('tag', 10, 0) expect(results).to eq empty_results.merge(hashtags: [tag]) end @@ -89,7 +87,7 @@ describe SearchService, type: :service do query = '@username' allow(Tag).to receive(:search_for) - results = subject.call(query, current_account, 10) + results = subject.call(query, nil, 10) expect(Tag).not_to have_received(:search_for) expect(results).to eq empty_results end -- cgit From cb71c95e2292730befb8e12cf3b05d09d3e7443b Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 8 Apr 2019 07:28:27 +0200 Subject: Export and import show_reblogs together with following list (#10495) * Refactor imports * Export show_reblogs when exporting list of followed users * Add support for importing show_reblogs with following collection * Fix tests --- app/models/export.rb | 6 +- app/services/import_service.rb | 12 +-- app/workers/import/relationship_worker.rb | 7 +- .../exports/following_accounts_controller_spec.rb | 2 +- spec/fixtures/files/new-following-imports.txt | 4 + spec/models/export_spec.rb | 7 +- spec/services/import_service_spec.rb | 91 +++++++++++++++++++++- 7 files changed, 112 insertions(+), 17 deletions(-) create mode 100644 spec/fixtures/files/new-following-imports.txt (limited to 'spec') diff --git a/app/models/export.rb b/app/models/export.rb index b35632c60..cab01f11a 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -22,7 +22,11 @@ class Export end def to_following_accounts_csv - to_csv account.following.select(:username, :domain) + CSV.generate(headers: ['Account address', 'Show boosts'], write_headers: true) do |csv| + account.active_relationships.includes(:target_account).reorder(id: :desc).each do |follow| + csv << [acct(follow.target_account), follow.show_reblogs] + end + end end def to_lists_csv diff --git a/app/services/import_service.rb b/app/services/import_service.rb index c1c88e0dd..4ee431ea3 100644 --- a/app/services/import_service.rb +++ b/app/services/import_service.rb @@ -25,7 +25,7 @@ class ImportService < BaseService def import_follows! parse_import_data!(['Account address']) - import_relationships!('follow', 'unfollow', @account.following, follow_limit) + import_relationships!('follow', 'unfollow', @account.following, follow_limit, reblogs: 'Show boosts') end def import_blocks! @@ -35,7 +35,7 @@ class ImportService < BaseService def import_mutes! parse_import_data!(['Account address']) - import_relationships!('mute', 'unmute', @account.muting, ROWS_PROCESSING_LIMIT) + import_relationships!('mute', 'unmute', @account.muting, ROWS_PROCESSING_LIMIT, notifications: 'Hide notifications') end def import_domain_blocks! @@ -63,8 +63,8 @@ class ImportService < BaseService end end - def import_relationships!(action, undo_action, overwrite_scope, limit) - items = @data.take(limit).map { |row| [row['Account address']&.strip, row['Hide notifications']&.strip] }.reject { |(id, _)| id.blank? } + def import_relationships!(action, undo_action, overwrite_scope, limit, extra_fields = {}) + items = @data.take(limit).map { |row| [row['Account address']&.strip, Hash[extra_fields.map { |key, header| [key, row[header]&.strip] }]] }.reject { |(id, _)| id.blank? } if @import.overwrite? presence_hash = items.each_with_object({}) { |(id, extra), mapping| mapping[id] = [true, extra] } @@ -73,7 +73,7 @@ class ImportService < BaseService if presence_hash[target_account.acct] items.delete(target_account.acct) extra = presence_hash[target_account.acct][1] - Import::RelationshipWorker.perform_async(@account.id, target_account.acct, action, ActiveModel::Type::Boolean.new.cast(extra)) + Import::RelationshipWorker.perform_async(@account.id, target_account.acct, action, extra) else Import::RelationshipWorker.perform_async(@account.id, target_account.acct, undo_action) end @@ -81,7 +81,7 @@ class ImportService < BaseService end Import::RelationshipWorker.push_bulk(items) do |acct, extra| - [@account.id, acct, action, ActiveModel::Type::Boolean.new.cast(extra)] + [@account.id, acct, action, extra] end end diff --git a/app/workers/import/relationship_worker.rb b/app/workers/import/relationship_worker.rb index 43ec09ea2..616da6da9 100644 --- a/app/workers/import/relationship_worker.rb +++ b/app/workers/import/relationship_worker.rb @@ -5,15 +5,16 @@ class Import::RelationshipWorker sidekiq_options queue: 'pull', retry: 8, dead: false - def perform(account_id, target_account_uri, relationship, extra = nil) + def perform(account_id, target_account_uri, relationship, options = {}) from_account = Account.find(account_id) target_account = ResolveAccountService.new.call(target_account_uri) + options.symbolize_keys! return if target_account.nil? case relationship when 'follow' - FollowService.new.call(from_account, target_account) + FollowService.new.call(from_account, target_account, options) when 'unfollow' UnfollowService.new.call(from_account, target_account) when 'block' @@ -21,7 +22,7 @@ class Import::RelationshipWorker when 'unblock' UnblockService.new.call(from_account, target_account) when 'mute' - MuteService.new.call(from_account, target_account, notifications: extra) + MuteService.new.call(from_account, target_account, options) when 'unmute' UnmuteService.new.call(from_account, target_account) end diff --git a/spec/controllers/settings/exports/following_accounts_controller_spec.rb b/spec/controllers/settings/exports/following_accounts_controller_spec.rb index 786769d24..78858e772 100644 --- a/spec/controllers/settings/exports/following_accounts_controller_spec.rb +++ b/spec/controllers/settings/exports/following_accounts_controller_spec.rb @@ -11,7 +11,7 @@ describe Settings::Exports::FollowingAccountsController do sign_in user, scope: :user get :index, format: :csv - expect(response.body).to eq "username@domain\n" + expect(response.body).to eq "Account address,Show boosts\nusername@domain,true\n" end end end diff --git a/spec/fixtures/files/new-following-imports.txt b/spec/fixtures/files/new-following-imports.txt new file mode 100644 index 000000000..5ea6c7346 --- /dev/null +++ b/spec/fixtures/files/new-following-imports.txt @@ -0,0 +1,4 @@ +Account address,Show boosts +bob,true +eve@example.com,false + diff --git a/spec/models/export_spec.rb b/spec/models/export_spec.rb index 0553f4098..4e6b824bb 100644 --- a/spec/models/export_spec.rb +++ b/spec/models/export_spec.rb @@ -32,10 +32,11 @@ describe Export do target_accounts.each(&account.method(:follow!)) export = Export.new(account).to_following_accounts_csv - results = export.strip.split + results = export.strip.split("\n") - expect(results.size).to eq 2 - expect(results.first).to eq 'one@local.host' + expect(results.size).to eq 3 + expect(results.first).to eq 'Account address,Show boosts' + expect(results.second).to eq 'one@local.host,true' end end diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index bd23781ce..5cf2dadf0 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/import_service_spec.rb @@ -1,9 +1,9 @@ require 'rails_helper' RSpec.describe ImportService, type: :service do - let!(:account) { Fabricate(:account) } - let!(:bob) { Fabricate(:account, username: 'bob') } - let!(:eve) { Fabricate(:account, username: 'eve', domain: 'example.com') } + let!(:account) { Fabricate(:account, locked: false) } + let!(:bob) { Fabricate(:account, username: 'bob', locked: false) } + let!(:eve) { Fabricate(:account, username: 'eve', domain: 'example.com', locked: false) } context 'import old-style list of muted users' do subject { ImportService.new } @@ -81,4 +81,89 @@ RSpec.describe ImportService, type: :service do end end end + + context 'import old-style list of followed users' do + subject { ImportService.new } + + let(:csv) { attachment_fixture('mute-imports.txt') } + + before do + allow(NotificationWorker).to receive(:perform_async) + end + + describe 'when no accounts are followed' do + let(:import) { Import.create(account: account, type: 'following', data: csv) } + it 'follows the listed accounts, including boosts' do + subject.call(import) + expect(account.following.count).to eq 2 + expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true + end + end + + describe 'when some accounts are already followed and overwrite is not set' do + let(:import) { Import.create(account: account, type: 'following', data: csv) } + + it 'follows the listed accounts, including notifications' do + account.follow!(bob, reblogs: false) + subject.call(import) + expect(account.following.count).to eq 2 + expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true + end + end + + describe 'when some accounts are already followed and overwrite is set' do + let(:import) { Import.create(account: account, type: 'following', data: csv, overwrite: true) } + + it 'mutes the listed accounts, including notifications' do + account.follow!(bob, reblogs: false) + subject.call(import) + expect(account.following.count).to eq 2 + expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true + end + end + end + + context 'import new-style list of followed users' do + subject { ImportService.new } + + let(:csv) { attachment_fixture('new-following-imports.txt') } + + before do + allow(NotificationWorker).to receive(:perform_async) + end + + describe 'when no accounts are followed' do + let(:import) { Import.create(account: account, type: 'following', data: csv) } + it 'follows the listed accounts, respecting boosts' do + subject.call(import) + expect(account.following.count).to eq 2 + expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true + expect(Follow.find_by(account: account, target_account: eve).show_reblogs).to be false + end + end + + describe 'when some accounts are already followed and overwrite is not set' do + let(:import) { Import.create(account: account, type: 'following', data: csv) } + + it 'mutes the listed accounts, respecting notifications' do + account.follow!(bob, reblogs: true) + subject.call(import) + expect(account.following.count).to eq 2 + expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true + expect(Follow.find_by(account: account, target_account: eve).show_reblogs).to be false + end + end + + describe 'when some accounts are already followed and overwrite is set' do + let(:import) { Import.create(account: account, type: 'following', data: csv, overwrite: true) } + + it 'mutes the listed accounts, respecting notifications' do + account.follow!(bob, reblogs: true) + subject.call(import) + expect(account.following.count).to eq 2 + expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true + expect(Follow.find_by(account: account, target_account: eve).show_reblogs).to be false + end + end + end end -- cgit