Fix scheduled statuses decreasing statuses counts (#16791)

* Add tests

* Fix scheduled statuses decreasing statuses counts

Fixes #16774

diff --git a/app/models/status.rb b/app/models/status.rb
index 847921ac233dfbd13f54b11b5fb6f685471d6e57..3acf759f9de0a6d11453bbedd7c8357384607fce 100644 (file)
--- 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 0a383d6a392afefc326dd28c2651bea3a517a5ce..85aaec4d6585d9a2bb1ce43c279d62d3a80b464c 100644 (file)
--- 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 147a59fc315c7c81c702315b288b0059369ed0fc..d21270c793894687418d01dbc96e9ac7b7381332 100644 (file)
--- 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