Avoid latest featured tag use on post removal unless necessary (#32787)
This commit is contained in:
		
							parent
							
								
									871e3b25e8
								
							
						
					
					
						commit
						2bea74e69d
					
				| @ -44,8 +44,16 @@ class FeaturedTag < ApplicationRecord | |||||||
|     update(statuses_count: statuses_count + 1, last_status_at: timestamp) |     update(statuses_count: statuses_count + 1, last_status_at: timestamp) | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   def decrement(deleted_status_id) |   def decrement(deleted_status) | ||||||
|     update(statuses_count: [0, statuses_count - 1].max, last_status_at: visible_tagged_account_statuses.where.not(id: deleted_status_id).pick(:created_at)) |     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 |   end | ||||||
| 
 | 
 | ||||||
|   private |   private | ||||||
|  | |||||||
| @ -33,7 +33,7 @@ class ProcessHashtagsService < BaseService | |||||||
| 
 | 
 | ||||||
|     unless removed_tags.empty? |     unless removed_tags.empty? | ||||||
|       @account.featured_tags.where(tag_id: removed_tags.map(&:id)).find_each do |featured_tag| |       @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 |     end | ||||||
|   end |   end | ||||||
|  | |||||||
| @ -115,7 +115,7 @@ class RemoveStatusService < BaseService | |||||||
| 
 | 
 | ||||||
|   def remove_from_hashtags |   def remove_from_hashtags | ||||||
|     @account.featured_tags.where(tag_id: @status.tags.map(&:id)).find_each do |featured_tag| |     @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 |     end | ||||||
| 
 | 
 | ||||||
|     return unless @status.public_visibility? |     return unless @status.public_visibility? | ||||||
|  | |||||||
| @ -126,16 +126,54 @@ RSpec.describe FeaturedTag do | |||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   describe '#decrement' do |   describe '#decrement' do | ||||||
|     it 'decreases the count and updates the last_status_at timestamp' do |     let(:tag) { Fabricate(:tag, name: 'test') } | ||||||
|       tag = Fabricate :tag, name: 'test' |     let(:account) { Fabricate(:account) } | ||||||
|       status = Fabricate :status, visibility: :public, created_at: 10.days.ago |     let(:featured_tag) { Fabricate(:featured_tag, name: 'test', account: account) } | ||||||
|  | 
 | ||||||
|  |     context 'when removing the last status using the tag' do | ||||||
|  |       let(:status) { Fabricate(:status, visibility: :public, account: account, created_at: 10.days.ago) } | ||||||
|  | 
 | ||||||
|  |       before do | ||||||
|         status.tags << tag |         status.tags << tag | ||||||
|  |       end | ||||||
| 
 | 
 | ||||||
|       featured_tag = Fabricate :featured_tag, name: 'test', account: status.account |       it 'decreases the count and updates the last_status_at timestamp' do | ||||||
| 
 |         expect { featured_tag.decrement(status) } | ||||||
|       expect { featured_tag.decrement(status.id) } |  | ||||||
|           .to change(featured_tag, :statuses_count).from(1).to(0) |           .to change(featured_tag, :statuses_count).from(1).to(0) | ||||||
|           .and change(featured_tag, :last_status_at).to(nil) |           .and change(featured_tag, :last_status_at).to(nil) | ||||||
|       end |       end | ||||||
|     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 | end | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user