about summary refs log tree commit diff
diff options
context:
space:
mode:
authorClaire <claire.github-309c@sitedethib.com>2022-05-16 18:26:49 +0200
committerGitHub <noreply@github.com>2022-05-16 18:26:49 +0200
commitb91196f4b73fff91997b8077619ae25b6d04a59e (patch)
tree9d52a2fbed1170b5180540878ff4fa9292ad3ff1
parent3a084113067656ef9318b9fb5bcfea4fd2de6ffe (diff)
Add confirmation page when importing blocked domains (#1773)
* Move glitch-soc-specific strings to glitch-soc-specific locale files

* Add confirmation page when importing blocked domains
-rw-r--r--app/controllers/admin/domain_blocks_controller.rb21
-rw-r--r--app/controllers/admin/export_domain_blocks_controller.rb49
-rw-r--r--app/javascript/core/admin.js6
-rw-r--r--app/models/form/domain_block_batch.rb35
-rw-r--r--app/views/admin/export_domain_blocks/_domain_block.html.haml27
-rw-r--r--app/views/admin/export_domain_blocks/import.html.haml21
-rw-r--r--config/locales-glitch/en.yml20
-rw-r--r--config/locales/en.yml12
-rw-r--r--config/routes.rb7
-rw-r--r--spec/controllers/admin/domain_blocks_controller_spec.rb21
-rw-r--r--spec/controllers/admin/export_domain_blocks_controller_spec.rb16
11 files changed, 185 insertions, 50 deletions
diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb
index 16defc1ea..48e9781d6 100644
--- a/app/controllers/admin/domain_blocks_controller.rb
+++ b/app/controllers/admin/domain_blocks_controller.rb
@@ -4,6 +4,17 @@ module Admin
   class DomainBlocksController < BaseController
     before_action :set_domain_block, only: [:show, :destroy, :edit, :update]
 
+    def batch
+      @form = Form::DomainBlockBatch.new(form_domain_block_batch_params.merge(current_account: current_account, action: action_from_button))
+      @form.save
+    rescue ActionController::ParameterMissing
+      flash[:alert] = I18n.t('admin.email_domain_blocks.no_domain_block_selected')
+    rescue Mastodon::NotPermittedError
+      flash[:alert] = I18n.t('admin.domain_blocks.created_msg')
+    else
+      redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg')
+    end
+
     def new
       authorize :domain_block, :create?
       @domain_block = DomainBlock.new(domain: params[:_domain])
@@ -76,5 +87,15 @@ module Admin
     def resource_params
       params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate)
     end
+
+    def form_domain_block_batch_params
+      params.require(:form_domain_block_batch).permit(domain_blocks_attributes: [:enabled, :domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate])
+    end
+
+    def action_from_button
+      if params[:save]
+        'save'
+      end
+    end
   end
 end
diff --git a/app/controllers/admin/export_domain_blocks_controller.rb b/app/controllers/admin/export_domain_blocks_controller.rb
index 0ad5b92b5..db8863551 100644
--- a/app/controllers/admin/export_domain_blocks_controller.rb
+++ b/app/controllers/admin/export_domain_blocks_controller.rb
@@ -21,30 +21,33 @@ module Admin
 
     def import
       authorize :domain_block, :create?
-      begin
-        @import = Admin::Import.new(import_params)
-        parse_import_data!(export_headers)
-
-        @data.take(ROWS_PROCESSING_LIMIT).each 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,
-                                         public_comment: row['#public_comment'].strip,
-                                         obfuscate: row['#obfuscate'].strip)
-          if domain_block.save
-            DomainBlockWorker.perform_async(domain_block.id)
-            log_action :create, domain_block
-          end
-        end
-        flash[:notice] = I18n.t('admin.domain_blocks.created_msg')
-      rescue ActionController::ParameterMissing
-        flash[:error] = I18n.t('admin.export_domain_blocks.no_file')
+
+      @import = Admin::Import.new(import_params)
+      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(ROWS_PROCESSING_LIMIT).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,
+                                       private_comment: @global_private_comment,
+                                       public_comment: row['#public_comment']&.strip,
+                                       obfuscate: row['#obfuscate'].strip)
+
+        domain_block if domain_block.valid?
       end
-      redirect_to admin_instances_path(limited: '1')
+
+      @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)
+    rescue ActionController::ParameterMissing
+      flash.now[:alert] = I18n.t('admin.export_domain_blocks.no_file')
+      set_dummy_import!
+      render :new
     end
 
     private
