From d9b62e34da0c0238176f27557ac7b953da94df7e Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Tue, 10 Apr 2018 20:27:59 +0200 Subject: [PATCH] Feature: Improve reports ui (#7032) * Further improvements to Reports UI - Clean up notes display - Clean up add new note form - Simplify controller - Allow reopening a report with a note - Show created at date for reports - Fix report details table formatting * Show history of report using Admin::ActionLog beneath the report * Fix incorrect log message when reopening a report * Implement fetching of all ActionLog items that could be related to the report * Ensure adding a report_note updates the report's updated_at * Limit Report History to actions that happened between the report being created and the report being resolved * Fix linting issues * Improve report history builder Thanks @gargron for the improvements --- .../admin/report_notes_controller.rb | 17 +++++-- app/controllers/admin/reports_controller.rb | 20 ++++---- app/javascript/styles/mastodon/admin.scss | 35 ++++++++++++++ app/models/report.rb | 46 ++++++++++++++++++ app/models/report_note.rb | 2 +- .../admin/report_notes/_report_note.html.haml | 12 ++--- app/views/admin/reports/show.html.haml | 47 ++++++++++++------- config/locales/en.yml | 11 +++-- 8 files changed, 147 insertions(+), 43 deletions(-) diff --git a/app/controllers/admin/report_notes_controller.rb b/app/controllers/admin/report_notes_controller.rb index ef8c0f469..bcb3f2026 100644 --- a/app/controllers/admin/report_notes_controller.rb +++ b/app/controllers/admin/report_notes_controller.rb @@ -8,19 +8,26 @@ module Admin authorize ReportNote, :create? @report_note = current_account.report_notes.new(resource_params) + @report = @report_note.report 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 + @report.resolve!(current_account) + log_action :resolve, @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') + return end + + if params[:create_and_unresolve] + @report.unresolve! + log_action :reopen, @report + end + + redirect_to admin_report_path(@report), notice: I18n.t('admin.report_notes.created_msg') else - @report = @report_note.report @report_notes = @report.notes.latest + @report_history = @report.history @form = Form::StatusBatch.new render template: 'admin/reports/show' diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index fc3785e3b..a4ae9507d 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -13,6 +13,7 @@ module Admin authorize @report, :show? @report_note = @report.notes.new @report_notes = @report.notes.latest + @report_history = @report.history @form = Form::StatusBatch.new end @@ -38,36 +39,33 @@ module Admin @report.update!(assigned_account_id: nil) log_action :unassigned, @report when 'reopen' - @report.update!(action_taken: false, action_taken_by_account_id: nil) + @report.unresolve! log_action :reopen, @report when 'resolve' - @report.update!(action_taken_by_current_attributes) + @report.resolve!(current_account) log_action :resolve, @report when 'suspend' Admin::SuspensionWorker.perform_async(@report.target_account.id) + 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 - end - - def action_taken_by_current_attributes - { action_taken: true, action_taken_by_account_id: current_account.id } + @report.reload end def resolve_all_target_account_reports - unresolved_reports_for_target_account.update_all( - action_taken_by_current_attributes - ) + unresolved_reports_for_target_account.update_all(action_taken: true, action_taken_by_account_id: current_account.id) end def unresolved_reports_for_target_account diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index e6bd0c717..6bd659030 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -145,6 +145,11 @@ border: 0; background: transparent; border-bottom: 1px solid $ui-base-color; + + &.section-break { + margin: 30px 0; + border-bottom: 2px solid $ui-base-lighter-color; + } } .muted-hint { @@ -330,6 +335,36 @@ } } +.report-note__comment { + margin-bottom: 20px; +} + +.report-note__form { + margin-bottom: 20px; + + .report-note__textarea { + box-sizing: border-box; + border: 0; + padding: 7px 4px; + margin-bottom: 10px; + font-size: 16px; + color: $ui-base-color; + display: block; + width: 100%; + outline: 0; + font-family: inherit; + resize: vertical; + } + + .report-note__buttons { + text-align: right; + } + + .report-note__button { + margin: 0 0 5px 5px; + } +} + .batch-form-box { display: flex; flex-wrap: wrap; diff --git a/app/models/report.rb b/app/models/report.rb index f5b37cb6d..5b90c7bce 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -39,4 +39,50 @@ class Report < ApplicationRecord def media_attachments MediaAttachment.where(status_id: status_ids) end + + def assign_to_self!(current_account) + update!(assigned_account_id: current_account.id) + end + + def unassign! + update!(assigned_account_id: nil) + end + + def resolve!(acting_account) + update!(action_taken: true, action_taken_by_account_id: acting_account.id) + end + + def unresolve! + update!(action_taken: false, action_taken_by_account_id: nil) + end + + def unresolved? + !action_taken? + end + + def history + time_range = created_at..updated_at + + sql = [ + Admin::ActionLog.where( + target_type: 'Report', + target_id: id, + created_at: time_range + ).unscope(:order), + + Admin::ActionLog.where( + target_type: 'Account', + target_id: target_account_id, + created_at: time_range + ).unscope(:order), + + Admin::ActionLog.where( + target_type: 'Status', + target_id: status_ids, + created_at: time_range + ).unscope(:order), + ].map { |query| "(#{query.to_sql})" }.join(' UNION ALL ') + + Admin::ActionLog.from("(#{sql}) AS admin_action_logs") + end end diff --git a/app/models/report_note.rb b/app/models/report_note.rb index 3d12cf7b6..6d9dec80a 100644 --- a/app/models/report_note.rb +++ b/app/models/report_note.rb @@ -13,7 +13,7 @@ class ReportNote < ApplicationRecord belongs_to :account - belongs_to :report, inverse_of: :notes + belongs_to :report, inverse_of: :notes, touch: true scope :latest, -> { reorder('created_at ASC') } diff --git a/app/views/admin/report_notes/_report_note.html.haml b/app/views/admin/report_notes/_report_note.html.haml index 60ac5d0d5..1f621e0d3 100644 --- a/app/views/admin/report_notes/_report_note.html.haml +++ b/app/views/admin/report_notes/_report_note.html.haml @@ -1,11 +1,9 @@ -%tr - %td - %p - %strong= report_note.account.acct - on +%li + %h4 + = report_note.account.acct + %div{ style: 'float: right' } %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/ + %div{ class: 'report-note__comment' } = simple_format(h(report_note.content)) diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index e7634a034..d57b4adc8 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -5,7 +5,7 @@ = t('admin.reports.report', id: @report.id) %div{ style: 'overflow: hidden; margin-bottom: 20px' } - - if !@report.action_taken? + - if @report.unresolved? %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' @@ -17,22 +17,29 @@ .table-wrapper %table.table.inline-table %tbody + %tr + %th= t('admin.reports.created_at') + %td{colspan: 2} + %time.formatted{ datetime: @report.created_at.iso8601 } %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} + %td - 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') + %td{style: "text-align: right; overflow: hidden;"} + - if @report.action_taken? + = table_link_to 'envelope-open', t('admin.reports.reopen'), admin_report_path(@report, outcome: 'reopen'), method: :put - if !@report.action_taken_by_account.nil? %tr %th= t('admin.reports.action_taken_by') - %td= @report.action_taken_by_account.acct + %td{colspan: 2} + = @report.action_taken_by_account.acct - else %tr %th= t('admin.reports.assigned') @@ -47,6 +54,8 @@ - if !@report.assigned_account.nil? = table_link_to 'trash', t('admin.reports.unassign'), admin_report_path(@report, outcome: 'unassign'), method: :put +%hr{ class: "section-break"}/ + .report-accounts .report-accounts__item %h3= t('admin.reports.reported_account') @@ -88,22 +97,28 @@ = link_to admin_report_reported_status_path(@report, status), method: :delete, class: 'icon-button trash-button', title: t('admin.reports.delete'), data: { confirm: t('admin.reports.are_you_sure') }, remote: true do = fa_icon 'trash' -%hr/ +%hr{ class: "section-break"}/ %h3= t('admin.reports.notes.label') - if @report_notes.length > 0 - .table-wrapper - %table.table - %thead - %tr - %th - %tbody - = render @report_notes + %ul + = render @report_notes -= simple_form_for @report_note, url: admin_report_notes_path do |f| +%h4= t('admin.reports.notes.new_label') += form_for @report_note, url: admin_report_notes_path, html: { class: 'report-note__form' } do |f| = render 'shared/error_messages', object: @report_note - = f.input :content + = f.text_area :content, placeholder: t('admin.reports.notes.placeholder'), rows: 6, class: 'report-note__textarea' = 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 + %div{ class: 'report-note__buttons' } + - if @report.unresolved? + = f.submit t('admin.reports.notes.create_and_resolve'), name: :create_and_resolve, class: 'button report-note__button' + - else + = f.submit t('admin.reports.notes.create_and_unresolve'), name: :create_and_unresolve, class: 'button report-note__button' + = f.submit t('admin.reports.notes.create'), class: 'button report-note__button' + +- if @report_history.length > 0 + %h3= t('admin.reports.history') + + %ul + = render @report_history diff --git a/config/locales/en.yml b/config/locales/en.yml index 11f3fb924..98f135966 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -256,8 +256,8 @@ en: title: Filter title: Invites report_notes: - created_msg: Moderation note successfully created! - destroyed_msg: Moderation note successfully destroyed! + created_msg: Report note successfully created! + destroyed_msg: Report note successfully deleted! reports: action_taken_by: Action taken by are_you_sure: Are you sure? @@ -266,15 +266,20 @@ en: comment: label: Report Comment none: None + created_at: Reported delete: Delete + history: Moderation History id: ID mark_as_resolved: Mark as resolved mark_as_unresolved: Mark as unresolved notes: create: Add Note create_and_resolve: Resolve with Note + create_and_unresolve: Reopen with Note delete: Delete - label: Notes + label: Moderator Notes + new_label: Add Moderator Note + placeholder: Describe what actions have been taken, or any other updates to this report… nsfw: 'false': Unhide media attachments 'true': Hide media attachments