From 5fd56512de244263b4b0df998b8a83c303c3d1c5 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 15 May 2024 15:38:36 +0200 Subject: [PATCH] Improve Report Notes and Account Moderation Notes (#30288) --- app/models/account_moderation_note.rb | 2 +- app/models/report_note.rb | 2 +- app/views/admin/accounts/show.html.haml | 10 ++++---- app/views/admin/reports/show.html.haml | 14 ++++++----- config/navigation.rb | 4 ++-- ...ccount_moderation_notes_controller_spec.rb | 13 ++++++++-- .../admin/report_notes_controller_spec.rb | 24 ++++++++++++++----- 7 files changed, 47 insertions(+), 22 deletions(-) diff --git a/app/models/account_moderation_note.rb b/app/models/account_moderation_note.rb index ad49b24229f..79b8b4d25ee 100644 --- a/app/models/account_moderation_note.rb +++ b/app/models/account_moderation_note.rb @@ -13,7 +13,7 @@ # class AccountModerationNote < ApplicationRecord - CONTENT_SIZE_LIMIT = 500 + CONTENT_SIZE_LIMIT = 2_000 belongs_to :account belongs_to :target_account, class_name: 'Account' diff --git a/app/models/report_note.rb b/app/models/report_note.rb index b5c40a18b14..7361c97e670 100644 --- a/app/models/report_note.rb +++ b/app/models/report_note.rb @@ -13,7 +13,7 @@ # class ReportNote < ApplicationRecord - CONTENT_SIZE_LIMIT = 500 + CONTENT_SIZE_LIMIT = 2_000 belongs_to :account belongs_to :report, inverse_of: :notes, touch: true diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 41fcafa29d5..bcf7c073141 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -62,14 +62,16 @@ .report-notes = render partial: 'admin/report_notes/report_note', collection: @moderation_notes - = simple_form_for @account_moderation_note, url: admin_account_moderation_notes_path do |f| - = f.hidden_field :target_account_id + = simple_form_for @account_moderation_note, url: admin_account_moderation_notes_path do |form| + = form.hidden_field :target_account_id + + = render 'shared/error_messages', object: @account_moderation_note .field-group - = f.input :content, placeholder: t('admin.reports.notes.placeholder'), rows: 6 + = form.input :content, input_html: { placeholder: t('admin.reports.notes.placeholder'), maxlength: AccountModerationNote::CONTENT_SIZE_LIMIT, rows: 6, autofocus: @account_moderation_note.errors.any? } .actions - = f.button :button, t('admin.account_moderation_notes.create'), type: :submit + = form.button :button, t('admin.account_moderation_notes.create'), type: :submit %hr.spacer/ diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index c880021cffb..842aa515970 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -83,15 +83,17 @@ .report-notes = render @report_notes -= simple_form_for @report_note, url: admin_report_notes_path do |f| - = f.input :report_id, as: :hidden += simple_form_for @report_note, url: admin_report_notes_path do |form| + = form.input :report_id, as: :hidden + + = render 'shared/error_messages', object: @report_note .field-group - = f.input :content, placeholder: t('admin.reports.notes.placeholder'), rows: 6 + = form.input :content, input_html: { placeholder: t('admin.reports.notes.placeholder'), maxlength: ReportNote::CONTENT_SIZE_LIMIT, rows: 6, autofocus: @report_note.errors.any? } .actions - if @report.unresolved? - = f.button :button, t('admin.reports.notes.create_and_resolve'), name: :create_and_resolve, type: :submit + = form.button :button, t('admin.reports.notes.create_and_resolve'), name: :create_and_resolve, type: :submit - else - = f.button :button, t('admin.reports.notes.create_and_unresolve'), name: :create_and_unresolve, type: :submit - = f.button :button, t('admin.reports.notes.create'), type: :submit + = form.button :button, t('admin.reports.notes.create_and_unresolve'), name: :create_and_unresolve, type: :submit + = form.button :button, t('admin.reports.notes.create'), type: :submit diff --git a/config/navigation.rb b/config/navigation.rb index 791025d526a..b6e3f495056 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -45,8 +45,8 @@ SimpleNavigation::Configuration.run do |navigation| end n.item :moderation, safe_join([fa_icon('gavel fw'), t('moderation.title')]), nil, if: -> { current_user.can?(:manage_reports, :view_audit_log, :manage_users, :manage_invites, :manage_taxonomies, :manage_federation, :manage_blocks) && !self_destruct } do |s| - s.item :reports, safe_join([fa_icon('flag fw'), t('admin.reports.title')]), admin_reports_path, highlights_on: %r{/admin/reports}, if: -> { current_user.can?(:manage_reports) } - s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_path(origin: 'local'), highlights_on: %r{/admin/accounts|/admin/pending_accounts|/admin/disputes|/admin/users}, if: -> { current_user.can?(:manage_users) } + s.item :reports, safe_join([fa_icon('flag fw'), t('admin.reports.title')]), admin_reports_path, highlights_on: %r{/admin/reports|admin/report_notes}, if: -> { current_user.can?(:manage_reports) } + s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_path(origin: 'local'), highlights_on: %r{/admin/accounts|admin/account_moderation_notes|/admin/pending_accounts|/admin/disputes|/admin/users}, if: -> { current_user.can?(:manage_users) } s.item :invites, safe_join([fa_icon('user-plus fw'), t('admin.invites.title')]), admin_invites_path, if: -> { current_user.can?(:manage_invites) } s.item :follow_recommendations, safe_join([fa_icon('user-plus fw'), t('admin.follow_recommendations.title')]), admin_follow_recommendations_path, highlights_on: %r{/admin/follow_recommendations}, if: -> { current_user.can?(:manage_taxonomies) } s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_path(limited: limited_federation_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.can?(:manage_federation) } diff --git a/spec/controllers/admin/account_moderation_notes_controller_spec.rb b/spec/controllers/admin/account_moderation_notes_controller_spec.rb index 8d24a7af373..5ea546f4180 100644 --- a/spec/controllers/admin/account_moderation_notes_controller_spec.rb +++ b/spec/controllers/admin/account_moderation_notes_controller_spec.rb @@ -24,10 +24,19 @@ RSpec.describe Admin::AccountModerationNotesController do end end - context 'when parameters are invalid' do + context 'when the content is too short' do let(:params) { { account_moderation_note: { target_account_id: target_account.id, content: '' } } } - it 'falls to create a note' do + it 'fails to create a note' do + expect { subject }.to_not change(AccountModerationNote, :count) + expect(response).to render_template 'admin/accounts/show' + end + end + + context 'when the content is too long' do + let(:params) { { account_moderation_note: { target_account_id: target_account.id, content: 'test' * AccountModerationNote::CONTENT_SIZE_LIMIT } } } + + it 'fails to create a note' do expect { subject }.to_not change(AccountModerationNote, :count) expect(response).to render_template 'admin/accounts/show' end diff --git a/spec/controllers/admin/report_notes_controller_spec.rb b/spec/controllers/admin/report_notes_controller_spec.rb index 4ddf4a4e247..8d5b5c7aecf 100644 --- a/spec/controllers/admin/report_notes_controller_spec.rb +++ b/spec/controllers/admin/report_notes_controller_spec.rb @@ -22,7 +22,7 @@ describe Admin::ReportNotesController do let(:account_id) { nil } context 'when create_and_resolve flag is on' do - let(:params) { { report_note: { content: 'test content', report_id: report.id }, create_and_resolve: nil } } + let(:params) { { report_note: { report_id: report.id, content: 'test content' }, create_and_resolve: nil } } it 'creates a report note and resolves report' do expect { subject }.to change(ReportNote, :count).by(1) @@ -32,7 +32,7 @@ describe Admin::ReportNotesController do end context 'when create_and_resolve flag is false' do - let(:params) { { report_note: { content: 'test content', report_id: report.id } } } + let(:params) { { report_note: { report_id: report.id, content: 'test content' } } } it 'creates a report note and does not resolve report' do expect { subject }.to change(ReportNote, :count).by(1) @@ -47,7 +47,7 @@ describe Admin::ReportNotesController do let(:account_id) { user.account.id } context 'when create_and_unresolve flag is on' do - let(:params) { { report_note: { content: 'test content', report_id: report.id }, create_and_unresolve: nil } } + let(:params) { { report_note: { report_id: report.id, content: 'test content' }, create_and_unresolve: nil } } it 'creates a report note and unresolves report' do expect { subject }.to change(ReportNote, :count).by(1) @@ -57,7 +57,7 @@ describe Admin::ReportNotesController do end context 'when create_and_unresolve flag is false' do - let(:params) { { report_note: { content: 'test content', report_id: report.id } } } + let(:params) { { report_note: { report_id: report.id, content: 'test content' } } } it 'creates a report note and does not unresolve report' do expect { subject }.to change(ReportNote, :count).by(1) @@ -68,12 +68,24 @@ describe Admin::ReportNotesController do end end - context 'when parameter is invalid' do - let(:params) { { report_note: { content: '', report_id: report.id } } } + context 'when content is too short' do + let(:params) { { report_note: { report_id: report.id, content: '' } } } let(:action_taken) { nil } let(:account_id) { nil } it 'renders admin/reports/show' do + expect { subject }.to_not change(ReportNote, :count) + expect(subject).to render_template 'admin/reports/show' + end + end + + context 'when content is too long' do + let(:params) { { report_note: { report_id: report.id, content: 'test' * ReportNote::CONTENT_SIZE_LIMIT } } } + let(:action_taken) { nil } + let(:account_id) { nil } + + it 'renders admin/reports/show' do + expect { subject }.to_not change(ReportNote, :count) expect(subject).to render_template 'admin/reports/show' end end