diff --git a/app/javascript/core/admin.js b/app/javascript/core/admin.js
index ef0a8f267..c1b9f07a4 100644
--- a/app/javascript/core/admin.js
+++ b/app/javascript/core/admin.js
@@ -102,6 +102,12 @@ ready(() => {
   const registrationMode = document.getElementById('form_admin_settings_registrations_mode');
   if (registrationMode) onChangeRegistrationMode(registrationMode);
 
+  const checkAllElement = document.querySelector('#batch_checkbox_all');
+  if (checkAllElement) {
+    checkAllElement.checked = [].every.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked);
+    checkAllElement.indeterminate = !checkAllElement.checked && [].some.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked);
+  }
+
   document.querySelector('a#add-instance-button')?.addEventListener('click', (e) => {
     const domain = document.getElementById('by_domain')?.value;
 
diff --git a/app/models/form/domain_block_batch.rb b/app/models/form/domain_block_batch.rb
new file mode 100644
index 000000000..39012df51
--- /dev/null
+++ b/app/models/form/domain_block_batch.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class Form::DomainBlockBatch
+  include ActiveModel::Model
+  include Authorization
+  include AccountableConcern
+
+  attr_accessor :domain_blocks_attributes, :action, :current_account
+
+  def save
+    case action
+    when 'save'
+      save!
+    end
+  end
+
+  private
+
+  def domain_blocks
+    @domain_blocks ||= domain_blocks_attributes.values.filter_map do |attributes|
+      DomainBlock.new(attributes.without('enabled')) if ActiveModel::Type::Boolean.new.cast(attributes['enabled'])
+    end
+  end
+
+  def save!
+    domain_blocks.each do |domain_block|
+      authorize(domain_block, :create?)
+      next if DomainBlock.rule_for(domain_block.domain).present?
+
+      domain_block.save!
+      DomainBlockWorker.perform_async(domain_block.id)
+      log_action :create, domain_block
+    end
+  end
+end
diff --git a/app/views/admin/export_domain_blocks/_domain_block.html.haml b/app/views/admin/export_domain_blocks/_domain_block.html.haml
new file mode 100644
index 000000000..5d4b6c4d0
--- /dev/null
+++ b/app/views/admin/export_domain_blocks/_domain_block.html.haml
@@ -0,0 +1,27 @@
+- existing_relationships ||= false
+
+.batch-table__row{ class: [existing_relationships && 'batch-table__row--attention'] }
+  %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
+    = f.check_box :enabled, checked: !existing_relationships
+  .batch-table__row__content.pending-account
+    .pending-account__header
+      %strong
+        = f.object.domain
+      = f.hidden_field :domain
+      = f.hidden_field :severity
+      = f.hidden_field :reject_media
+      = f.hidden_field :reject_reports
+      = f.hidden_field :obfuscate
+      = f.hidden_field :private_comment
+      = f.hidden_field :public_comment
+
+      %br/
+
+      = f.object.policies.map { |policy| t(policy, scope: 'admin.instances.content_policies.policies') }.join(' • ')
+      - if f.object.public_comment.present?
+        •
+        = f.object.public_comment
+      - if existing_relationships
+        •
+        = fa_icon 'warning fw'
+        = t('admin.export_domain_blocks.import.existing_relationships_warning')
diff --git a/app/views/admin/export_domain_blocks/import.html.haml b/app/views/admin/export_domain_blocks/import.html.haml
new file mode 100644
index 000000000..01add232d
--- /dev/null
+++ b/app/views/admin/export_domain_blocks/import.html.haml
@@ -0,0 +1,21 @@
+- content_for :page_title do
+  = t('admin.export_domain_blocks.import.title')
+
+%p= t('admin.export_domain_blocks.import.description_html')
+
+- if defined?(@global_private_comment) && @global_private_comment.present?
+  %p= t('admin.export_domain_blocks.import.private_comment_description_html', comment: @global_private_comment)
+
+= form_for(@form, url: batch_admin_domain_blocks_path) do |f|
+  .batch-table
+    .batch-table__toolbar
+      %label.batch-table__toolbar__select.batch-checkbox-all
+        = check_box_tag :batch_checkbox_all, nil, false
+      .batch-table__toolbar__actions
+        = f.button safe_join([fa_icon('copy'), t('admin.domain_blocks.import')]), name: :save, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+    .batch-table__body
+      - if @domain_blocks.empty?
+        = nothing_here 'nothing-here--under-tabs'
+      - else
+        = f.simple_fields_for :domain_blocks, @domain_blocks do |ff|
+          = render 'domain_block', f: ff, existing_relationships: @warning_domains.include?(ff.object.domain)
diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml
index 3b554f4a2..78933f54e 100644
--- a/config/locales-glitch/en.yml
+++ b/config/locales-glitch/en.yml
@@ -4,6 +4,26 @@ en:
     custom_emojis:
       batch_copy_error: 'An error occurred when copying some of the selected emoji: %{message}'
       batch_error: 'An error occurred: %{message}'
+    domain_allows:
+      export: Export
+      import: Import
+    domain_blocks:
+      export: Export
+      import: Import
+    export_domain_allows:
+      new:
+        title: Import domain allows
+      no_file: No file selected
+    export_domain_blocks:
+      import:
+        description_html: You are about to import a list of domain blocks. Please review this list very carefully, especially if you have not authored this list yourself.
+        existing_relationships_warning: Existing follow relationships
+        private_comment_description_html: 'To help you track where imported blocks come from, imported blocks will be created with the following private comment: <q>%{comment}</q>'
+        private_comment_template: Imported from %{source} on %{date}
+        title: Import domain blocks
+      new:
+        title: Import domain blocks
+      no_file: No file selected
     settings:
       captcha_enabled:
         desc_html: This relies on external scripts from hCaptcha, which may be a security and privacy concern. In addition, <strong>this can make the registration process significantly less accessible to some (especially disabled) people</strong>. For these reasons, please consider alternative measures such as approval-based or invite-based registration.<br>Users that have been invited through a limited-use invite will not need to solve a CAPTCHA
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4b39d549b..b90402cdd 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -421,8 +421,6 @@ en:
       add_new: Allow federation with domain
       created_msg: Domain has been successfully allowed for federation
       destroyed_msg: Domain has been disallowed from federation
-      export: Export
-      import: Import
       undo: Disallow federation with domain
     domain_blocks:
       add_new: Add new domain block
@@ -431,8 +429,6 @@ en:
       domain: Domain
       edit: Edit domain block
       existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to <a href="%{unblock_url}">unblock it</a> first.
-      export: Export
-      import: Import
       new:
         create: Create block
         hint: The domain block will not prevent creation of account entries in the database, but will retroactively and automatically apply specific moderation methods on those accounts.
@@ -473,14 +469,6 @@ en:
       resolved_dns_records_hint_html: The domain name resolves to the following MX domains, which are ultimately responsible for accepting e-mail. Blocking an MX domain will block sign-ups from any e-mail address which uses the same MX domain, even if the visible domain name is different. <strong>Be careful not to block major e-mail providers.</strong>
       resolved_through_html: Resolved through %{domain}
       title: Blocked e-mail domains
-    export_domain_allows:
-      new:
-        title: Import domain allows
-      no_file: No file selected
-    export_domain_blocks:
-      new:
-        title: Import domain blocks
-      no_file: No file selected
     follow_recommendations:
       description_html: "<strong>Follow recommendations help new users quickly find interesting content</strong>. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language."
       language: For language
diff --git a/config/routes.rb b/config/routes.rb
index 787665192..5ab3ec1e1 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -194,7 +194,11 @@ Rails.application.routes.draw do
     get '/dashboard', to: 'dashboard#index'
 
     resources :domain_allows, only: [:new, :create, :show, :destroy]
-    resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit]
+    resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] do
+      collection do
+        post :batch
+      end
+    end
 
     resources :export_domain_allows, only: [:new] do
       collection do
