From e63370db191eb9ae12112789afe8e027095f9112 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 14 Oct 2021 19:59:21 +0200 Subject: [PATCH] Fix scheduled statuses decreasing statuses counts (#16791) * Add tests * Fix scheduled statuses decreasing statuses counts Fixes #16774 --- app/models/status.rb | 2 +- app/services/post_status_service.rb | 3 ++ spec/services/post_status_service_spec.rb | 42 +++++++++++++---------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 847921ac233..3acf759f9de 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -426,7 +426,7 @@ class Status < ApplicationRecord end def decrement_counter_caches - return if direct_visibility? + return if direct_visibility? || new_record? account&.decrement_count!(:statuses_count) reblog&.decrement_count!(:reblogs_count) if reblog? diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 0a383d6a392..85aaec4d658 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -74,6 +74,9 @@ class PostStatusService < BaseService status_for_validation = @account.statuses.build(status_attributes) if status_for_validation.valid? + # Marking the status as destroyed is necessary to prevent the status from being + # persisted when the associated media attachments get updated when creating the + # scheduled status. status_for_validation.destroy # The following transaction block is needed to wrap the UPDATEs to diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 147a59fc315..d21270c7938 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -25,29 +25,33 @@ RSpec.describe PostStatusService, type: :service do expect(status.thread).to eq in_reply_to_status end - it 'schedules a status' do - account = Fabricate(:account) - future = Time.now.utc + 2.hours + context 'when scheduling a status' do + let!(:account) { Fabricate(:account) } + let!(:future) { Time.now.utc + 2.hours } + let!(:previous_status) { Fabricate(:status, account: account) } - status = subject.call(account, text: 'Hi future!', scheduled_at: future) + it 'schedules a status' do + status = subject.call(account, text: 'Hi future!', scheduled_at: future) + expect(status).to be_a ScheduledStatus + expect(status.scheduled_at).to eq future + expect(status.params['text']).to eq 'Hi future!' + end - expect(status).to be_a ScheduledStatus - expect(status.scheduled_at).to eq future - expect(status.params['text']).to eq 'Hi future!' - end + it 'does not immediately create a status' do + media = Fabricate(:media_attachment, account: account) + status = subject.call(account, text: 'Hi future!', media_ids: [media.id], scheduled_at: future) - it 'does not immediately create a status when scheduling a status' do - account = Fabricate(:account) - media = Fabricate(:media_attachment) - future = Time.now.utc + 2.hours + expect(status).to be_a ScheduledStatus + expect(status.scheduled_at).to eq future + expect(status.params['text']).to eq 'Hi future!' + expect(status.params['media_ids']).to eq [media.id] + expect(media.reload.status).to be_nil + expect(Status.where(text: 'Hi future!').exists?).to be_falsey + end - status = subject.call(account, text: 'Hi future!', media_ids: [media.id], scheduled_at: future) - - expect(status).to be_a ScheduledStatus - expect(status.scheduled_at).to eq future - expect(status.params['text']).to eq 'Hi future!' - expect(media.reload.status).to be_nil - expect(Status.where(text: 'Hi future!').exists?).to be_falsey + it 'does not change statuses count' do + expect { subject.call(account, text: 'Hi future!', scheduled_at: future, thread: previous_status) }.not_to change { [account.statuses_count, previous_status.replies_count] } + end end it 'creates response to the original status of boost' do