Change domain block CSV parsing to be more robust and handle more lists (#21470)

* Change domain block CSV parsing to be more robust and handle more lists

* Add some tests

* Improve domain block import validation and reporting

4 files changed, 53 insertions, 28 deletions

diff --git a/app/controllers/admin/export_domain_allows_controller.rb b/app/controllers/admin/export_domain_allows_controller.rb
index 57fb12c62..adfc39da2 100644
--- a/app/controllers/admin/export_domain_allows_controller.rb
+++ b/app/controllers/admin/export_domain_allows_controller.rb
@@ -23,9 +23,7 @@ module Admin
         @import = Admin::Import.new(import_params)
         return render :new unless @import.validate
 
-        parse_import_data!(export_headers)
-
-        @data.take(Admin::Import::ROWS_PROCESSING_LIMIT).each do |row|
+        @import.csv_rows.each do |row|
           domain = row['#domain'].strip
           next if DomainAllow.allowed?(domain)
 
diff --git a/app/controllers/admin/export_domain_blocks_controller.rb b/app/controllers/admin/export_domain_blocks_controller.rb
index fb0cd05d2..816422d4f 100644
--- a/app/controllers/admin/export_domain_blocks_controller.rb
+++ b/app/controllers/admin/export_domain_blocks_controller.rb
@@ -23,24 +23,30 @@ module Admin
       @import = Admin::Import.new(import_params)
       return render :new unless @import.validate
 
-      parse_import_data!(export_headers)
-
       @global_private_comment = I18n.t('admin.export_domain_blocks.import.private_comment_template', source: @import.data_file_name, date: I18n.l(Time.now.utc))
 
       @form = Form::DomainBlockBatch.new
-      @domain_blocks = @data.take(Admin::Import::ROWS_PROCESSING_LIMIT).filter_map do |row|
+      @domain_blocks = @import.csv_rows.filter_map do |row|
         domain = row['#domain'].strip
         next if DomainBlock.rule_for(domain).present?
 
         domain_block = DomainBlock.new(domain: domain,
-                                       severity: row['#severity'].strip,
-                                       reject_media: row['#reject_media'].strip,
-                                       reject_reports: row['#reject_reports'].strip,
+                                       severity: row.fetch('#severity', :suspend),
+                                       reject_media: row.fetch('#reject_media', false),
+                                       reject_reports: row.fetch('#reject_reports', false),
                                        private_comment: @global_private_comment,
-                                       public_comment: row['#public_comment']&.strip,
-                                       obfuscate: row['#obfuscate'].strip)
+                                       public_comment: row['#public_comment'],
+                                       obfuscate: row.fetch('#obfuscate', false))
+
+        if domain_block.invalid?
+          flash.now[:alert] = I18n.t('admin.export_domain_blocks.invalid_domain_block', error: domain_block.errors.full_messages.join(', '))
+          next
+        end
 
-        domain_block if domain_block.valid?
+        domain_block
+      rescue ArgumentError => e
+        flash.now[:alert] = I18n.t('admin.export_domain_blocks.invalid_domain_block', error: e.message)
+        next
       end
 
       @warning_domains = Instance.where(domain: @domain_blocks.map(&:domain)).where('EXISTS (SELECT 1 FROM follows JOIN accounts ON follows.account_id = accounts.id OR follows.target_account_id = accounts.id WHERE accounts.domain = instances.domain)').pluck(:domain)
diff --git a/app/controllers/concerns/admin_export_controller_concern.rb b/app/controllers/concerns/admin_export_controller_concern.rb
index b40c76557..4ac48a04b 100644
--- a/app/controllers/concerns/admin_export_controller_concern.rb
+++ b/app/controllers/concerns/admin_export_controller_concern.rb
@@ -26,14 +26,4 @@ module AdminExportControllerConcern
   def import_params
     params.require(:admin_import).permit(:data)
   end
-
-  def import_data_path
-    params[:admin_import][:data].path
-  end
-
-  def parse_import_data!(default_headers)
-    data = CSV.read(import_data_path, headers: true, encoding: 'UTF-8')
-    data = CSV.read(import_data_path, headers: default_headers, encoding: 'UTF-8') unless data.headers&.first&.strip&.include?(default_headers[0])
-    @data = data.reject(&:blank?)
-  end
 end
diff --git a/app/models/admin/import.rb b/app/models/admin/import.rb
index 79c0722d5..fecde4878 100644
--- a/app/models/admin/import.rb
+++ b/app/models/admin/import.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'csv'
+
 # A non-activerecord helper class for csv upload
 class Admin::Import
   include ActiveModel::Model
@@ -15,17 +17,46 @@ class Admin::Import
     data.original_filename
   end
 
+  def csv_rows
+    csv_data.rewind
+
+    csv_data.take(ROWS_PROCESSING_LIMIT + 1)
+  end
+
   private
 
-  def validate_data
-    return if data.blank?
+  def csv_data
+    return @csv_data if defined?(@csv_data)
+
+    csv_converter = lambda do |field, field_info|
+      case field_info.header
+      when '#domain', '#public_comment'
+        field&.strip
+      when '#severity'
+        field&.strip&.to_sym
+      when '#reject_media', '#reject_reports', '#obfuscate'
+        ActiveModel::Type::Boolean.new.cast(field)
+      else
+        field
+      end
+    end
+
+    @csv_data = CSV.open(data.path, encoding: 'UTF-8', skip_blanks: true, headers: true, converters: csv_converter)
+    @csv_data.take(1) # Ensure the headers are read
+    @csv_data = CSV.open(data.path, encoding: 'UTF-8', skip_blanks: true, headers: ['#domain'], converters: csv_converter) unless @csv_data.headers&.first == '#domain'
+    @csv_data
+  end
 
-    csv_data = CSV.read(data.path, encoding: 'UTF-8')
+  def csv_row_count
+    return @csv_row_count if defined?(@csv_row_count)
 
-    row_count  = csv_data.size
-    row_count -= 1 if csv_data.first&.first == '#domain'
+    csv_data.rewind
+    @csv_row_count = csv_data.take(ROWS_PROCESSING_LIMIT + 2).count
+  end
 
-    errors.add(:data, I18n.t('imports.errors.over_rows_processing_limit', count: ROWS_PROCESSING_LIMIT)) if row_count > ROWS_PROCESSING_LIMIT
+  def validate_data
+    return if data.nil?
+    errors.add(:data, I18n.t('imports.errors.over_rows_processing_limit', count: ROWS_PROCESSING_LIMIT)) if csv_row_count > ROWS_PROCESSING_LIMIT
   rescue CSV::MalformedCSVError => e
     errors.add(:data, I18n.t('imports.errors.invalid_csv_file', error: e.message))
   end