@@ -485,6 +489,7 @@ Rails.application.routes.draw do
       end
 
       resource :domain_blocks, only: [:show, :create, :destroy]
+
       resource :directory, only: [:show]
 
       resources :follow_requests, only: [:index] do
diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb
index ecc79292b..a35b2fb3b 100644
--- a/spec/controllers/admin/domain_blocks_controller_spec.rb
+++ b/spec/controllers/admin/domain_blocks_controller_spec.rb
@@ -16,6 +16,27 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do
     end
   end
 
+  describe 'POST #batch' do
+    it 'blocks the domains when succeeded to save' do
+      allow(DomainBlockWorker).to receive(:perform_async).and_return(true)
+
+      post :batch, params: {
+        save: '',
+        form_domain_block_batch: {
+          domain_blocks_attributes: {
+            '0' => { enabled: '1', domain: 'example.com', severity: 'silence' },
+            '1' => { enabled: '0', domain: 'mastodon.social', severity: 'suspend' },
+            '2' => { enabled: '1', domain: 'mastodon.online', severity: 'suspend' }
+          }
+        }
+      }
+
+      expect(DomainBlockWorker).to have_received(:perform_async).exactly(2).times
+      expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg')
+      expect(response).to redirect_to(admin_instances_path(limited: '1'))
+    end
+  end
+
   describe 'POST #create' do
     it 'blocks the domain when succeeded to save' do
       allow(DomainBlockWorker).to receive(:perform_async).and_return(true)
diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb
index 0cb221972..0493df859 100644
--- a/spec/controllers/admin/export_domain_blocks_controller_spec.rb
+++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb
@@ -22,26 +22,14 @@ RSpec.describe Admin::ExportDomainBlocksController, type: :controller do
 
   describe 'POST #import' do
     it 'blocks imported domains' do
-      allow(DomainBlockWorker).to receive(:perform_async).and_return(true)
-
       post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks.csv') } }
 
-      expect(response).to redirect_to(admin_instances_path(limited: '1'))
-      expect(DomainBlockWorker).to have_received(:perform_async).exactly(3).times
-
-      # Header should not be imported
-      expect(DomainBlock.where(domain: '#domain').present?).to eq(false)
-
-      # Domains should now be added
-      get :export, params: { format: :csv }
-      expect(response).to have_http_status(200)
-      expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_blocks.csv')))
+      expect(assigns(:domain_blocks).map(&:domain)).to match_array ['bad.domain', 'worse.domain', 'reject.media']
     end
   end
 
   it 'displays error on no file selected' do
     post :import, params: { admin_import: {} }
-    expect(response).to redirect_to(admin_instances_path(limited: '1'))
-    expect(flash[:error]).to eq(I18n.t('admin.export_domain_blocks.no_file'))
+    expect(flash[:alert]).to eq(I18n.t('admin.export_domain_blocks.no_file'))
   end
 end