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

pull/2813/head
Claire 2024-08-14 09:34:30 +02:00 committed by GitHub
parent 3e3450be36
commit 7996a9543d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 83 additions and 6 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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