Feature: Report improvements (#6967) (#7000)

* Implement Assignment of Reports (#6967)

* Change translation of admin.report.comment.label to "Report Comment" for clarity

As we'll soon add the ability for reports to have comments on them, this clarification makes sense.

* Implement notes for Reports

This enables moderators to leave comments about a report whilst they work on it

* Fix display of report moderation notes

* Allow reports to be reopened / marked as unresolved

* Redirect to reports listing upon resolution of report

* Implement "resolve with note" functionality

* Add inverse relationship for report notes

* Remove additional database querying when loading report notes

* Fix tests for reports

* Fix localisations for report notes / reports

app/controllers/admin/report_notes_controller.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module Admin
+  class ReportNotesController < BaseController
+    before_action :set_report_note, only: [:destroy]
+
+    def create
+      authorize ReportNote, :create?
+
+      @report_note = current_account.report_notes.new(resource_params)
+
+      if @report_note.save
+        if params[:create_and_resolve]
+          @report_note.report.update!(action_taken: true, action_taken_by_account_id: current_account.id)
+          log_action :resolve, @report_note.report
+
+          redirect_to admin_reports_path, notice: I18n.t('admin.reports.resolved_msg')
+        else
+          redirect_to admin_report_path(@report_note.report_id), notice: I18n.t('admin.report_notes.created_msg')
+        end
+      else
+        @report       = @report_note.report
+        @report_notes = @report.notes.latest
+        @form = Form::StatusBatch.new
+
+        render template: 'admin/reports/show'
+      end
+    end
+
+    def destroy
+      authorize @report_note, :destroy?
+      @report_note.destroy!
+      redirect_to admin_report_path(@report_note.report_id), notice: I18n.t('admin.report_notes.destroyed_msg')
+    end
+
+    private
+
+    def resource_params
+      params.require(:report_note).permit(
+        :content,
+        :report_id
+      )
+    end
+
+    def set_report_note
+      @report_note = ReportNote.find(params[:id])
+    end
+  end
+end

app/controllers/admin/reports_controller.rb

@@ -11,19 +11,35 @@ module Admin
 
     def show
       authorize @report, :show?
+      @report_note = @report.notes.new
+      @report_notes = @report.notes.latest
       @form = Form::StatusBatch.new
     end
 
     def update
       authorize @report, :update?
       process_report
-      redirect_to admin_report_path(@report)
+
+      if @report.action_taken?
+        redirect_to admin_reports_path, notice: I18n.t('admin.reports.resolved_msg')
+      else
+        redirect_to admin_report_path(@report)
+      end
     end
 
     private
 
     def process_report
       case params[:outcome].to_s
+      when 'assign_to_self'
+        @report.update!(assigned_account_id: current_account.id)
+        log_action :assigned_to_self, @report
+      when 'unassign'
+        @report.update!(assigned_account_id: nil)
+        log_action :unassigned, @report
+      when 'reopen'
+        @report.update!(action_taken: false, action_taken_by_account_id: nil)
+        log_action :reopen, @report
       when 'resolve'
         @report.update!(action_taken_by_current_attributes)
         log_action :resolve, @report
@@ -32,11 +48,13 @@ module Admin
         log_action :resolve, @report
         log_action :suspend, @report.target_account
         resolve_all_target_account_reports
+        @report.reload
       when 'silence'
         @report.target_account.update!(silenced: true)
         log_action :resolve, @report
         log_action :silence, @report.target_account
         resolve_all_target_account_reports
+        @report.reload
       else
         raise ActiveRecord::RecordNotFound
       end

app/helpers/admin/action_logs_helper.rb

@@ -86,7 +86,7 @@ module Admin::ActionLogsHelper
       opposite_verbs?(log) ? 'negative' : 'positive'
     when :update, :reset_password, :disable_2fa, :memorialize
       'neutral'
-    when :demote, :silence, :disable, :suspend, :remove_avatar
+    when :demote, :silence, :disable, :suspend, :remove_avatar, :reopen
       'negative'
     when :destroy
       opposite_verbs?(log) ? 'positive' : 'negative'

app/models/account.rb

@@ -95,6 +95,8 @@ class Account < ApplicationRecord
   has_many :reports
   has_many :targeted_reports, class_name: 'Report', foreign_key: :target_account_id
 
+  has_many :report_notes, dependent: :destroy
+
   # Moderation notes
   has_many :account_moderation_notes, dependent: :destroy
   has_many :targeted_moderation_notes, class_name: 'AccountModerationNote', foreign_key: :target_account_id, dependent: :destroy

app/models/report.rb

@@ -12,12 +12,16 @@
 #  account_id                 :integer          not null
 #  action_taken_by_account_id :integer
 #  target_account_id          :integer          not null
+#  assigned_account_id        :integer
 #
 
 class Report < ApplicationRecord
   belongs_to :account
   belongs_to :target_account, class_name: 'Account'
   belongs_to :action_taken_by_account, class_name: 'Account', optional: true
+  belongs_to :assigned_account, class_name: 'Account', optional: true
+
+  has_many :notes, class_name: 'ReportNote', foreign_key: :report_id, inverse_of: :report, dependent: :destroy
 
   scope :unresolved, -> { where(action_taken: false) }
   scope :resolved,   -> { where(action_taken: true) }

app/models/report_note.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+# == Schema Information
+#
+# Table name: report_notes
+#
+#  id         :integer          not null, primary key
+#  content    :text             not null
+#  report_id  :integer          not null
+#  account_id :integer          not null
+#  created_at :datetime         not null
+#  updated_at :datetime         not null
+#
+
+class ReportNote < ApplicationRecord
+  belongs_to :account
+  belongs_to :report, inverse_of: :notes
+
+  scope :latest, -> { reorder('created_at ASC') }
+
+  validates :content, presence: true, length: { maximum: 500 }
+end

app/policies/report_note_policy.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class ReportNotePolicy < ApplicationPolicy
+  def create?
+    staff?
+  end
+
+  def destroy?
+    admin? || owner?
+  end
+
+  private
+
+  def owner?
+    record.account_id == current_account&.id
+  end
+end

app/views/admin/report_notes/_report_note.html.haml

@@ -0,0 +1,11 @@
+%tr
+  %td
+    %p
+      %strong= report_note.account.acct
+      on
+      %time.formatted{ datetime: report_note.created_at.iso8601, title: l(report_note.created_at) }
+        = l report_note.created_at
+      = table_link_to 'trash', t('admin.reports.notes.delete'), admin_report_note_path(report_note), method: :delete if can?(:destroy, report_note)
+      %br/
+      %br/
+    = simple_format(h(report_note.content))

app/views/admin/reports/_report.html.haml

@@ -17,5 +17,10 @@
       %span{ title: t('admin.accounts.media_attachments') }
         = fa_icon('camera')
         = report.media_attachments.count
+  %td
+    - if report.assigned_account.nil?
+      \-
+    - else
+      = link_to report.assigned_account.acct, admin_account_path(report.assigned_account.id)
   %td
     = table_link_to 'circle', t('admin.reports.view'), admin_report_path(report)

app/views/admin/reports/index.html.haml

@@ -20,6 +20,7 @@
           %th= t('admin.reports.reported_by')
           %th= t('admin.reports.comment.label')
           %th= t('admin.reports.report_contents')
+          %th= t('admin.reports.assigned')
           %th
       %tbody
         = render @reports

app/views/admin/reports/show.html.haml

@@ -4,24 +4,68 @@
 - content_for :page_title do
   = t('admin.reports.report', id: @report.id)
 
+%div{ style: 'overflow: hidden; margin-bottom: 20px' }
+  - if !@report.action_taken?
+    %div{ style: 'float: right' }
+      = link_to t('admin.reports.silence_account'), admin_report_path(@report, outcome: 'silence'), method: :put, class: 'button'
+      = link_to t('admin.reports.suspend_account'), admin_report_path(@report, outcome: 'suspend'), method: :put, class: 'button'
+    %div{ style: 'float: left' }
+      = link_to t('admin.reports.mark_as_resolved'), admin_report_path(@report, outcome: 'resolve'), method: :put, class: 'button'
+  - else
+    = link_to t('admin.reports.mark_as_unresolved'), admin_report_path(@report, outcome: 'reopen'), method: :put, class: 'button'
+
+.table-wrapper
+  %table.table.inline-table
+    %tbody
+      %tr
+        %th= t('admin.reports.updated_at')
+        %td{colspan: 2}
+          %time.formatted{ datetime: @report.updated_at.iso8601 }
+      %tr
+        %th= t('admin.reports.status')
+        %td{colspan: 2}
+          - if @report.action_taken?
+            = t('admin.reports.resolved')
+            = table_link_to 'envelope-open', t('admin.reports.reopen'), admin_report_path(@report, outcome: 'reopen'), method: :put
+          - else
+            = t('admin.reports.unresolved')
+      - if !@report.action_taken_by_account.nil?
+        %tr
+          %th= t('admin.reports.action_taken_by')
+          %td= @report.action_taken_by_account.acct
+      - else
+        %tr
+          %th= t('admin.reports.assigned')
+          %td
+            - if @report.assigned_account.nil?
+              \-
+            - else
+              = link_to @report.assigned_account.acct, admin_account_path(@report.assigned_account.id)
+          %td{style: "text-align: right"}
+            - if @report.assigned_account != current_user.account
+              = table_link_to 'user', t('admin.reports.assign_to_self'), admin_report_path(@report, outcome: 'assign_to_self'), method: :put
+            - if !@report.assigned_account.nil?
+              = table_link_to 'trash', t('admin.reports.unassign'), admin_report_path(@report, outcome: 'unassign'), method: :put
+
 .report-accounts
   .report-accounts__item
-    %strong= t('admin.reports.reported_account')
+    %h3= t('admin.reports.reported_account')
     = render 'authorize_follows/card', account: @report.target_account, admin: true
     = render 'admin/accounts/card', account: @report.target_account
   .report-accounts__item
-    %strong= t('admin.reports.reported_by')
+    %h3= t('admin.reports.reported_by')
     = render 'authorize_follows/card', account: @report.account, admin: true
     = render 'admin/accounts/card', account: @report.account
 
-%p
-  %strong= t('admin.reports.comment.label')
-  \:
-  = simple_format(@report.comment.presence || t('admin.reports.comment.none'))
+%h3= t('admin.reports.comment.label')
+
+= simple_format(@report.comment.presence || t('admin.reports.comment.none'))
 
 - unless @report.statuses.empty?
   %hr/
 
+  %h3= t('admin.reports.statuses')
+
   = form_for(@form, url: admin_report_reported_statuses_path(@report.id)) do |f|
     .batch-form-box
       .batch-checkbox-all
@@ -46,14 +90,20 @@
 
 %hr/
 
-- if !@report.action_taken?
-  %div{ style: 'overflow: hidden' }
-    %div{ style: 'float: right' }
-      = link_to t('admin.reports.silence_account'), admin_report_path(@report, outcome: 'silence'), method: :put, class: 'button'
-      = link_to t('admin.reports.suspend_account'), admin_report_path(@report, outcome: 'suspend'), method: :put, class: 'button'
-    %div{ style: 'float: left' }
-      = link_to t('admin.reports.mark_as_resolved'), admin_report_path(@report, outcome: 'resolve'), method: :put, class: 'button'
-- elsif !@report.action_taken_by_account.nil?
-  %p
-    %strong #{t('admin.reports.action_taken_by')}:
-    = @report.action_taken_by_account.acct
+%h3= t('admin.reports.notes.label')
+
+- if @report_notes.length > 0
+  .table-wrapper
+    %table.table
+      %thead
+        %tr
+          %th
+      %tbody
+        = render @report_notes
+
+= simple_form_for @report_note, url: admin_report_notes_path do |f|
+  = render 'shared/error_messages', object: @report_note
+  = f.input :content
+  = f.hidden_field :report_id
+  = f.button :button, t('admin.reports.notes.create'), type: :submit
+  = f.button :button, t('admin.reports.notes.create_and_resolve'), type: :submit, name: :create_and_resolve

config/locales/en.yml

@@ -137,6 +137,7 @@ en:
       web: Web
     action_logs:
       actions:
+        assigned_to_self_report: "%{name} assigned report %{target} to themselves"
         confirm_user: "%{name} confirmed e-mail address of user %{target}"
         create_custom_emoji: "%{name} uploaded new emoji %{target}"
         create_domain_block: "%{name} blocked domain %{target}"
@@ -153,10 +154,12 @@ en:
         memorialize_account: "%{name} turned %{target}'s account into a memoriam page"
         promote_user: "%{name} promoted user %{target}"
         remove_avatar_user: "%{name} removed %{target}'s avatar"
+        reopen_report: "%{name} reopened report %{target}"
         reset_password_user: "%{name} reset password of user %{target}"
-        resolve_report: "%{name} dismissed report %{target}"
+        resolve_report: "%{name} resolved report %{target}"
         silence_account: "%{name} silenced %{target}'s account"
         suspend_account: "%{name} suspended %{target}'s account"
+        unassigned_report: "%{name} unassigned report %{target}"
         unsilence_account: "%{name} unsilenced %{target}'s account"
         unsuspend_account: "%{name} unsuspended %{target}'s account"
         update_custom_emoji: "%{name} updated emoji %{target}"
@@ -242,15 +245,26 @@ en:
         expired: Expired
         title: Filter
       title: Invites
+    report_notes:
+      created_msg: Moderation note successfully created!
+      destroyed_msg: Moderation note successfully destroyed!
     reports:
       action_taken_by: Action taken by
       are_you_sure: Are you sure?
+      assign_to_self: Assign to me
+      assigned: Assigned Moderator
       comment:
-        label: Comment
+        label: Report Comment
         none: None
       delete: Delete
       id: ID
       mark_as_resolved: Mark as resolved
+      mark_as_unresolved: Mark as unresolved
+      notes:
+        create: Add Note
+        create_and_resolve: Resolve with Note
+        delete: Delete
+        label: Notes
       nsfw:
         'false': Unhide media attachments
         'true': Hide media attachments
@@ -259,12 +273,16 @@ en:
       reported_account: Reported account
       reported_by: Reported by
       resolved: Resolved
+      resolved_msg: Report successfully resolved!
       silence_account: Silence account
       status: Status
+      statuses: Reported Toots
       suspend_account: Suspend account
       target: Target
       title: Reports
+      unassign: Unassign
       unresolved: Unresolved
+      updated_at: Updated
       view: View
     settings:
       activity_api_enabled:

config/routes.rb

@@ -137,6 +137,8 @@ Rails.application.routes.draw do
       resources :reported_statuses, only: [:create, :update, :destroy]
     end
 
+    resources :report_notes, only: [:create, :destroy]
+
     resources :accounts, only: [:index, :show] do
       member do
         post :subscribe

db/migrate/20180402031200_add_assigned_account_id_to_reports.rb

@@ -0,0 +1,5 @@
+class AddAssignedAccountIdToReports < ActiveRecord::Migration[5.1]
+  def change
+    add_reference :reports, :assigned_account, null: true, default: nil, foreign_key: { on_delete: :nullify, to_table: :accounts }, index: false
+  end
+end

db/migrate/20180402040909_create_report_notes.rb

@@ -0,0 +1,14 @@
+class CreateReportNotes < ActiveRecord::Migration[5.1]
+  def change
+    create_table :report_notes do |t|
+      t.text :content, null: false
+      t.references :report, null: false
+      t.references :account, null: false
+
+      t.timestamps
+    end
+
+    add_foreign_key :report_notes, :reports, column: :report_id, on_delete: :cascade
+    add_foreign_key :report_notes, :accounts, column: :account_id, on_delete: :cascade
+  end
+end

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: 20180310000000) do
+ActiveRecord::Schema.define(version: 20180402040909) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -355,6 +355,16 @@ ActiveRecord::Schema.define(version: 20180310000000) do
     t.index ["status_id", "preview_card_id"], name: "index_preview_cards_statuses_on_status_id_and_preview_card_id"
   end
 
+  create_table "report_notes", force: :cascade do |t|
+    t.text "content", null: false
+    t.bigint "report_id", null: false
+    t.bigint "account_id", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["account_id"], name: "index_report_notes_on_account_id"
+    t.index ["report_id"], name: "index_report_notes_on_report_id"
+  end
+
   create_table "reports", force: :cascade do |t|
     t.bigint "status_ids", default: [], null: false, array: true
     t.text "comment", default: "", null: false
@@ -364,6 +374,7 @@ ActiveRecord::Schema.define(version: 20180310000000) do
     t.bigint "account_id", null: false
     t.bigint "action_taken_by_account_id"
     t.bigint "target_account_id", null: false
+    t.bigint "assigned_account_id"
     t.index ["account_id"], name: "index_reports_on_account_id"
     t.index ["target_account_id"], name: "index_reports_on_target_account_id"
   end
@@ -569,7 +580,10 @@ ActiveRecord::Schema.define(version: 20180310000000) do
   add_foreign_key "oauth_access_tokens", "oauth_applications", column: "application_id", name: "fk_f5fc4c1ee3", on_delete: :cascade
   add_foreign_key "oauth_access_tokens", "users", column: "resource_owner_id", name: "fk_e84df68546", on_delete: :cascade
   add_foreign_key "oauth_applications", "users", column: "owner_id", name: "fk_b0988c7c0a", on_delete: :cascade
+  add_foreign_key "report_notes", "accounts", on_delete: :cascade
+  add_foreign_key "report_notes", "reports", on_delete: :cascade
   add_foreign_key "reports", "accounts", column: "action_taken_by_account_id", name: "fk_bca45b75fd", on_delete: :nullify
+  add_foreign_key "reports", "accounts", column: "assigned_account_id", on_delete: :nullify
   add_foreign_key "reports", "accounts", column: "target_account_id", name: "fk_eb37af34f0", on_delete: :cascade
   add_foreign_key "reports", "accounts", name: "fk_4b81f7522c", on_delete: :cascade
   add_foreign_key "session_activations", "oauth_access_tokens", column: "access_token_id", name: "fk_957e5bda89", on_delete: :cascade

spec/controllers/admin/reports_controller_spec.rb

@@ -61,7 +61,7 @@ describe Admin::ReportsController do
         report = Fabricate(:report)
 
         put :update, params: { id: report, outcome: 'resolve' }
-        expect(response).to redirect_to(admin_report_path(report))
+        expect(response).to redirect_to(admin_reports_path)
         report.reload
         expect(report.action_taken_by_account).to eq user.account
         expect(report.action_taken).to eq true
@@ -74,7 +74,7 @@ describe Admin::ReportsController do
         allow(Admin::SuspensionWorker).to receive(:perform_async)
 
         put :update, params: { id: report, outcome: 'suspend' }
-        expect(response).to redirect_to(admin_report_path(report))
+        expect(response).to redirect_to(admin_reports_path)
         report.reload
         expect(report.action_taken_by_account).to eq user.account
         expect(report.action_taken).to eq true
@@ -88,12 +88,46 @@ describe Admin::ReportsController do
         report = Fabricate(:report)
 
         put :update, params: { id: report, outcome: 'silence' }
-        expect(response).to redirect_to(admin_report_path(report))
+        expect(response).to redirect_to(admin_reports_path)
         report.reload
         expect(report.action_taken_by_account).to eq user.account
         expect(report.action_taken).to eq true
         expect(report.target_account).to be_silenced
       end
     end
+
+    describe 'with an outsome of `reopen`' do
+      it 'reopens the report' do
+        report = Fabricate(:report)
+
+        put :update, params: { id: report, outcome: 'reopen' }
+        expect(response).to redirect_to(admin_report_path(report))
+        report.reload
+        expect(report.action_taken_by_account).to eq nil
+        expect(report.action_taken).to eq false
+      end
+    end
+
+    describe 'with an outsome of `assign_to_self`' do
+      it 'reopens the report' do
+        report = Fabricate(:report)
+
+        put :update, params: { id: report, outcome: 'assign_to_self' }
+        expect(response).to redirect_to(admin_report_path(report))
+        report.reload
+        expect(report.assigned_account).to eq user.account
+      end
+    end
+
+    describe 'with an outsome of `unassign`' do
+      it 'reopens the report' do
+        report = Fabricate(:report)
+
+        put :update, params: { id: report, outcome: 'unassign' }
+        expect(response).to redirect_to(admin_report_path(report))
+        report.reload
+        expect(report.assigned_account).to eq nil
+      end
+    end
   end
 end