From cb71c95e2292730befb8e12cf3b05d09d3e7443b Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 8 Apr 2019 07:28:27 +0200 Subject: [PATCH] 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 +- .../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 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