vbatts/mastodon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix AccountNote not having a maximum length (#16942)

Browse source
This commit is contained in:
Claire 2021-11-06 00:12:25 +01:00 committed by Eugen Rochko
parent 3251b8eead
commit 76c2028859
4 changed files with 86 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
app/models/account_note.rb
View file

@ -17,4 +17,5 @@ class AccountNote < ApplicationRecord
belongs_to :target_account, class_name: 'Account'
validates :account_id, uniqueness: { scope: :target_account_id }
validates :comment, length: { maximum: 2_000 }
end

8
app/workers/move_worker.rb
View file

@ -53,10 +53,16 @@ class MoveWorker
new_note = AccountNote.find_by(account: note.account, target_account: @target_account)
if new_note.nil?
AccountNote.create!(account: note.account, target_account: @target_account, comment: [text, note.comment].join("\n"))
begin
AccountNote.create!(account: note.account, target_account: @target_account, comment: [text, note.comment].join("\n"))
rescue ActiveRecord::RecordInvalid
AccountNote.create!(account: note.account, target_account: @target_account, comment: note.comment)
end
else
new_note.update!(comment: [text, note.comment, "\n", new_note.comment].join("\n"))
end
rescue ActiveRecord::RecordInvalid
nil
rescue => e
@deferred_error = e
end

48
spec/controllers/api/v1/accounts/notes_controller_spec.rb Normal file
View file

@ -0,0 +1,48 @@
require 'rails_helper'
describe Api::V1::Accounts::NotesController do
render_views
let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'write:accounts') }
let(:account) { Fabricate(:account) }
let(:comment) { 'foo' }
before do
allow(controller).to receive(:doorkeeper_token) { token }
end
describe 'POST #create' do
subject do
post :create, params: { account_id: account.id, comment: comment }
end
context 'when account note has reasonable length' do
let(:comment) { 'foo' }
it 'returns http success' do
subject
expect(response).to have_http_status(200)
end
it 'updates account note' do
subject
expect(AccountNote.find_by(account_id: user.account.id, target_account_id: account.id).comment).to eq comment
end
end
context 'when account note exceends allowed length' do
let(:comment) { 'a' * 2_001 }
it 'returns 422' do
subject
expect(response).to have_http_status(422)
end
it 'does not create account note' do
subject
expect(AccountNote.where(account_id: user.account.id, target_account_id: account.id).exists?).to be_falsey
end
end
end
end

41
spec/workers/move_worker_spec.rb
View file

@ -9,7 +9,8 @@ describe MoveWorker do
let(:source_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') }
let(:target_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') }
let(:local_user) { Fabricate(:user) }
let!(:account_note) { Fabricate(:account_note, account: local_user.account, target_account: source_account) }
let(:comment) { 'old note prior to move' }
let!(:account_note) { Fabricate(:account_note, account: local_user.account, target_account: source_account, comment: comment) }
let(:block_service) { double }
@ -26,19 +27,37 @@ describe MoveWorker do
end
shared_examples 'user note handling' do
it 'copies user note' do
subject.perform(source_account.id, target_account.id)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment)
context 'when user notes are short enough' do
it 'copies user note with prelude' do
subject.perform(source_account.id, target_account.id)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment)
end
it 'merges user notes when needed' do
new_account_note = AccountNote.create!(account: account_note.account, target_account: target_account, comment: 'new note prior to move')
subject.perform(source_account.id, target_account.id)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(new_account_note.comment)
end
end
it 'merges user notes when needed' do
new_account_note = AccountNote.create!(account: account_note.account, target_account: target_account, comment: 'new note prior to move')
context 'when user notes are too long' do
let(:comment) { 'abc' * 333 }
subject.perform(source_account.id, target_account.id)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(new_account_note.comment)
it 'copies user note without prelude' do
subject.perform(source_account.id, target_account.id)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment)
end
it 'keeps user notes unchanged' do
new_account_note = AccountNote.create!(account: account_note.account, target_account: target_account, comment: 'new note prior to move')
subject.perform(source_account.id, target_account.id)
expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(new_account_note.comment)
end
end
end