Browse Source

Fix scheduled statuses decreasing statuses counts (#16791)

* Add tests

* Fix scheduled statuses decreasing statuses counts

Fixes #16774
Claire 2 years ago
parent
commit
e63370db19

+ 1 - 1
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?

+ 3 - 0
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

+ 27 - 23
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