Change notification request acceptance to immediately delete the request (#31256)

4 files changed, 83 insertions, 6 deletions

diff --git a/app/services/accept_notification_request_service.rb b/app/services/accept_notification_request_service.rb
index e49eae6fd30..ad27ae33007 100644
--- a/app/services/accept_notification_request_service.rb
+++ b/app/services/accept_notification_request_service.rb
@@ -3,6 +3,7 @@
 class AcceptNotificationRequestService < BaseService
   def call(request)
     NotificationPermission.create!(account: request.account, from_account: request.from_account)
-    UnfilterNotificationsWorker.perform_async(request.id)
+    UnfilterNotificationsWorker.perform_async(request.account_id, request.from_account_id)
+    request.destroy!
   end
 end
diff --git a/app/workers/unfilter_notifications_worker.rb b/app/workers/unfilter_notifications_worker.rb
index 223654aa165..5939a691fb6 100644
--- a/app/workers/unfilter_notifications_worker.rb
+++ b/app/workers/unfilter_notifications_worker.rb
@@ -3,8 +3,19 @@
 class UnfilterNotificationsWorker
   include Sidekiq::Worker
 
-  def perform(notification_request_id)
-    @notification_request = NotificationRequest.find(notification_request_id)
+  # Earlier versions of the feature passed a `notification_request` ID
+  # If `to_account_id` is passed, the first argument is an account ID
+  # TODO for after 4.3.0: drop the single-argument case
+  def perform(notification_request_or_account_id, from_account_id = nil)
+    if from_account_id.present?
+      @notification_request = nil
+      @from_account = Account.find(from_account_id)
+      @recipient    = Account.find(notification_request_or_account_id)
+    else
+      @notification_request = NotificationRequest.find(notification_request_or_account_id)
+      @from_account = @notification_request.from_account
+      @recipient    = @notification_request.account
+    end
 
     push_to_conversations!
     unfilter_notifications!
@@ -16,7 +27,7 @@ class UnfilterNotificationsWorker
   private
 
   def push_to_conversations!
-    notifications_with_private_mentions.find_each { |notification| AccountConversation.add_status(@notification_request.account, notification.target_status) }
+    notifications_with_private_mentions.reorder(nil).find_each(order: :desc) { |notification| AccountConversation.add_status(@recipient, notification.target_status) }
   end
 
   def unfilter_notifications!
@@ -24,11 +35,11 @@ class UnfilterNotificationsWorker
   end
 
   def remove_request!
-    @notification_request.destroy!
+    @notification_request&.destroy!
   end
 
   def filtered_notifications
-    Notification.where(account: @notification_request.account, from_account: @notification_request.from_account, filtered: true)
+    Notification.where(account: @recipient, from_account: @from_account, filtered: true)
   end
 
   def notifications_with_private_mentions
diff --git a/spec/services/accept_notification_request_service_spec.rb b/spec/services/accept_notification_request_service_spec.rb
new file mode 100644
index 00000000000..bf67a52225f
--- /dev/null
+++ b/spec/services/accept_notification_request_service_spec.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AcceptNotificationRequestService do
+  subject { described_class.new }
+
+  let(:notification_request) { Fabricate(:notification_request) }
+
+  describe '#call' do
+    it 'destroys the notification request, creates a permission, and queues a worker' do
+      expect { subject.call(notification_request) }
+        .to change { NotificationRequest.exists?(notification_request.id) }.to(false)
+        .and change { NotificationPermission.exists?(account_id: notification_request.account_id, from_account_id: notification_request.from_account_id) }.to(true)
+
+      expect(UnfilterNotificationsWorker).to have_enqueued_sidekiq_job(notification_request.account_id, notification_request.from_account_id)
+    end
+  end
+end
diff --git a/spec/workers/unfilter_notifications_worker_spec.rb b/spec/workers/unfilter_notifications_worker_spec.rb
new file mode 100644
index 00000000000..3f43b298a50
--- /dev/null
+++ b/spec/workers/unfilter_notifications_worker_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe UnfilterNotificationsWorker do
+  let(:recipient) { Fabricate(:account) }
+  let(:sender) { Fabricate(:account) }
+
+  before do
+    # Populate multiple kinds of filtered notifications
+    private_message = Fabricate(:status, account: sender, visibility: :direct)
+    mention = Fabricate(:mention, account: recipient, status: private_message)
+    Fabricate(:notification, filtered: true, from_account: sender, account: recipient, type: :mention, activity: mention)
+    follow_request = sender.request_follow!(recipient)
+    Fabricate(:notification, filtered: true, from_account: sender, account: recipient, type: :follow_request, activity: follow_request)
+  end
+
+  shared_examples 'shared behavior' do
+    it 'unfilters notifications and adds private messages to conversations' do
+      expect { subject }
+        .to change { recipient.notifications.where(from_account_id: sender.id).pluck(:filtered) }.from([true, true]).to([false, false])
+        .and change { recipient.conversations.exists?(last_status_id: sender.statuses.first.id) }.to(true)
+    end
+  end
+
+  describe '#perform' do
+    context 'with single argument (prerelease behavior)' do
+      subject { described_class.new.perform(notification_request.id) }
+
+      let(:notification_request) { Fabricate(:notification_request, from_account: sender, account: recipient) }
+
+      it_behaves_like 'shared behavior'
+
+      it 'destroys the notification request' do
+        expect { subject }
+          .to change { NotificationRequest.exists?(notification_request.id) }.to(false)
+      end
+    end
+
+    context 'with two arguments' do
+      subject { described_class.new.perform(recipient.id, sender.id) }
+
+      it_behaves_like 'shared behavior'
+    end
+  end
+end