From 2b5faa2ba322d22cf9b345f815db7cbf89874415 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 19 Nov 2024 11:04:12 +0100 Subject: [PATCH] Change lists to reflect added and removed users retroactively (#32930) --- .../api/v1/lists/accounts_controller.rb | 13 ++------ app/models/follow_request.rb | 11 +++++-- app/services/add_accounts_to_list_service.rb | 33 +++++++++++++++++++ app/services/follow_service.rb | 5 ++- .../remove_accounts_from_list_service.rb | 29 ++++++++++++++++ app/services/unfollow_service.rb | 8 ++++- app/services/unmute_service.rb | 8 ++++- app/workers/merge_worker.rb | 30 +++++++++++++++-- app/workers/mute_worker.rb | 11 ++++++- app/workers/unmerge_worker.rb | 30 +++++++++++++++-- spec/models/follow_request_spec.rb | 2 +- spec/services/unmute_service_spec.rb | 2 +- 12 files changed, 157 insertions(+), 25 deletions(-) create mode 100644 app/services/add_accounts_to_list_service.rb create mode 100644 app/services/remove_accounts_from_list_service.rb diff --git a/app/controllers/api/v1/lists/accounts_controller.rb b/app/controllers/api/v1/lists/accounts_controller.rb index b1c0e609d0..616159f05f 100644 --- a/app/controllers/api/v1/lists/accounts_controller.rb +++ b/app/controllers/api/v1/lists/accounts_controller.rb @@ -15,17 +15,12 @@ class Api::V1::Lists::AccountsController < Api::BaseController end def create - ApplicationRecord.transaction do - list_accounts.each do |account| - @list.accounts << account - end - end - + AddAccountsToListService.new.call(@list, Account.find(account_ids)) render_empty end def destroy - ListAccount.where(list: @list, account_id: account_ids).destroy_all + RemoveAccountsFromListService.new.call(@list, Account.where(id: account_ids)) render_empty end @@ -43,10 +38,6 @@ class Api::V1::Lists::AccountsController < Api::BaseController end end - def list_accounts - Account.find(account_ids) - end - def account_ids Array(resource_params[:account_ids]) end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index c13cc718d8..964d4e279a 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -33,8 +33,15 @@ class FollowRequest < ApplicationRecord def authorize! follow = account.follow!(target_account, reblogs: show_reblogs, notify: notify, languages: languages, uri: uri, bypass_limit: true) - ListAccount.where(follow_request: self).update_all(follow_request_id: nil, follow_id: follow.id) - MergeWorker.perform_async(target_account.id, account.id) if account.local? + + if account.local? + ListAccount.where(follow_request: self).update_all(follow_request_id: nil, follow_id: follow.id) + MergeWorker.perform_async(target_account.id, account.id, 'home') + MergeWorker.push_bulk(List.where(account: account).joins(:list_accounts).where(list_accounts: { account_id: target_account.id }).pluck(:id)) do |list_id| + [target_account.id, list_id, 'list'] + end + end + destroy! end diff --git a/app/services/add_accounts_to_list_service.rb b/app/services/add_accounts_to_list_service.rb new file mode 100644 index 0000000000..df4e4c8314 --- /dev/null +++ b/app/services/add_accounts_to_list_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class AddAccountsToListService < BaseService + def call(list, accounts) + @list = list + @accounts = accounts + + return if @accounts.empty? + + update_list! + merge_into_list! + end + + private + + def update_list! + ApplicationRecord.transaction do + @accounts.each do |account| + @list.accounts << account + end + end + end + + def merge_into_list! + MergeWorker.push_bulk(merge_account_ids) do |account_id| + [account_id, @list.id, 'list'] + end + end + + def merge_account_ids + ListAccount.where(list: @list, account: @accounts).where.not(follow_id: nil).pluck(:account_id) + end +end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index af5f996077..cff38b8e6e 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -81,7 +81,10 @@ class FollowService < BaseService follow = @source_account.follow!(@target_account, **follow_options.merge(rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])) LocalNotificationWorker.perform_async(@target_account.id, follow.id, follow.class.name, 'follow') - MergeWorker.perform_async(@target_account.id, @source_account.id) + MergeWorker.perform_async(@target_account.id, @source_account.id, 'home') + MergeWorker.push_bulk(List.where(account: @source_account).joins(:list_accounts).where(list_accounts: { account_id: @target_account.id }).pluck(:id)) do |list_id| + [@target_account.id, list_id, 'list'] + end follow end diff --git a/app/services/remove_accounts_from_list_service.rb b/app/services/remove_accounts_from_list_service.rb new file mode 100644 index 0000000000..bd5b7c439e --- /dev/null +++ b/app/services/remove_accounts_from_list_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class RemoveAccountsFromListService < BaseService + def call(list, accounts) + @list = list + @accounts = accounts + + return if @accounts.empty? + + unmerge_from_list! + update_list! + end + + private + + def update_list! + ListAccount.where(list: @list, account: @accounts).destroy_all + end + + def unmerge_from_list! + UnmergeWorker.push_bulk(unmerge_account_ids) do |account_id| + [account_id, @list.id, 'list'] + end + end + + def unmerge_account_ids + ListAccount.where(list: @list, account: @accounts).where.not(follow_id: nil).pluck(:account_id) + end +end diff --git a/app/services/unfollow_service.rb b/app/services/unfollow_service.rb index fe9a7f0d87..b3f2cd66f6 100644 --- a/app/services/unfollow_service.rb +++ b/app/services/unfollow_service.rb @@ -31,7 +31,13 @@ class UnfollowService < BaseService create_notification(follow) if !@target_account.local? && @target_account.activitypub? create_reject_notification(follow) if @target_account.local? && !@source_account.local? && @source_account.activitypub? - UnmergeWorker.perform_async(@target_account.id, @source_account.id) unless @options[:skip_unmerge] + + unless @options[:skip_unmerge] + UnmergeWorker.perform_async(@target_account.id, @source_account.id, 'home') + UnmergeWorker.push_bulk(List.where(account: @source_account).joins(:list_accounts).where(list_accounts: { account_id: @target_account.id }).pluck(:list_id)) do |list_id| + [@target_account.id, list_id, 'list'] + end + end follow end diff --git a/app/services/unmute_service.rb b/app/services/unmute_service.rb index 6aeea358f7..9262961f7d 100644 --- a/app/services/unmute_service.rb +++ b/app/services/unmute_service.rb @@ -6,6 +6,12 @@ class UnmuteService < BaseService account.unmute!(target_account) - MergeWorker.perform_async(target_account.id, account.id) if account.following?(target_account) + if account.following?(target_account) + MergeWorker.perform_async(target_account.id, account.id, 'home') + + MergeWorker.push_bulk(List.where(account: account).joins(:list_accounts).where(list_accounts: { account_id: target_account.id }).pluck(:id)) do |list_id| + [target_account.id, list_id, 'list'] + end + end end end diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 8e1614ad21..d4dcb326bc 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -5,18 +5,42 @@ class MergeWorker include Redisable include DatabaseHelper - def perform(from_account_id, into_account_id) + def perform(from_account_id, into_id, type = 'home') with_primary do @from_account = Account.find(from_account_id) + end + + case type + when 'home' + merge_into_home!(into_id) + when 'list' + merge_into_list!(into_id) + end + rescue ActiveRecord::RecordNotFound + true + end + + private + + def merge_into_home!(into_account_id) + with_primary do @into_account = Account.find(into_account_id) end with_read_replica do FeedManager.instance.merge_into_home(@from_account, @into_account) end - rescue ActiveRecord::RecordNotFound - true ensure redis.del("account:#{into_account_id}:regeneration") end + + def merge_into_list!(into_list_id) + with_primary do + @into_list = List.find(into_list_id) + end + + with_read_replica do + FeedManager.instance.merge_into_list(@from_account, @into_list) + end + end end diff --git a/app/workers/mute_worker.rb b/app/workers/mute_worker.rb index c74f657cba..ebd401dc20 100644 --- a/app/workers/mute_worker.rb +++ b/app/workers/mute_worker.rb @@ -2,9 +2,18 @@ class MuteWorker include Sidekiq::Worker + include DatabaseHelper def perform(account_id, target_account_id) - FeedManager.instance.clear_from_home(Account.find(account_id), Account.find(target_account_id)) + with_primary do + @account = Account.find(account_id) + @target_account = Account.find(target_account_id) + end + + with_read_replica do + FeedManager.instance.clear_from_home(@account, @target_account) + FeedManager.instance.clear_from_lists(@account, @target_account) + end rescue ActiveRecord::RecordNotFound true end diff --git a/app/workers/unmerge_worker.rb b/app/workers/unmerge_worker.rb index e8ac535dfe..e8a3bf9b78 100644 --- a/app/workers/unmerge_worker.rb +++ b/app/workers/unmerge_worker.rb @@ -6,16 +6,40 @@ class UnmergeWorker sidekiq_options queue: 'pull' - def perform(from_account_id, into_account_id) + def perform(from_account_id, into_id, type = 'home') with_primary do @from_account = Account.find(from_account_id) + end + + case type + when 'home' + unmerge_from_home!(into_id) + when 'list' + unmerge_from_list!(into_id) + end + rescue ActiveRecord::RecordNotFound + true + end + + private + + def unmerge_from_home!(into_account_id) + with_primary do @into_account = Account.find(into_account_id) end with_read_replica do FeedManager.instance.unmerge_from_home(@from_account, @into_account) end - rescue ActiveRecord::RecordNotFound - true + end + + def unmerge_from_list!(into_list_id) + with_primary do + @into_list = List.find(into_list_id) + end + + with_read_replica do + FeedManager.instance.unmerge_from_list(@from_account, @into_list) + end end end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index 9cccb82903..237875deab 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -30,7 +30,7 @@ RSpec.describe FollowRequest do follow_request.authorize! expect(account).to have_received(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri, languages: nil, bypass_limit: true) - expect(MergeWorker).to have_received(:perform_async).with(target_account.id, account.id) + expect(MergeWorker).to have_received(:perform_async).with(target_account.id, account.id, 'home') expect(follow_request).to have_received(:destroy!) end diff --git a/spec/services/unmute_service_spec.rb b/spec/services/unmute_service_spec.rb index 92c7a70d65..a052e0dd0a 100644 --- a/spec/services/unmute_service_spec.rb +++ b/spec/services/unmute_service_spec.rb @@ -16,7 +16,7 @@ RSpec.describe UnmuteService do it 'removes the account mute and sets up a merge' do expect { subject.call(account, target_account) } .to remove_account_mute - expect(MergeWorker).to have_enqueued_sidekiq_job(target_account.id, account.id) + expect(MergeWorker).to have_enqueued_sidekiq_job(target_account.id, account.id, 'home') end end