about summary refs log tree commit diff
diff options
context:
space:
mode:
authorClaire <claire.github-309c@sitedethib.com>2020-12-21 18:28:23 +0100
committerClaire <claire.github-309c@sitedethib.com>2020-12-21 18:28:23 +0100
commit38bfaf885543f349e88bb18edbc3536cefc0701c (patch)
tree8cd8f0b68c26bb46e14b20484b828df1159363a5
parent61d5744dcc33d4cfdd26a38af138a4a416a93868 (diff)
parent43961035a906cd8bccdf4c1ac023980b37823bb3 (diff)
Merge branch 'master' into glitch-soc/merge-upstream
-rw-r--r--app/models/poll.rb2
-rw-r--r--app/models/user.rb2
-rw-r--r--app/services/batched_remove_status_service.rb3
-rw-r--r--app/services/delete_account_service.rb6
-rw-r--r--app/workers/account_deletion_worker.rb2
-rw-r--r--config/boot.rb5
-rw-r--r--spec/services/delete_account_service_spec.rb108
-rw-r--r--spec/services/remove_status_service_spec.rb14
8 files changed, 85 insertions, 57 deletions
diff --git a/app/models/poll.rb b/app/models/poll.rb
index b5deafcc2..e1ca55252 100644
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -25,7 +25,7 @@ class Poll < ApplicationRecord
   belongs_to :account
   belongs_to :status
 
-  has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :destroy
+  has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :delete_all
 
   has_many :notifications, as: :activity, dependent: :destroy
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 6495ae04c..8cc0a21ff 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -83,7 +83,7 @@ class User < ApplicationRecord
 
   has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy
   accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text }
-  validates :invite_request, presence: true, on: :create, if: -> { Setting.require_invite_text }
+  validates :invite_request, presence: true, on: :create, if: -> { Setting.require_invite_text && !invited? }
 
   validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale?
   validates_with BlacklistedEmailValidator, on: :create
diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb
index 707672ee0..e083234ae 100644
--- a/app/services/batched_remove_status_service.rb
+++ b/app/services/batched_remove_status_service.rb
@@ -4,8 +4,6 @@ class BatchedRemoveStatusService < BaseService
   include Redisable
 
   # Delete given statuses and reblogs of them
-  # Dispatch PuSH updates of the deleted statuses, but only local ones
-  # Dispatch Salmon deletes, unique per domain, of the deleted statuses, but only local ones
   # Remove statuses from home feeds
   # Push delete events to streaming API for home feeds and public feeds
   # @param [Enumerable<Status>] statuses A preferably batched array of statuses
@@ -19,7 +17,6 @@ class BatchedRemoveStatusService < BaseService
 
     @json_payloads = statuses.each_with_object({}) { |s, h| h[s.id] = Oj.dump(event: :delete, payload: s.id.to_s) }
 
-    # Ensure that rendered XML reflects destroyed state
     statuses.each do |status|
       status.mark_for_mass_destruction!
       status.destroy
diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb
index 9cb80c95a..fa834e775 100644
--- a/app/services/delete_account_service.rb
+++ b/app/services/delete_account_service.rb
@@ -122,7 +122,11 @@ class DeleteAccountService < BaseService
     @account.polls.reorder(nil).find_each do |poll|
       next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id)
 
-      poll.destroy
+      # We can safely delete the poll rather than destroy it, as any non-reported
+      # status should have been deleted already, as long as we take care of
+      # notifications.
+      Notification.where(poll: poll).delete_all
+      poll.delete
     end
 
     associations_for_destruction.each do |association_name|
diff --git a/app/workers/account_deletion_worker.rb b/app/workers/account_deletion_worker.rb
index 98b67419d..fdf013e01 100644
--- a/app/workers/account_deletion_worker.rb
+++ b/app/workers/account_deletion_worker.rb
@@ -3,7 +3,7 @@
 class AccountDeletionWorker
   include Sidekiq::Worker
 
-  sidekiq_options queue: 'pull'
+  sidekiq_options queue: 'pull', lock: :until_executed
 
   def perform(account_id, options = {})
     reserve_username = options.with_indifferent_access.fetch(:reserve_username, true)
diff --git a/config/boot.rb b/config/boot.rb
index f3e36203a..6cde5319d 100644
--- a/config/boot.rb
+++ b/config/boot.rb
@@ -1,3 +1,8 @@
+unless ENV.key?('RAILS_ENV')
+  STDERR.puts 'ERROR: Missing RAILS_ENV environment variable, please set it to "production", "development", or "test".'
+  exit 1
+end
+
 ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
 
 require 'bundler/setup' # Set up gems listed in the Gemfile.
diff --git a/spec/services/delete_account_service_spec.rb b/spec/services/delete_account_service_spec.rb
index d208b25b8..cd7d32d59 100644
--- a/spec/services/delete_account_service_spec.rb
+++ b/spec/services/delete_account_service_spec.rb
@@ -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,54 +34,63 @@ RSpec.describe DeleteAccountService, type: :service do
           account.favourites,
           account.active_relationships,
           account.passive_relationships,
+          account.polls,
+        ].map(&:count)
+      }.from([2, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0])
+    end
+
+    it 'deletes associated target records' do
+      is_expected.to change {
+        [
           AccountPin.where(target_account: account),
         ].map(&:count)
-      }.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0])
+      }.from([1]).to([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 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 remote account' 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
 
-    subject do
-      -> { described_class.new.call(remote_bob) }
-    end
-
-    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])
+    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
+
+  describe '#call on remote 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
 
-    it 'sends a reject follow to follwer inboxes' do
-      subject.call
-      expect(a_request(:post, remote_bob.inbox_url)).to have_been_made.once
+    include_examples 'common behavior' do
+      let!(:account) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) }
+      let!(:local_follower) { Fabricate(:account) }
+
+      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
diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb
index 06676ec45..7ce75b2c7 100644
--- a/spec/services/remove_status_service_spec.rb
+++ b/spec/services/remove_status_service_spec.rb
@@ -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