diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index a4e7b7cf6f..529056f9c6 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -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 diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index 17c347b088..0baea0185c 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -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 diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 221791ad32..dc9fb6cab6 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -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? diff --git a/spec/models/featured_tag_spec.rb b/spec/models/featured_tag_spec.rb index 0f5ead8f97..20059cfba4 100644 --- a/spec/models/featured_tag_spec.rb +++ b/spec/models/featured_tag_spec.rb @@ -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