Fix last_status_at not accurately tracking last status time for remote accounts (#37619)
This commit is contained in:
parent
a28ccf2950
commit
9ea64fcb5a
@ -20,8 +20,8 @@ module Account::Counters
|
|||||||
to: :account_stat
|
to: :account_stat
|
||||||
|
|
||||||
# @param [Symbol] key
|
# @param [Symbol] key
|
||||||
def increment_count!(key)
|
def increment_count!(key, status_created_at: nil)
|
||||||
update_count!(key, 1)
|
update_count!(key, 1, status_created_at:)
|
||||||
end
|
end
|
||||||
|
|
||||||
# @param [Symbol] key
|
# @param [Symbol] key
|
||||||
@ -31,11 +31,11 @@ module Account::Counters
|
|||||||
|
|
||||||
# @param [Symbol] key
|
# @param [Symbol] key
|
||||||
# @param [Integer] value
|
# @param [Integer] value
|
||||||
def update_count!(key, value)
|
def update_count!(key, value, status_created_at: nil)
|
||||||
raise ArgumentError, "Invalid key #{key}" unless ALLOWED_COUNTER_KEYS.include?(key)
|
raise ArgumentError, "Invalid key #{key}" unless ALLOWED_COUNTER_KEYS.include?(key)
|
||||||
raise ArgumentError, 'Do not call update_count! on dirty objects' if association(:account_stat).loaded? && account_stat&.changed? && account_stat.changed_attribute_names_to_save == %w(id)
|
raise ArgumentError, 'Do not call update_count! on dirty objects' if association(:account_stat).loaded? && account_stat&.changed? && account_stat.changed_attribute_names_to_save == %w(id)
|
||||||
|
|
||||||
result = updated_account_stat(key, value.to_i)
|
result = updated_account_stat(key, value.to_i, status_created_at:)
|
||||||
|
|
||||||
# Reload account_stat if it was loaded, taking into account newly-created unsaved records
|
# Reload account_stat if it was loaded, taking into account newly-created unsaved records
|
||||||
if association(:account_stat).loaded?
|
if association(:account_stat).loaded?
|
||||||
@ -50,25 +50,27 @@ module Account::Counters
|
|||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def updated_account_stat(key, value)
|
def updated_account_stat(key, value, status_created_at: nil)
|
||||||
|
status_created_at = Time.now.utc if status_created_at.nil? || status_created_at > Time.now.utc
|
||||||
|
|
||||||
AccountStat.upsert(
|
AccountStat.upsert(
|
||||||
initial_values(key, value),
|
initial_values(key, value, status_created_at:),
|
||||||
on_duplicate: Arel.sql(
|
on_duplicate: Arel.sql(
|
||||||
duplicate_values(key, value).join(', ')
|
duplicate_values(key, value, status_created_at:).join(', ')
|
||||||
),
|
),
|
||||||
unique_by: :account_id
|
unique_by: :account_id
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
def initial_values(key, value)
|
def initial_values(key, value, status_created_at: nil)
|
||||||
{ :account_id => id, key => [value, 0].max }.tap do |values|
|
{ :account_id => id, key => [value, 0].max }.tap do |values|
|
||||||
values.merge!(last_status_at: Time.current) if key == :statuses_count
|
values.merge!(last_status_at: status_created_at) if key == :statuses_count
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def duplicate_values(key, value)
|
def duplicate_values(key, value, status_created_at: nil)
|
||||||
["#{key} = (account_stats.#{key} + #{value})", 'updated_at = CURRENT_TIMESTAMP'].tap do |values|
|
["#{key} = (account_stats.#{key} + #{value})", 'updated_at = CURRENT_TIMESTAMP'].tap do |values|
|
||||||
values << 'last_status_at = CURRENT_TIMESTAMP' if key == :statuses_count && value.positive?
|
values << AccountStat.sanitize_sql_array(['last_status_at = GREATEST(account_stats.last_status_at, ?::timestamp)', status_created_at]) if key == :statuses_count && value.positive?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@ -481,7 +481,7 @@ class Status < ApplicationRecord
|
|||||||
def increment_counter_caches
|
def increment_counter_caches
|
||||||
return if direct_visibility?
|
return if direct_visibility?
|
||||||
|
|
||||||
account&.increment_count!(:statuses_count)
|
account&.increment_count!(:statuses_count, status_created_at: created_at)
|
||||||
reblog&.increment_count!(:reblogs_count) if reblog?
|
reblog&.increment_count!(:reblogs_count) if reblog?
|
||||||
thread&.increment_count!(:replies_count) if in_reply_to_id.present? && distributable?
|
thread&.increment_count!(:replies_count) if in_reply_to_id.present? && distributable?
|
||||||
end
|
end
|
||||||
|
|||||||
@ -21,6 +21,28 @@ RSpec.describe Account::Counters do
|
|||||||
|
|
||||||
expect(account.statuses_count).to eq increment_by
|
expect(account.statuses_count).to eq increment_by
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'updates last_status_at when discovering a new post' do
|
||||||
|
status_created_at = Time.now.utc
|
||||||
|
|
||||||
|
expect { account.increment_count!(:statuses_count, status_created_at:) }
|
||||||
|
.to(change { account.reload.last_status_at })
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not update last_status_at when discovering an older post' do
|
||||||
|
account_stat = Fabricate(
|
||||||
|
:account_stat,
|
||||||
|
account: account,
|
||||||
|
last_status_at: 1.day.ago.utc,
|
||||||
|
statuses_count: 10
|
||||||
|
)
|
||||||
|
|
||||||
|
status_created_at = 2.days.ago.utc
|
||||||
|
|
||||||
|
expect { account.increment_count!(:statuses_count, status_created_at:) }
|
||||||
|
.to change { account_stat.reload.statuses_count }
|
||||||
|
.and(not_change { account_stat.reload.last_status_at })
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#decrement_count!' do
|
describe '#decrement_count!' do
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user