From c88ba523ee6eebb11413690639818ef1c926399c Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Fri, 6 Sep 2024 16:58:36 +0200 Subject: [PATCH] Fix sort order of moderation notes on Reports and Accounts (#31528) --- .../account_moderation_notes_controller.rb | 2 +- app/controllers/admin/accounts_controller.rb | 2 +- .../admin/report_notes_controller.rb | 2 +- app/controllers/admin/reports_controller.rb | 2 +- app/models/account_moderation_note.rb | 2 +- app/models/report_note.rb | 2 +- .../admin/accounts_controller_spec.rb | 17 ++++++++++ .../admin/reports_controller_spec.rb | 18 +++++++++++ .../account_moderation_note_fabricator.rb | 2 +- spec/fabricators/report_note_fabricator.rb | 2 +- spec/models/account_moderation_note_spec.rb | 31 +++++++++++++++++++ spec/models/report_note_spec.rb | 31 +++++++++++++++++++ 12 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 spec/models/account_moderation_note_spec.rb create mode 100644 spec/models/report_note_spec.rb diff --git a/app/controllers/admin/account_moderation_notes_controller.rb b/app/controllers/admin/account_moderation_notes_controller.rb index 8b6c1a4454..a3c4adf59a 100644 --- a/app/controllers/admin/account_moderation_notes_controller.rb +++ b/app/controllers/admin/account_moderation_notes_controller.rb @@ -13,7 +13,7 @@ module Admin redirect_to admin_account_path(@account_moderation_note.target_account_id), notice: I18n.t('admin.account_moderation_notes.created_msg') else @account = @account_moderation_note.target_account - @moderation_notes = @account.targeted_moderation_notes.latest + @moderation_notes = @account.targeted_moderation_notes.chronological.includes(:account) @warnings = @account.strikes.custom.latest render 'admin/accounts/show' diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 9beb8fde6b..7b169ba26a 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -33,7 +33,7 @@ module Admin @deletion_request = @account.deletion_request @account_moderation_note = current_account.account_moderation_notes.new(target_account: @account) - @moderation_notes = @account.targeted_moderation_notes.latest + @moderation_notes = @account.targeted_moderation_notes.chronological.includes(:account) @warnings = @account.strikes.includes(:target_account, :account, :appeal).latest @domain_block = DomainBlock.rule_for(@account.domain) end diff --git a/app/controllers/admin/report_notes_controller.rb b/app/controllers/admin/report_notes_controller.rb index b5f04a1caa..6b16c29fc7 100644 --- a/app/controllers/admin/report_notes_controller.rb +++ b/app/controllers/admin/report_notes_controller.rb @@ -21,7 +21,7 @@ module Admin redirect_to after_create_redirect_path, notice: I18n.t('admin.report_notes.created_msg') else - @report_notes = @report.notes.includes(:account).order(id: :desc) + @report_notes = @report.notes.chronological.includes(:account) @action_logs = @report.history.includes(:target) @form = Admin::StatusBatchAction.new @statuses = @report.statuses.with_includes diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 00d200d7c8..aa877f1448 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -13,7 +13,7 @@ module Admin authorize @report, :show? @report_note = @report.notes.new - @report_notes = @report.notes.includes(:account).order(id: :desc) + @report_notes = @report.notes.chronological.includes(:account) @action_logs = @report.history.includes(:target) @form = Admin::StatusBatchAction.new @statuses = @report.statuses.with_includes diff --git a/app/models/account_moderation_note.rb b/app/models/account_moderation_note.rb index 79b8b4d25e..ca7f8e3d5f 100644 --- a/app/models/account_moderation_note.rb +++ b/app/models/account_moderation_note.rb @@ -18,7 +18,7 @@ class AccountModerationNote < ApplicationRecord belongs_to :account belongs_to :target_account, class_name: 'Account' - scope :latest, -> { reorder('created_at DESC') } + scope :chronological, -> { reorder(id: :asc) } validates :content, presence: true, length: { maximum: CONTENT_SIZE_LIMIT } end diff --git a/app/models/report_note.rb b/app/models/report_note.rb index 7361c97e67..9d3be52594 100644 --- a/app/models/report_note.rb +++ b/app/models/report_note.rb @@ -18,7 +18,7 @@ class ReportNote < ApplicationRecord belongs_to :account belongs_to :report, inverse_of: :notes, touch: true - scope :latest, -> { reorder(created_at: :desc) } + scope :chronological, -> { reorder(id: :asc) } validates :content, presence: true, length: { maximum: CONTENT_SIZE_LIMIT } end diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index ca399fbd9b..a182300106 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -55,6 +55,23 @@ RSpec.describe Admin::AccountsController do describe 'GET #show' do let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } + describe 'account moderation notes' do + let(:account) { Fabricate(:account) } + + it 'includes moderation notes' do + note1 = Fabricate(:account_moderation_note, target_account: account) + note2 = Fabricate(:account_moderation_note, target_account: account) + + get :show, params: { id: account.id } + expect(response).to have_http_status(200) + + moderation_notes = assigns(:moderation_notes).to_a + + expect(moderation_notes.size).to be 2 + expect(moderation_notes).to eq [note1, note2] + end + end + context 'with a remote account' do let(:account) { Fabricate(:account, domain: 'example.com') } diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index d07468a37b..1252ceb1f4 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -47,6 +47,24 @@ RSpec.describe Admin::ReportsController do expect(response.body) .to include(report.comment) end + + describe 'account moderation notes' do + let(:report) { Fabricate(:report) } + + it 'includes moderation notes' do + note1 = Fabricate(:report_note, report: report) + note2 = Fabricate(:report_note, report: report) + + get :show, params: { id: report } + + expect(response).to have_http_status(200) + + report_notes = assigns(:report_notes).to_a + + expect(report_notes.size).to be 2 + expect(report_notes).to eq [note1, note2] + end + end end describe 'POST #resolve' do diff --git a/spec/fabricators/account_moderation_note_fabricator.rb b/spec/fabricators/account_moderation_note_fabricator.rb index 05a687bf4e..1ded862638 100644 --- a/spec/fabricators/account_moderation_note_fabricator.rb +++ b/spec/fabricators/account_moderation_note_fabricator.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true Fabricator(:account_moderation_note) do - content 'MyText' + content { Faker::Lorem.sentences } account { Fabricate.build(:account) } target_account { Fabricate.build(:account) } end diff --git a/spec/fabricators/report_note_fabricator.rb b/spec/fabricators/report_note_fabricator.rb index 080fad51ac..a5e9cc9009 100644 --- a/spec/fabricators/report_note_fabricator.rb +++ b/spec/fabricators/report_note_fabricator.rb @@ -3,5 +3,5 @@ Fabricator(:report_note) do report { Fabricate.build(:report) } account { Fabricate.build(:account) } - content 'Test Content' + content { Faker::Lorem.sentences } end diff --git a/spec/models/account_moderation_note_spec.rb b/spec/models/account_moderation_note_spec.rb new file mode 100644 index 0000000000..079774c492 --- /dev/null +++ b/spec/models/account_moderation_note_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AccountModerationNote do + describe 'chronological scope' do + it 'returns account moderation notes oldest to newest' do + account = Fabricate(:account) + note1 = Fabricate(:account_moderation_note, target_account: account) + note2 = Fabricate(:account_moderation_note, target_account: account) + + expect(account.targeted_moderation_notes.chronological).to eq [note1, note2] + end + end + + describe 'validations' do + it 'is invalid if the content is empty' do + report = Fabricate.build(:account_moderation_note, content: '') + expect(report.valid?).to be false + end + + it 'is invalid if content is longer than character limit' do + report = Fabricate.build(:account_moderation_note, content: comment_over_limit) + expect(report.valid?).to be false + end + + def comment_over_limit + Faker::Lorem.paragraph_by_chars(number: described_class::CONTENT_SIZE_LIMIT * 2) + end + end +end diff --git a/spec/models/report_note_spec.rb b/spec/models/report_note_spec.rb new file mode 100644 index 0000000000..417971c9a1 --- /dev/null +++ b/spec/models/report_note_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ReportNote do + describe 'chronological scope' do + it 'returns report notes oldest to newest' do + report = Fabricate(:report) + note1 = Fabricate(:report_note, report: report) + note2 = Fabricate(:report_note, report: report) + + expect(report.notes.chronological).to eq [note1, note2] + end + end + + describe 'validations' do + it 'is invalid if the content is empty' do + report = Fabricate.build(:report_note, content: '') + expect(report.valid?).to be false + end + + it 'is invalid if content is longer than character limit' do + report = Fabricate.build(:report_note, content: comment_over_limit) + expect(report.valid?).to be false + end + + def comment_over_limit + Faker::Lorem.paragraph_by_chars(number: described_class::CONTENT_SIZE_LIMIT * 2) + end + end +end