Fix some notifications not being deleted on poll/status deletion (#15402)
* Fix deleting polls not deleting notifications * Fix fav notification deletion when deleting a toot * Refactor DeleteAccountService spec * Add DeleteAccountService tests for other associations and notifications * Add favourite handling spec in status removal Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
ThibG
GitHub
parent
6f51fd7435
commit
43961035a9
4 changed files with 80 additions and 56 deletions
|
|
@ -56,7 +56,7 @@ class Status < ApplicationRecord
|
|||
belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true
|
||||
belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true
|
||||
|
||||
has_many :favourites, inverse_of: :status, dependent: :delete_all
|
||||
has_many :favourites, inverse_of: :status, dependent: :destroy
|
||||
has_many :bookmarks, inverse_of: :status, dependent: :destroy
|
||||
has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy
|
||||
has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread
|
||||
|
|
|
|||
|
|
@ -123,7 +123,9 @@ class DeleteAccountService < BaseService
|
|||
next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id)
|
||||
|
||||
# We can safely delete the poll rather than destroy it, as any non-reported
|
||||
# status should have been deleted already
|
||||
# status should have been deleted already, as long as we take care of
|
||||
# notifications.
|
||||
Notification.where(poll: poll).delete_all
|
||||
poll.delete
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -1,28 +1,31 @@
|
|||
require 'rails_helper'
|
||||
|
||||
RSpec.describe DeleteAccountService, type: :service do
|
||||
describe '#call on local account' do
|
||||
before do
|
||||
stub_request(:post, "https://alice.com/inbox").to_return(status: 201)
|
||||
stub_request(:post, "https://bob.com/inbox").to_return(status: 201)
|
||||
end
|
||||
shared_examples 'common behavior' do
|
||||
let!(:status) { Fabricate(:status, account: account) }
|
||||
let!(:mention) { Fabricate(:mention, account: local_follower) }
|
||||
let!(:status_with_mention) { Fabricate(:status, account: account, mentions: [mention]) }
|
||||
let!(:media_attachment) { Fabricate(:media_attachment, account: account) }
|
||||
let!(:notification) { Fabricate(:notification, account: account) }
|
||||
let!(:favourite) { Fabricate(:favourite, account: account, status: Fabricate(:status, account: local_follower)) }
|
||||
let!(:poll) { Fabricate(:poll, account: account) }
|
||||
let!(:poll_vote) { Fabricate(:poll_vote, account: local_follower, poll: poll) }
|
||||
|
||||
let!(:active_relationship) { Fabricate(:follow, account: account, target_account: local_follower) }
|
||||
let!(:passive_relationship) { Fabricate(:follow, account: local_follower, target_account: account) }
|
||||
let!(:endorsement) { Fabricate(:account_pin, account: local_follower, target_account: account) }
|
||||
|
||||
let!(:mention_notification) { Fabricate(:notification, account: local_follower, activity: mention, type: :mention) }
|
||||
let!(:status_notification) { Fabricate(:notification, account: local_follower, activity: status, type: :status) }
|
||||
let!(:poll_notification) { Fabricate(:notification, account: local_follower, activity: poll, type: :poll) }
|
||||
let!(:favourite_notification) { Fabricate(:notification, account: local_follower, activity: favourite, type: :favourite) }
|
||||
let!(:follow_notification) { Fabricate(:notification, account: local_follower, activity: active_relationship, type: :follow) }
|
||||
|
||||
subject do
|
||||
-> { described_class.new.call(account) }
|
||||
end
|
||||
|
||||
let!(:account) { Fabricate(:account) }
|
||||
let!(:status) { Fabricate(:status, account: account) }
|
||||
let!(:media_attachment) { Fabricate(:media_attachment, account: account) }
|
||||
let!(:notification) { Fabricate(:notification, account: account) }
|
||||
let!(:favourite) { Fabricate(:favourite, account: account) }
|
||||
let!(:active_relationship) { Fabricate(:follow, account: account) }
|
||||
let!(:passive_relationship) { Fabricate(:follow, target_account: account) }
|
||||
let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) }
|
||||
let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) }
|
||||
let!(:endorsment) { Fabricate(:account_pin, account: passive_relationship.account, target_account: account) }
|
||||
|
||||
it 'deletes associated records' do
|
||||
it 'deletes associated owned records' do
|
||||
is_expected.to change {
|
||||
[
|
||||
account.statuses,
|
||||
|
|
@ -31,15 +34,46 @@ RSpec.describe DeleteAccountService, type: :service do
|
|||
account.favourites,
|
||||
account.active_relationships,
|
||||
account.passive_relationships,
|
||||
AccountPin.where(target_account: account),
|
||||
account.polls,
|
||||
].map(&:count)
|
||||
}.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0])
|
||||
}.from([2, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0])
|
||||
end
|
||||
|
||||
it 'sends a delete actor activity to all known inboxes' do
|
||||
subject.call
|
||||
expect(a_request(:post, "https://alice.com/inbox")).to have_been_made.once
|
||||
expect(a_request(:post, "https://bob.com/inbox")).to have_been_made.once
|
||||
it 'deletes associated target records' do
|
||||
is_expected.to change {
|
||||
[
|
||||
AccountPin.where(target_account: account),
|
||||
].map(&:count)
|
||||
}.from([1]).to([0])
|
||||
end
|
||||
|
||||
it 'deletes associated target notifications' do
|
||||
is_expected.to change {
|
||||
[
|
||||
'poll', 'favourite', 'status', 'mention', 'follow'
|
||||
].map { |type| Notification.where(type: type).count }
|
||||
}.from([1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0])
|
||||
end
|
||||
end
|
||||
|
||||
describe '#call on local account' do
|
||||
before do
|
||||
stub_request(:post, "https://alice.com/inbox").to_return(status: 201)
|
||||
stub_request(:post, "https://bob.com/inbox").to_return(status: 201)
|
||||
end
|
||||
|
||||
let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) }
|
||||
let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) }
|
||||
|
||||
include_examples 'common behavior' do
|
||||
let!(:account) { Fabricate(:account) }
|
||||
let!(:local_follower) { Fabricate(:account) }
|
||||
|
||||
it 'sends a delete actor activity to all known inboxes' do
|
||||
subject.call
|
||||
expect(a_request(:post, "https://alice.com/inbox")).to have_been_made.once
|
||||
expect(a_request(:post, "https://bob.com/inbox")).to have_been_made.once
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -49,36 +83,14 @@ RSpec.describe DeleteAccountService, type: :service do
|
|||
stub_request(:post, "https://bob.com/inbox").to_return(status: 201)
|
||||
end
|
||||
|
||||
subject do
|
||||
-> { described_class.new.call(remote_bob) }
|
||||
end
|
||||
include_examples 'common behavior' do
|
||||
let!(:account) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) }
|
||||
let!(:local_follower) { Fabricate(:account) }
|
||||
|
||||
let!(:account) { Fabricate(:account) }
|
||||
let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) }
|
||||
let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) }
|
||||
let!(:status) { Fabricate(:status, account: remote_bob) }
|
||||
let!(:media_attachment) { Fabricate(:media_attachment, account: remote_bob) }
|
||||
let!(:notification) { Fabricate(:notification, account: remote_bob) }
|
||||
let!(:favourite) { Fabricate(:favourite, account: remote_bob) }
|
||||
let!(:active_relationship) { Fabricate(:follow, account: remote_bob, target_account: account) }
|
||||
let!(:passive_relationship) { Fabricate(:follow, target_account: remote_bob) }
|
||||
|
||||
it 'deletes associated records' do
|
||||
is_expected.to change {
|
||||
[
|
||||
remote_bob.statuses,
|
||||
remote_bob.media_attachments,
|
||||
remote_bob.notifications,
|
||||
remote_bob.favourites,
|
||||
remote_bob.active_relationships,
|
||||
remote_bob.passive_relationships,
|
||||
].map(&:count)
|
||||
}.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0])
|
||||
end
|
||||
|
||||
it 'sends a reject follow to follwer inboxes' do
|
||||
subject.call
|
||||
expect(a_request(:post, remote_bob.inbox_url)).to have_been_made.once
|
||||
it 'sends a reject follow to follwer inboxes' do
|
||||
subject.call
|
||||
expect(a_request(:post, account.inbox_url)).to have_been_made.once
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ require 'rails_helper'
|
|||
RSpec.describe RemoveStatusService, type: :service do
|
||||
subject { RemoveStatusService.new }
|
||||
|
||||
let!(:alice) { Fabricate(:account) }
|
||||
let!(:alice) { Fabricate(:account, user: Fabricate(:user)) }
|
||||
let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://example.com/salmon') }
|
||||
let!(:jeff) { Fabricate(:account) }
|
||||
let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') }
|
||||
|
|
@ -17,23 +17,33 @@ RSpec.describe RemoveStatusService, type: :service do
|
|||
hank.follow!(alice)
|
||||
|
||||
@status = PostStatusService.new.call(alice, text: 'Hello @bob@example.com')
|
||||
FavouriteService.new.call(jeff, @status)
|
||||
Fabricate(:status, account: bill, reblog: @status, uri: 'hoge')
|
||||
subject.call(@status)
|
||||
end
|
||||
|
||||
it 'removes status from author\'s home feed' do
|
||||
subject.call(@status)
|
||||
expect(HomeFeed.new(alice).get(10)).to_not include(@status.id)
|
||||
end
|
||||
|
||||
it 'removes status from local follower\'s home feed' do
|
||||
subject.call(@status)
|
||||
expect(HomeFeed.new(jeff).get(10)).to_not include(@status.id)
|
||||
end
|
||||
|
||||
it 'sends delete activity to followers' do
|
||||
subject.call(@status)
|
||||
expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.twice
|
||||
end
|
||||
|
||||
it 'sends delete activity to rebloggers' do
|
||||
subject.call(@status)
|
||||
expect(a_request(:post, 'http://example2.com/inbox')).to have_been_made
|
||||
end
|
||||
|
||||
it 'remove status from notifications' do
|
||||
expect { subject.call(@status) }.to change {
|
||||
Notification.where(activity_type: 'Favourite', from_account: jeff, account: alice).count
|
||||
}.from(1).to(0)
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in a new issue