Use Rails upsert to generate update_count! query in Counters concern (#28738)

Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
Matt Jankowski 2024-04-17 04:16:51 -04:00 committed by GitHub
parent 5915bd7f45
commit 6fed108703
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 37 additions and 31 deletions

View file

@ -35,40 +35,11 @@ module Account::Counters
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)
value = value.to_i result = updated_account_stat(key, value.to_i)
default_value = value.positive? ? value : 0
# We do an upsert using manually written SQL, as Rails' upsert method does
# not seem to support writing expressions in the UPDATE clause, but only
# re-insert the provided values instead.
# Even ARel seem to be missing proper handling of upserts.
sql = if value.positive? && key == :statuses_count
<<-SQL.squish
INSERT INTO account_stats(account_id, #{key}, created_at, updated_at, last_status_at)
VALUES (:account_id, :default_value, now(), now(), now())
ON CONFLICT (account_id) DO UPDATE
SET #{key} = account_stats.#{key} + :value,
last_status_at = now(),
updated_at = now()
RETURNING id;
SQL
else
<<-SQL.squish
INSERT INTO account_stats(account_id, #{key}, created_at, updated_at)
VALUES (:account_id, :default_value, now(), now())
ON CONFLICT (account_id) DO UPDATE
SET #{key} = account_stats.#{key} + :value,
updated_at = now()
RETURNING id;
SQL
end
sql = AccountStat.sanitize_sql([sql, account_id: id, default_value: default_value, value: value])
account_stat_id = AccountStat.connection.exec_query(sql)[0]['id']
# 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?
account_stat.id = account_stat_id if account_stat.new_record? account_stat.id = result.first['id'] if account_stat.new_record?
account_stat.reload account_stat.reload
end end
end end
@ -79,6 +50,28 @@ module Account::Counters
private private
def updated_account_stat(key, value)
AccountStat.upsert(
initial_values(key, value),
on_duplicate: Arel.sql(
duplicate_values(key, value).join(', ')
),
unique_by: :account_id
)
end
def initial_values(key, value)
{ :account_id => id, key => [value, 0].max }.tap do |values|
values.merge!(last_status_at: Time.current) if key == :statuses_count
end
end
def duplicate_values(key, value)
["#{key} = (account_stats.#{key} + #{value})", 'updated_at = CURRENT_TIMESTAMP'].tap do |values|
values << 'last_status_at = CURRENT_TIMESTAMP' if key == :statuses_count && value.positive?
end
end
def save_account_stat def save_account_stat
return unless association(:account_stat).loaded? && account_stat&.changed? return unless association(:account_stat).loaded? && account_stat&.changed?

View file

@ -44,5 +44,18 @@ describe Account::Counters do
expect(account.statuses_count).to eq 5 expect(account.statuses_count).to eq 5
end end
it 'preserves last_status_at when decrementing statuses_count' do
account_stat = Fabricate(
:account_stat,
account: account,
last_status_at: 3.days.ago,
statuses_count: 10
)
expect { account.decrement_count!(:statuses_count) }
.to change(account_stat.reload, :statuses_count).by(-1)
.and not_change(account_stat.reload, :last_status_at)
end
end end
end end