Avoid latest featured tag use on post removal unless necessary (#32787)
This commit is contained in:
parent
c634da32cf
commit
dc9a106d4c
4 changed files with 58 additions and 12 deletions
|
@ -44,8 +44,16 @@ class FeaturedTag < ApplicationRecord
|
|||
update(statuses_count: statuses_count + 1, last_status_at: timestamp)
|
||||
end
|
||||
|
||||
def decrement(deleted_status_id)
|
||||
update(statuses_count: [0, statuses_count - 1].max, last_status_at: visible_tagged_account_statuses.where.not(id: deleted_status_id).pick(:created_at))
|
||||
def decrement(deleted_status)
|
||||
if statuses_count <= 1
|
||||
update(statuses_count: 0, last_status_at: nil)
|
||||
elsif last_status_at > deleted_status.created_at
|
||||
update(statuses_count: statuses_count - 1)
|
||||
else
|
||||
# Fetching the latest status creation time can be expensive, so only perform it
|
||||
# if we know we are deleting the latest status using this tag
|
||||
update(statuses_count: statuses_count - 1, last_status_at: visible_tagged_account_statuses.where(id: ...deleted_status.id).pick(:created_at))
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -33,7 +33,7 @@ class ProcessHashtagsService < BaseService
|
|||
|
||||
unless removed_tags.empty?
|
||||
@account.featured_tags.where(tag_id: removed_tags.map(&:id)).find_each do |featured_tag|
|
||||
featured_tag.decrement(@status.id)
|
||||
featured_tag.decrement(@status)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -115,7 +115,7 @@ class RemoveStatusService < BaseService
|
|||
|
||||
def remove_from_hashtags
|
||||
@account.featured_tags.where(tag_id: @status.tags.map(&:id)).find_each do |featured_tag|
|
||||
featured_tag.decrement(@status.id)
|
||||
featured_tag.decrement(@status)
|
||||
end
|
||||
|
||||
return unless @status.public_visibility?
|
||||
|
|
|
@ -126,16 +126,54 @@ RSpec.describe FeaturedTag do
|
|||
end
|
||||
|
||||
describe '#decrement' do
|
||||
it 'decreases the count and updates the last_status_at timestamp' do
|
||||
tag = Fabricate :tag, name: 'test'
|
||||
status = Fabricate :status, visibility: :public, created_at: 10.days.ago
|
||||
status.tags << tag
|
||||
let(:tag) { Fabricate(:tag, name: 'test') }
|
||||
let(:account) { Fabricate(:account) }
|
||||
let(:featured_tag) { Fabricate(:featured_tag, name: 'test', account: account) }
|
||||
|
||||
featured_tag = Fabricate :featured_tag, name: 'test', account: status.account
|
||||
context 'when removing the last status using the tag' do
|
||||
let(:status) { Fabricate(:status, visibility: :public, account: account, created_at: 10.days.ago) }
|
||||
|
||||
expect { featured_tag.decrement(status.id) }
|
||||
.to change(featured_tag, :statuses_count).from(1).to(0)
|
||||
.and change(featured_tag, :last_status_at).to(nil)
|
||||
before do
|
||||
status.tags << tag
|
||||
end
|
||||
|
||||
it 'decreases the count and updates the last_status_at timestamp' do
|
||||
expect { featured_tag.decrement(status) }
|
||||
.to change(featured_tag, :statuses_count).from(1).to(0)
|
||||
.and change(featured_tag, :last_status_at).to(nil)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when removing a previous status using the tag' do
|
||||
let(:previous_status) { Fabricate(:status, visibility: :public, account: account, created_at: 1.month.ago) }
|
||||
let(:status) { Fabricate(:status, visibility: :public, account: account, created_at: 10.days.ago) }
|
||||
|
||||
before do
|
||||
previous_status.tags << tag
|
||||
status.tags << tag
|
||||
end
|
||||
|
||||
it 'decreases the count and updates the last_status_at timestamp' do
|
||||
expect { featured_tag.decrement(previous_status) }
|
||||
.to change(featured_tag, :statuses_count).from(2).to(1)
|
||||
.and not_change(featured_tag, :last_status_at)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when removing the most recent use of the tag' do
|
||||
let(:previous_status) { Fabricate(:status, visibility: :public, account: account, created_at: 1.month.ago) }
|
||||
let(:status) { Fabricate(:status, visibility: :public, account: account, created_at: 10.days.ago) }
|
||||
|
||||
before do
|
||||
previous_status.tags << tag
|
||||
status.tags << tag
|
||||
end
|
||||
|
||||
it 'decreases the count and updates the last_status_at timestamp' do
|
||||
expect { featured_tag.decrement(status) }
|
||||
.to change(featured_tag, :statuses_count).from(2).to(1)
|
||||
.and change(featured_tag, :last_status_at)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue