From d4882aa64a7a483ec30ca61a507c2a6f5d3294f5 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 3 Apr 2019 18:17:43 +0200 Subject: [PATCH] Export and import `hide_notifications` alongside user mutes (#10335) * Export hide_notifications along with user mutes * Import hide_notifications along with muted users list * Add headers for CSV exports --- app/models/export.rb | 6 +- app/services/import_service.rb | 23 +++-- app/workers/import/relationship_worker.rb | 4 +- .../exports/muted_accounts_controller_spec.rb | 2 +- spec/fixtures/files/mute-imports.txt | 4 + spec/fixtures/files/new-mute-imports.txt | 4 + spec/models/export_spec.rb | 7 +- spec/services/import_service_spec.rb | 84 +++++++++++++++++++ 8 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 spec/fixtures/files/mute-imports.txt create mode 100644 spec/fixtures/files/new-mute-imports.txt create mode 100644 spec/services/import_service_spec.rb diff --git a/app/models/export.rb b/app/models/export.rb index 9bf866d35..b35632c60 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -14,7 +14,11 @@ class Export end def to_muted_accounts_csv - to_csv account.muting.select(:username, :domain) + CSV.generate(headers: ['Account address', 'Hide notifications'], write_headers: true) do |csv| + account.mute_relationships.includes(:target_account).reorder(id: :desc).each do |mute| + csv << [acct(mute.target_account), mute.hide_notifications] + end + end end def to_following_accounts_csv diff --git a/app/services/import_service.rb b/app/services/import_service.rb index 3f558626e..c1c88e0dd 100644 --- a/app/services/import_service.rb +++ b/app/services/import_service.rb @@ -8,7 +8,6 @@ class ImportService < BaseService def call(import) @import = import @account = @import.account - @data = CSV.new(import_data).reject(&:blank?) case @import.type when 'following' @@ -25,19 +24,23 @@ class ImportService < BaseService private def import_follows! + parse_import_data!(['Account address']) import_relationships!('follow', 'unfollow', @account.following, follow_limit) end def import_blocks! + parse_import_data!(['Account address']) import_relationships!('block', 'unblock', @account.blocking, ROWS_PROCESSING_LIMIT) end def import_mutes! + parse_import_data!(['Account address']) import_relationships!('mute', 'unmute', @account.muting, ROWS_PROCESSING_LIMIT) end def import_domain_blocks! - items = @data.take(ROWS_PROCESSING_LIMIT).map { |row| row.first.strip } + parse_import_data!(['#domain']) + items = @data.take(ROWS_PROCESSING_LIMIT).map { |row| row['#domain'].strip } if @import.overwrite? presence_hash = items.each_with_object({}) { |id, mapping| mapping[id] = true } @@ -61,25 +64,33 @@ class ImportService < BaseService end def import_relationships!(action, undo_action, overwrite_scope, limit) - items = @data.take(limit).map { |row| row.first.strip } + items = @data.take(limit).map { |row| [row['Account address']&.strip, row['Hide notifications']&.strip] }.reject { |(id, _)| id.blank? } if @import.overwrite? - presence_hash = items.each_with_object({}) { |id, mapping| mapping[id] = true } + presence_hash = items.each_with_object({}) { |(id, extra), mapping| mapping[id] = [true, extra] } overwrite_scope.find_each do |target_account| 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)) else Import::RelationshipWorker.perform_async(@account.id, target_account.acct, undo_action) end end end - Import::RelationshipWorker.push_bulk(items) do |acct| - [@account.id, acct, action] + Import::RelationshipWorker.push_bulk(items) do |acct, extra| + [@account.id, acct, action, ActiveModel::Type::Boolean.new.cast(extra)] end end + def parse_import_data!(default_headers) + data = CSV.parse(import_data, headers: true) + data = CSV.parse(import_data, headers: default_headers) unless data.headers&.first&.strip&.include?(' ') + @data = data.reject(&:blank?) + end + def import_data Paperclip.io_adapters.for(@import.data).read end diff --git a/app/workers/import/relationship_worker.rb b/app/workers/import/relationship_worker.rb index e9db20a46..43ec09ea2 100644 --- a/app/workers/import/relationship_worker.rb +++ b/app/workers/import/relationship_worker.rb @@ -5,7 +5,7 @@ class Import::RelationshipWorker sidekiq_options queue: 'pull', retry: 8, dead: false - def perform(account_id, target_account_uri, relationship) + def perform(account_id, target_account_uri, relationship, extra = nil) from_account = Account.find(account_id) target_account = ResolveAccountService.new.call(target_account_uri) @@ -21,7 +21,7 @@ class Import::RelationshipWorker when 'unblock' UnblockService.new.call(from_account, target_account) when 'mute' - MuteService.new.call(from_account, target_account) + MuteService.new.call(from_account, target_account, notifications: extra) when 'unmute' UnmuteService.new.call(from_account, target_account) end diff --git a/spec/controllers/settings/exports/muted_accounts_controller_spec.rb b/spec/controllers/settings/exports/muted_accounts_controller_spec.rb index f42d7881e..642f0a9b8 100644 --- a/spec/controllers/settings/exports/muted_accounts_controller_spec.rb +++ b/spec/controllers/settings/exports/muted_accounts_controller_spec.rb @@ -11,7 +11,7 @@ describe Settings::Exports::MutedAccountsController 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,Hide notifications\nusername@domain,true\n" end end end diff --git a/spec/fixtures/files/mute-imports.txt b/spec/fixtures/files/mute-imports.txt new file mode 100644 index 000000000..125cbd384 --- /dev/null +++ b/spec/fixtures/files/mute-imports.txt @@ -0,0 +1,4 @@ +bob + +eve@example.com + diff --git a/spec/fixtures/files/new-mute-imports.txt b/spec/fixtures/files/new-mute-imports.txt new file mode 100644 index 000000000..c1c9bca9b --- /dev/null +++ b/spec/fixtures/files/new-mute-imports.txt @@ -0,0 +1,4 @@ +Account address,Hide notifications +bob,true +eve@example.com,false + diff --git a/spec/models/export_spec.rb b/spec/models/export_spec.rb index 277dcc526..0553f4098 100644 --- a/spec/models/export_spec.rb +++ b/spec/models/export_spec.rb @@ -21,10 +21,11 @@ describe Export do target_accounts.each(&account.method(:mute!)) export = Export.new(account).to_muted_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,Hide notifications' + expect(results.second).to eq 'one@local.host,true' end it 'returns a csv of the following accounts' do diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb new file mode 100644 index 000000000..bd23781ce --- /dev/null +++ b/spec/services/import_service_spec.rb @@ -0,0 +1,84 @@ +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') } + + context 'import old-style list of muted users' do + subject { ImportService.new } + + let(:csv) { attachment_fixture('mute-imports.txt') } + + describe 'when no accounts are muted' do + let(:import) { Import.create(account: account, type: 'muting', data: csv) } + it 'mutes the listed accounts, including notifications' do + subject.call(import) + expect(account.muting.count).to eq 2 + expect(Mute.find_by(account: account, target_account: bob).hide_notifications).to be true + end + end + + describe 'when some accounts are muted and overwrite is not set' do + let(:import) { Import.create(account: account, type: 'muting', data: csv) } + + it 'mutes the listed accounts, including notifications' do + account.mute!(bob, notifications: false) + subject.call(import) + expect(account.muting.count).to eq 2 + expect(Mute.find_by(account: account, target_account: bob).hide_notifications).to be true + end + end + + describe 'when some accounts are muted and overwrite is set' do + let(:import) { Import.create(account: account, type: 'muting', data: csv, overwrite: true) } + + it 'mutes the listed accounts, including notifications' do + account.mute!(bob, notifications: false) + subject.call(import) + expect(account.muting.count).to eq 2 + expect(Mute.find_by(account: account, target_account: bob).hide_notifications).to be true + end + end + end + + context 'import new-style list of muted users' do + subject { ImportService.new } + + let(:csv) { attachment_fixture('new-mute-imports.txt') } + + describe 'when no accounts are muted' do + let(:import) { Import.create(account: account, type: 'muting', data: csv) } + it 'mutes the listed accounts, respecting notifications' do + subject.call(import) + expect(account.muting.count).to eq 2 + expect(Mute.find_by(account: account, target_account: bob).hide_notifications).to be true + expect(Mute.find_by(account: account, target_account: eve).hide_notifications).to be false + end + end + + describe 'when some accounts are muted and overwrite is not set' do + let(:import) { Import.create(account: account, type: 'muting', data: csv) } + + it 'mutes the listed accounts, respecting notifications' do + account.mute!(bob, notifications: true) + subject.call(import) + expect(account.muting.count).to eq 2 + expect(Mute.find_by(account: account, target_account: bob).hide_notifications).to be true + expect(Mute.find_by(account: account, target_account: eve).hide_notifications).to be false + end + end + + describe 'when some accounts are muted and overwrite is set' do + let(:import) { Import.create(account: account, type: 'muting', data: csv, overwrite: true) } + + it 'mutes the listed accounts, respecting notifications' do + account.mute!(bob, notifications: true) + subject.call(import) + expect(account.muting.count).to eq 2 + expect(Mute.find_by(account: account, target_account: bob).hide_notifications).to be true + expect(Mute.find_by(account: account, target_account: eve).hide_notifications).to be false + end + end + end +end