Fix follow limit preventing re-following of a moved account (#14207)

diff --git a/app/models/follow.rb b/app/models/follow.rb
index 55a9da7928b70cc450fc8ff30c15afbffa29c2dc..69a1722b303ac3f22dabcbc03ca67d45cbcf83da 100644 (file)
--- a/app/models/follow.rb
+++ b/app/models/follow.rb
@@ -26,7 +26,7 @@ class Follow < ApplicationRecord
   has_one :notification, as: :activity, dependent: :destroy
 
   validates :account_id, uniqueness: { scope: :target_account_id }
-  validates_with FollowLimitValidator, on: :create
+  validates_with FollowLimitValidator, on: :create, if: :rate_limit?
 
   scope :recent, -> { reorder(id: :desc) }
 

diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb
index c1f19149b1f23af1b78bb98991318085402ab5d6..2d2a77b59c6efa9d95a89a44491833dc8ccb6d5a 100644 (file)
--- a/app/models/follow_request.rb
+++ b/app/models/follow_request.rb
@@ -26,7 +26,7 @@ class FollowRequest < ApplicationRecord
   has_one :notification, as: :activity, dependent: :destroy
 
   validates :account_id, uniqueness: { scope: :target_account_id }
-  validates_with FollowLimitValidator, on: :create
+  validates_with FollowLimitValidator, on: :create, if: :rate_limit?
 
   def authorize!
     account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri)

diff --git a/app/models/import.rb b/app/models/import.rb
index 70245328959de01d14991368aac5350eb58c6098..00a54892ef2d4314e4f766ec1703d0431217d5c4 100644 (file)
--- a/app/models/import.rb
+++ b/app/models/import.rb
@@ -27,6 +27,7 @@ class Import < ApplicationRecord
   enum type: [:following, :blocking, :muting, :domain_blocking, :bookmarks]
 
   validates :type, presence: true
+  validates_with ImportValidator, on: :create
 
   has_attached_file :data
   validates_attachment_content_type :data, content_type: FILE_TYPES

diff --git a/app/services/import_service.rb b/app/services/import_service.rb
index 288e47f1ea12dec22d54846088315c76f26e30fb..0c6ef2238e94afc430ee7bc1821b07e5e0cc7317 100644 (file)
--- a/app/services/import_service.rb
+++ b/app/services/import_service.rb
@@ -27,7 +27,7 @@ class ImportService < BaseService
 
   def import_follows!
     parse_import_data!(['Account address'])
-    import_relationships!('follow', 'unfollow', @account.following, follow_limit, reblogs: { header: 'Show boosts', default: true })
+    import_relationships!('follow', 'unfollow', @account.following, ROWS_PROCESSING_LIMIT, reblogs: { header: 'Show boosts', default: true })
   end
 
   def import_blocks!
@@ -85,6 +85,7 @@ class ImportService < BaseService
 
     head_items = items.uniq { |acct, _| acct.split('@')[1] }
     tail_items = items - head_items
+
     Import::RelationshipWorker.push_bulk(head_items + tail_items) do |acct, extra|
       [@account.id, acct, action, extra]
     end
@@ -133,10 +134,6 @@ class ImportService < BaseService
     Paperclip.io_adapters.for(@import.data).read
   end
 
-  def follow_limit
-    FollowLimitValidator.limit_for_account(@account)
-  end
-
   def relations_map_for_account(account, account_ids)
     {
       blocking: {},

diff --git a/app/validators/import_validator.rb b/app/validators/import_validator.rb
new file mode 100644 (file)
index 0000000..a182abf
--- /dev/null
+++ b/app/validators/import_validator.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+class ImportValidator < ActiveModel::Validator
+  KNOWN_HEADERS = [
+    'Account address',
+    '#domain',
+    '#uri',
+  ].freeze
+
+  def validate(import)
+    return if import.type.blank? || import.data.blank?
+
+    # We parse because newlines could be part of individual rows. This
+    # runs on create so we should be reading the local file here before
+    # it is uploaded to object storage or moved anywhere...
+    csv_data = CSV.parse(import.data.queued_for_write[:original].read)
+
+    row_count  = csv_data.size
+    row_count -= 1 if KNOWN_HEADERS.include?(csv_data.first&.first)
+
+    import.errors.add(:data, I18n.t('imports.errors.over_rows_processing_limit', count: ImportService::ROWS_PROCESSING_LIMIT)) if row_count > ImportService::ROWS_PROCESSING_LIMIT
+
+    case import.type
+    when 'following'
+      validate_following_import(import, row_count)
+    end
+  end
+
+  private
+
+  def validate_following_import(import, row_count)
+    base_limit = FollowLimitValidator.limit_for_account(import.account)
+
+    limit = begin
+      if import.overwrite?
+        base_limit
+      else
+        base_limit - import.account.following_count
+      end
+    end
+
+    import.errors.add(:data, I18n.t('users.follow_limit_reached', limit: base_limit)) if row_count > limit
+  end
+end

diff --git a/config/locales/en.yml b/config/locales/en.yml
index e91f4344f6591df48d606115a362137304a0f841..18a207f5e4bfd993fd12f133e7e8b974a32902de 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -923,6 +923,8 @@ en:
     status: Verification status
     view_proof: View proof
   imports:
+    errors:
+      over_rows_processing_limit: contains more than %{count} rows
     modes:
       merge: Merge
       merge_long: Keep existing records and add new ones

diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb
index 0c84e5e7bfa2458455859733d850cb746e1c3bbd..e723a1ef21143f222e52f094c0d9a9deacd197d0 100644 (file)
--- a/spec/models/follow_spec.rb
+++ b/spec/models/follow_spec.rb
@@ -5,7 +5,7 @@ RSpec.describe Follow, type: :model do
   let(:bob)   { Fabricate(:account, username: 'bob') }
 
   describe 'validations' do
-    subject { Follow.new(account: alice, target_account: bob) }
+    subject { Follow.new(account: alice, target_account: bob, rate_limit: true) }
 
     it 'has a valid fabricator' do
       follow = Fabricate.build(:follow)

diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb
index 321761166ec50a34e59dda7d80ed9f1618ca1994..a5eec172264a4e09c9b403bde8f6bc07507dbd36 100644 (file)
--- a/spec/models/import_spec.rb
+++ b/spec/models/import_spec.rb
@@ -20,5 +20,15 @@ RSpec.describe Import, type: :model do
       import = Import.create(account: account, type: type)
       expect(import).to model_have_error_on_field(:data)
     end
+
+    it 'is invalid with too many rows in data' do
+      import = Import.create(account: account, type: type, data: StringIO.new("foo@bar.com\n" * (ImportService::ROWS_PROCESSING_LIMIT + 10)))
+      expect(import).to model_have_error_on_field(:data)
+    end
+
+    it 'is invalid when there are more rows when following limit' do
+      import = Import.create(account: account, type: type, data: StringIO.new("foo@bar.com\n" * (FollowLimitValidator.limit_for_account(account) + 10)))
+      expect(import).to model_have_error_on_field(:data)
+    end
   end
 end