From 216570ad988108c69b7105740ab382c544c2b034 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
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 | 50 ++++++++++++-----------
3 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/app/models/status.rb b/app/models/status.rb
index 847921ac23..3acf759f9d 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 0a383d6a39..85aaec4d65 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 147a59fc31..d21270c793 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
-
- 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
-
- 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
-
- 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
+ context 'when scheduling a status' do
+ let!(:account) { Fabricate(:account) }
+ let!(:future) { Time.now.utc + 2.hours }
+ let!(:previous_status) { Fabricate(:status, account: account) }
+
+ 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
+
+ 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)
+
+ 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
+
+ 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
--
GitLab