ability to add domain moderation notes, edit existing domain policies in-place, and process asynchronously

20 files changed, 145 insertions, 46 deletions

diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb
index 47c2daa7a..d0858006a 100644
--- a/app/controllers/admin/domain_blocks_controller.rb
+++ b/app/controllers/admin/domain_blocks_controller.rb
@@ -2,7 +2,7 @@
 
 module Admin
   class DomainBlocksController < BaseController
-    before_action :set_domain_block, only: [:show, :destroy]
+    before_action :set_domain_block, only: [:show, :destroy, :update]
 
     def new
       authorize :domain_block, :create?
@@ -15,23 +15,17 @@ module Admin
       @domain_block = DomainBlock.new(resource_params)
       existing_domain_block = resource_params[:domain].present? ? DomainBlock.find_by(domain: resource_params[:domain]) : nil
 
-      if existing_domain_block.present? && !@domain_block.stricter_than?(existing_domain_block)
-        @domain_block.save
-        flash[:alert] = I18n.t('admin.domain_blocks.existing_domain_block_html', name: existing_domain_block.domain, unblock_url: admin_domain_block_path(existing_domain_block)).html_safe # rubocop:disable Rails/OutputSafety
-        @domain_block.errors[:domain].clear
-        render :new
+      if existing_domain_block.present?
+        @domain_block = existing_domain_block
+        @domain_block.update(resource_params.except(:undo))
+      end
+
+      if @domain_block.save
+        DomainBlockWorker.perform_async(@domain_block.id)
+        log_action :create, @domain_block
+        redirect_to admin_instance_path(id: @domain_block.domain, limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg')
       else
-        if existing_domain_block.present?
-          @domain_block = existing_domain_block
-          @domain_block.update(resource_params)
-        end
-        if @domain_block.save
-          DomainBlockWorker.perform_async(@domain_block.id)
-          log_action :create, @domain_block
-          redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg')
-        else
-          render :new
-        end
+        render :new
       end
     end
 
@@ -41,9 +35,25 @@ module Admin
 
     def destroy
       authorize @domain_block, :destroy?
-      UnblockDomainService.new.call(@domain_block)
+      DomainUnblockWorker.perform_async(@domain_block.id)
       log_action :destroy, @domain_block
-      redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.destroyed_msg')
+      flash[:notice] = I18n.t('admin.domain_blocks.destroyed_msg')
+      redirect_to controller: 'admin/instances', action: 'index', limited: '1'
+    end
+
+    def update
+      return destroy unless resource_params[:undo].to_i.zero?
+      authorize @domain_block, :update?
+      @domain_block.update(resource_params.except(:domain, :undo))
+      changed = @domain_block.changed
+      if @domain_block.save
+        DomainBlockWorker.perform_async(@domain_block.id) if (changed & %w(severity force_sensitive reject_media)).any?
+        log_action :update, @domain_block
+        flash[:notice] = I18n.t('admin.domain_blocks.updated_msg')
+      else
+        flash[:alert] = I18n.t('admin.domain_blocks.update_failed_msg')
+      end
+      redirect_to admin_instance_path(id: @domain_block.domain, limited: '1')
     end
 
     private
@@ -53,7 +63,7 @@ module Admin
     end
 
     def resource_params
-      params.require(:domain_block).permit(:domain, :severity, :force_sensitive, :reject_media, :reject_reports)
+      params.require(:domain_block).permit(:domain, :severity, :force_sensitive, :reject_media, :reject_reports, :reason, :undo)
     end
   end
 end
diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb
index 93ce447a1..427ac0c4e 100644
--- a/app/helpers/admin/action_logs_helper.rb
+++ b/app/helpers/admin/action_logs_helper.rb
@@ -87,7 +87,7 @@ module Admin::ActionLogsHelper
     when 'Report'
       link_to "##{record.id}", admin_report_path(record)
     when 'DomainBlock', 'EmailDomainBlock'
-      link_to record.domain, "https://#{record.domain}"
+      link_to record.domain, admin_instance_path(id: record.domain)
     when 'Status'
       link_to record.account.acct, TagManager.instance.url_for(record)
     when 'AccountWarning'
@@ -100,7 +100,7 @@ module Admin::ActionLogsHelper
     when 'CustomEmoji'
       attributes['shortcode']
     when 'DomainBlock', 'EmailDomainBlock'
-      link_to attributes['domain'], "https://#{attributes['domain']}"
+      link_to attributes['domain'], admin_instance_path(id: attributes['domain'])
     when 'Status'
       tmp_status = Status.new(attributes.except('reblogs_count', 'favourites_count'))
 
diff --git a/app/helpers/bangtag_helper.rb b/app/helpers/bangtag_helper.rb
index 1ace17f85..248f26113 100644
--- a/app/helpers/bangtag_helper.rb
+++ b/app/helpers/bangtag_helper.rb
@@ -4,7 +4,7 @@ module BangtagHelper
   POLICIES = %w(silence unsilence suspend unsuspend force_unlisted allow_public force_sensitive allow_nonsensitive reset)
   EXCLUDED_DOMAINS = %w(tailma.ws monsterpit.net monsterpit.cloud monsterpit.gallery monsterpit.blog)
 
-  def account_policy(username, domain = nil, policy)
+  def account_policy(username, domain, policy, reason = nil)
     return if policy.blank?
     policy = policy.to_s
     return false unless policy.in?(POLICIES)
@@ -50,6 +50,14 @@ module BangtagHelper
 
     acct.save
 
+    return true unless reason && !reason.strip.blank?
+
+    AccountModerationNote.create(
+      account_id: @account.id,
+      target_account_id: acct.id,
+      content: reason.strip
+    )
+
     true
   end
 
@@ -63,7 +71,7 @@ module BangtagHelper
     true
   end
 
-  def domain_policy(domain, policy, force_sensitive = false, reject_media = false, reject_reports = false)
+  def domain_policy(domain, policy, reason = nil, force_sensitive = false, reject_media = false, reject_reports = false)
     return if policy.blank?
     policy = policy.to_s
     return false unless policy.in?(POLICIES)
@@ -86,18 +94,19 @@ module BangtagHelper
       domain_block.force_sensitive = force_sensitive
       domain_block.reject_media = reject_media
       domain_block.reject_reports = reject_reports
+      domain_block.reason = reason.strip if reason && !reason.strip.blank?
       domain_block.save
 
       Admin::ActionLog.create(account: @account, action: :create, target: domain_block)
       user_friendly_action_log(@account, :create, domain_block)
-      BlockDomainService.new.call(domain_block)
+      DomainBlockWorker.perform_async(domain_block.id)
     else
       domain_block = DomainBlock.find_by(domain: domain)
       return false if domain_block.nil?
 
       Admin::ActionLog.create(account: @account, action: :destroy, target: domain_block)
       user_friendly_action_log(@account, :destroy, domain_block)
-      UnblockDomainService.new.call(domain_block)
+      DomainUnblockWorker.perform_async(domain_block.id)
     end
 
     true
diff --git a/app/helpers/log_helper.rb b/app/helpers/log_helper.rb
index f042dc19f..038a4cbbd 100644
--- a/app/helpers/log_helper.rb
+++ b/app/helpers/log_helper.rb
@@ -7,7 +7,7 @@ module LogHelper
     case action
     when :create
       if target.is_a? DomainBlock
-        LogWorker.perform_async("\xf0\x9f\x9a\xab <#{source}> applied a #{target.severity}#{target.force_sensitive? ? " and force sensitive media" : ''}#{target.reject_media? ? " and reject media" : ''} policy on https://#{target.domain}\u200b.", LOG_SCOPE_MODERATION)
+        LogWorker.perform_async("\xf0\x9f\x9a\xab <#{source}> applied a #{target.severity}#{target.force_sensitive? ? " and force sensitive media" : ''}#{target.reject_media? ? " and reject media" : ''} policy on https://#{target.domain}\u200b.\n\n#{target.reason? ? "Comment: #{target.reason}" : ''}", LOG_SCOPE_MODERATION)
       elsif target.is_a? EmailDomainBlock
         LogWorker.perform_async("\u26d4 <#{source}> added a registration block on email domain '#{target.domain}'.", LOG_SCOPE_MODERATION)
       elsif target.is_a? CustomEmoji
@@ -27,7 +27,9 @@ module LogHelper
       end
 
     when :update
-      if target.is_a? Status
+      if target.is_a? DomainBlock
+        LogWorker.perform_async("\xf0\x9f\x9a\xab <#{source}> changed the policy on https://#{target.domain} to #{target.severity}#{target.force_sensitive? ? " and force sensitive media" : ''}#{target.reject_media? ? " and reject media" : ''}.\n\n#{target.reason? ? "Comment: #{target.reason}" : ''}", LOG_SCOPE_MODERATION)
+      elsif target.is_a? Status
         LogWorker.perform_async("\xf0\x9f\x91\x81\xef\xb8\x8f <#{source}> changed visibility flags of post #{TagManager.instance.url_for(target)}\u200b.", LOG_SCOPE_MODERATION)
       elsif target.is_a? CustomEmoji
         LogWorker.perform_async("\xf0\x9f\x94\x81 <#{source}> replaced the '#{target.shortcode}' emoji. :#{target.shortcode}:", LOG_SCOPE_MODERATION)
diff --git a/app/lib/bangtags.rb b/app/lib/bangtags.rb
index acb83221e..0197477c5 100644
--- a/app/lib/bangtags.rb
+++ b/app/lib/bangtags.rb
@@ -744,12 +744,13 @@ class Bangtags
               end
             when 'silence', 'unsilence', 'suspend', 'unsuspend', 'force_unlisted', 'allow_public', 'force_sensitive', 'allow_nonsensitive', 'reset', 'forgive'
               action = 'reset' if action == 'forgive'
+              reason = tf_cmd[2..-1].join(':')
               chunk.split.each do |c|
                 if c.start_with?('@')
                   account_parts = c.split('@')[1..2]
-                  successful = account_policy(account_parts[0], account_parts[1], action)
+                  successful = account_policy(account_parts[0], account_parts[1], action, reason)
                 else
-                  successful = domain_policy(c, action)
+                  successful = domain_policy(c, action, reason)
                 end
                 if successful
                   output << "\u2705 <code>#{c}</code>"
@@ -757,7 +758,12 @@ class Bangtags
                   output << "\u274c <code>#{c}</code>"
                 end
               end
-              output = ['<em>No action.</em>'] if output.blank?
+              if output.blank?
+                output = ['<em>No action.</em>']
+              elsif !reason.blank?
+                output << ''
+                output << "<strong>Comment:</strong> <em>#{reason}</em>"
+              end
               chunk = output.join("\n") + "\n"
             end
           end
diff --git a/app/models/admin/action_log.rb b/app/models/admin/action_log.rb
index 1d1db1b7a..6794299de 100644
--- a/app/models/admin/action_log.rb
+++ b/app/models/admin/action_log.rb
@@ -34,7 +34,7 @@ class Admin::ActionLog < ApplicationRecord
     when :destroy, :create
       self.recorded_changes = target.attributes
     when :update, :promote, :demote
-      self.recorded_changes = target.previous_changes
+      self.recorded_changes = target_type != 'DomainBlock' ? target.previous_changes : target.attributes
     when :change_email
       self.recorded_changes = ActiveSupport::HashWithIndifferentAccess.new(
         email: [target.email, nil],
diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb
index 486e2865b..e4baee5f0 100644
--- a/app/models/domain_block.rb
+++ b/app/models/domain_block.rb
@@ -11,6 +11,7 @@
 #  reject_media    :boolean          default(FALSE), not null
 #  reject_reports  :boolean          default(FALSE), not null
 #  force_sensitive :boolean          default(FALSE), not null
+#  reason          :text
 #
 
 class DomainBlock < ApplicationRecord
@@ -53,4 +54,9 @@ class DomainBlock < ApplicationRecord
     additionals << "reject reports" if reject_reports?
     additionals
   end
+
+  # workaround for the domain policy editor
+  def undo
+    return false
+  end
 end
diff --git a/app/policies/domain_block_policy.rb b/app/policies/domain_block_policy.rb
index 8af666b8f..0ce6baccf 100644
--- a/app/policies/domain_block_policy.rb
+++ b/app/policies/domain_block_policy.rb
@@ -16,4 +16,8 @@ class DomainBlockPolicy < ApplicationPolicy
   def destroy?
     staff?
   end
+
+  def update?
+    staff?
+  end
 end
diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb
index 4a1218e3f..908deacf4 100644
--- a/app/services/block_domain_service.rb
+++ b/app/services/block_domain_service.rb
@@ -5,11 +5,16 @@ class BlockDomainService < BaseService
 
   def call(domain_block)
     @domain_block = domain_block
+    remove_existing_block!
     process_domain_block!
   end
 
   private
 
+  def remove_existing_block!
+    UnblockDomainService.new.call(@domain_block, false)
+  end
+
   def process_domain_block!
     clear_media! if domain_block.reject_media?
     force_accounts_sensitive! if domain_block.force_sensitive?
diff --git a/app/services/unblock_domain_service.rb b/app/services/unblock_domain_service.rb
index eceecd6d7..ab246203f 100644
--- a/app/services/unblock_domain_service.rb
+++ b/app/services/unblock_domain_service.rb
@@ -3,10 +3,10 @@
 class UnblockDomainService < BaseService
   attr_accessor :domain_block
 
-  def call(domain_block)
+  def call(domain_block, destroy_domain_block = true)
     @domain_block = domain_block
     process_retroactive_updates
-    domain_block.destroy
+    domain_block.destroy if destroy_domain_block
   end
 
   def process_retroactive_updates
diff --git a/app/views/admin/domain_blocks/new.html.haml b/app/views/admin/domain_blocks/new.html.haml
index 2517b2714..d925edd64 100644
--- a/app/views/admin/domain_blocks/new.html.haml
+++ b/app/views/admin/domain_blocks/new.html.haml
@@ -20,5 +20,8 @@
   .fields-group
     = f.input :reject_reports, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.reject_reports'), hint: I18n.t('admin.domain_blocks.reject_reports_hint')
 
+  .fields-group
+    = f.input :reason, placeholder: t('admin.domain_blocks.reason', rows: 6)
+
   .actions
     = f.button :button, t('.create'), type: :submit
diff --git a/app/views/admin/domain_blocks/show.html.haml b/app/views/admin/domain_blocks/show.html.haml
index dca4dbac7..02c73c268 100644
--- a/app/views/admin/domain_blocks/show.html.haml
+++ b/app/views/admin/domain_blocks/show.html.haml
@@ -1,13 +1,30 @@
 - content_for :page_title do
   = t('admin.domain_blocks.show.title', domain: @domain_block.domain)
 
-= simple_form_for @domain_block, url: admin_domain_block_path(@domain_block), method: :delete do |f|
+= simple_form_for @domain_block, url: admin_domain_block_path(@domain_block) do |f|
+  = render 'shared/error_messages', object: @domain_block
 
-  - unless (@domain_block.noop?)
-    %p= t(".retroactive.#{@domain_block.severity}")
-    %p.hint= t(:affected_accounts,
-      scope: [:admin, :domain_blocks, :show],
-      count: @domain_block.affected_accounts_count)
+  .fields-row
+    .fields-row__column.fields-row__column-6.fields-group
+      = f.input :domain, wrapper: :with_label, label: t('admin.domain_blocks.domain'), required: true, disabled: true
+
+    .fields-row__column.fields-row__column-6.fields-group
+      = f.input :severity, collection: DomainBlock.severities.keys, wrapper: :with_label, include_blank: false, label_method: lambda { |type| t(".severity.#{type}") }
+
+  .fields-group
+    = f.input :force_sensitive, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.force_sensitive'), hint: I18n.t('admin.domain_blocks.force_sensitive_hint')
+
+  .fields-group
+    = f.input :reject_media, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.reject_media'), hint: I18n.t('admin.domain_blocks.reject_media_hint')
+
+  .fields-group
+    = f.input :reject_reports, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.reject_reports'), hint: I18n.t('admin.domain_blocks.reject_reports_hint')
+
+  .fields-group
+    = f.input :undo, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.undo'), hint: I18n.t('admin.domain_blocks.undo_hint')
+
+  .fields-group
+    = f.input :reason, placeholder: t('admin.domain_blocks.reason', rows: 6)
 
   .actions
-    = f.button :button, t('.undo'), type: :submit
+    = f.button :button, t('.edit'), type: :submit
diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml
index 9574c3147..322106346 100644
--- a/app/views/admin/instances/index.html.haml
+++ b/app/views/admin/instances/index.html.haml
@@ -46,6 +46,9 @@
               &bull;
               = t('admin.domain_blocks.rejecting_reports')
 
+            - if instance.domain_block.reason
+              = simple_format(h("Policy reason: #{instance.domain_block.reason}"))
+
       .avatar-stack
         - instance.cached_sample_accounts.each do |account|
           = image_tag current_account&.user&.setting_auto_play_gif ? account.avatar_original_url : account.avatar_static_url, width: 48, height: 48, alt: '', class: 'account__avatar'
diff --git a/app/views/admin/instances/show.html.haml b/app/views/admin/instances/show.html.haml
index c7992a490..b9aac5f11 100644
--- a/app/views/admin/instances/show.html.haml
+++ b/app/views/admin/instances/show.html.haml
@@ -31,6 +31,13 @@
           = fa_icon 'times'
       .dashboard__counters__label= t 'admin.instances.delivery_available'
 
+- if @domain_block
+  %hr.spacer/
+
+  %h3= "Affected by #{@domain_block.severity.gsub('_', ' ')} policy"
+  - if @domain_block.reason
+    = simple_format(h(@domain_block.reason))
+
 %hr.spacer/
 
 %div{ style: 'overflow: hidden' }
@@ -39,6 +46,6 @@
 
   %div{ style: 'float: right' }
     - if @domain_block
-      = link_to t('admin.domain_blocks.undo'), admin_domain_block_path(@domain_block), class: 'button'
+      = link_to t('admin.domain_blocks.edit'), admin_domain_block_path(@domain_block), class: 'button'
     - else
       = link_to t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: @instance.domain), class: 'button'
diff --git a/app/workers/domain_block_worker.rb b/app/workers/domain_block_worker.rb
index 884477829..4ae36d6d4 100644
--- a/app/workers/domain_block_worker.rb
+++ b/app/workers/domain_block_worker.rb
@@ -3,6 +3,8 @@
 class DomainBlockWorker
   include Sidekiq::Worker
 
+  sidekiq_options unique: :until_executed
+
   def perform(domain_block_id)
     BlockDomainService.new.call(DomainBlock.find(domain_block_id))
   rescue ActiveRecord::RecordNotFound
diff --git a/app/workers/domain_unblock_worker.rb b/app/workers/domain_unblock_worker.rb
new file mode 100644
index 000000000..2dbf25ac7
--- /dev/null
+++ b/app/workers/domain_unblock_worker.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class DomainUnblockWorker
+  include Sidekiq::Worker
+
+  sidekiq_options unique: :until_executed
+
+  def perform(domain_block_id)
+    UnblockDomainService.new.call(DomainBlock.find(domain_block_id))
+  rescue ActiveRecord::RecordNotFound
+    true
+  end
+end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 8210ef70d..c9a363ec4 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -204,7 +204,7 @@ en:
         confirm_user: "%{name} confirmed e-mail address of creature %{target}"
         create_account_warning: "%{name} sent a warning to %{target}"
         create_custom_emoji: "%{name} uploaded new emoji %{target}"
-        create_domain_block: "%{name} changed policy for %{target}"
+        create_domain_block: "%{name} added or changed policy for %{target}"
         create_email_domain_block: "%{name} blacklisted e-mail domain %{target}"
         demote_user: "%{name} demoted creature %{target}"
         destroy_custom_emoji: "%{name} destroyed emoji %{target}"
@@ -233,6 +233,7 @@ en:
         unsilence_account: "%{name} unsilenced %{target}'s account"
         unsuspend_account: "%{name} unsuspended %{target}'s account"
         update_custom_emoji: "%{name} updated emoji %{target}"
+        update_domain_block: "%{name} updated policy for %{target}"
         update_status: "%{name} updated roar by %{target}"
       deleted_status: "(deleted roar)"
       title: Audit log
@@ -288,6 +289,7 @@ en:
     domain_blocks:
       add_new: Add new domain policy
       created_msg: Domain policy is now being processed
+      updated_msg: Domain policy change is now being processed
       destroyed_msg: Domain policy has been undone
       domain: Domain
       existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to <a href="%{unblock_url}">remove it</a> first.
@@ -303,6 +305,7 @@ en:
         title: New domain policy
       force_sensitive: Mark media sensitive
       force_sensitive_hint: Forces all media from this domain to be marked sensitive.
+      reason: Add notes here.
       reject_media: Reject media files
       reject_media_hint: Removes locally stored media files and refuses to download any in the future. Irrelevant for suspensions
       reject_reports: Reject reports
@@ -321,9 +324,12 @@ en:
           silence: Unsilence existing affected accounts from this domain
           suspend: Unsuspend existing affected accounts from this domain
           force_unlisted: Allow public roars on all existing accounts from this domain
-        title: Undo domain policy for %{domain}
+        title: Edit domain policy for %{domain}
         undo: Undo
+        edit: Edit
       undo: Undo domain policy
+      edit: Edit domain policy
+      undo_hint: Removes the domain policy, allowing for normal federation
     email_domain_blocks:
       add_new: Add new
       created_msg: Successfully added e-mail domain to blacklist
diff --git a/config/routes.rb b/config/routes.rb
index 5f4e5e7f8..e716ab383 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -150,7 +150,7 @@ Rails.application.routes.draw do
     get '/dashboard', to: 'dashboard#index'
 
     resources :subscriptions, only: [:index]
-    resources :domain_blocks, only: [:new, :create, :show, :destroy]
+    resources :domain_blocks, only: [:new, :create, :show, :destroy, :update]
     resources :email_domain_blocks, only: [:index, :new, :create, :destroy]
     resources :action_logs, only: [:index]
     resources :warning_presets, except: [:new]
diff --git a/db/migrate/20190730213656_add_reason_to_domain_blocks.rb b/db/migrate/20190730213656_add_reason_to_domain_blocks.rb
new file mode 100644
index 000000000..2e768b173
--- /dev/null
+++ b/db/migrate/20190730213656_add_reason_to_domain_blocks.rb
@@ -0,0 +1,5 @@
+class AddReasonToDomainBlocks < ActiveRecord::Migration[5.2]
+  def change
+    add_column :domain_blocks, :reason, :text
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index c246b70bf..4d7085515 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_07_28_201832) do
+ActiveRecord::Schema.define(version: 2019_07_30_213656) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -269,6 +269,7 @@ ActiveRecord::Schema.define(version: 2019_07_28_201832) do
     t.boolean "reject_media", default: false, null: false
     t.boolean "reject_reports", default: false, null: false
     t.boolean "force_sensitive", default: false, null: false
+    t.text "reason"
     t.index ["domain"], name: "index_domain_blocks_on_domain", unique: true
   end