From 2bea74e69d43f15aff646de5948cfbb4b9296145 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Wed, 6 Nov 2024 11:51:54 +0100
Subject: [PATCH] Avoid latest featured tag use on post removal unless
 necessary (#32787)

---
 app/models/featured_tag.rb               | 12 +++++-
 app/services/process_hashtags_service.rb |  2 +-
 app/services/remove_status_service.rb    |  2 +-
 spec/models/featured_tag_spec.rb         | 54 ++++++++++++++++++++----
 4 files changed, 58 insertions(+), 12 deletions(-)

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