diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index 0baea0185ce..828c2daf794 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -21,20 +21,32 @@ class ProcessHashtagsService < BaseService def update_featured_tags! return unless @status.distributable? - added_tags = @current_tags - @previous_tags + process_added_tags! unless added_tags.empty? - unless added_tags.empty? - @account.featured_tags.where(tag_id: added_tags.map(&:id)).find_each do |featured_tag| - featured_tag.increment(@status.created_at) - end - end + process_removed_tags! unless removed_tags.empty? + end - removed_tags = @previous_tags - @current_tags - - unless removed_tags.empty? - @account.featured_tags.where(tag_id: removed_tags.map(&:id)).find_each do |featured_tag| - featured_tag.decrement(@status) - end + def process_added_tags! + featured_tags_on(added_tags).find_each do |featured_tag| + featured_tag.increment(@status.created_at) end end + + def process_removed_tags! + featured_tags_on(removed_tags).find_each do |featured_tag| + featured_tag.decrement(@status) + end + end + + def featured_tags_on(tags) + @account.featured_tags.where(tag_id: tags.map(&:id)) + end + + def added_tags + @current_tags - @previous_tags + end + + def removed_tags + @previous_tags - @current_tags + end end diff --git a/spec/services/process_hashtags_service_spec.rb b/spec/services/process_hashtags_service_spec.rb index a0d5ef3464d..f3005a6b62b 100644 --- a/spec/services/process_hashtags_service_spec.rb +++ b/spec/services/process_hashtags_service_spec.rb @@ -4,13 +4,40 @@ require 'rails_helper' RSpec.describe ProcessHashtagsService do describe '#call' do - let(:status) { Fabricate(:status, visibility: :public, text: 'With tags #one #two') } + let!(:prior_tag) { Fabricate :tag, name: 'priortag' } + let!(:featured_tag) { Fabricate :featured_tag, account: status.account, tag: prior_tag } - it 'applies the tags from the status text' do - expect { subject.call(status) } - .to change(Tag, :count).by(2) - expect(status.reload.tags.map(&:name)) - .to contain_exactly('one', 'two') + context 'when status is distributable' do + let(:status) { Fabricate(:status, visibility: :public, text: 'With tags #one #two', tags: [prior_tag]) } + + it 'applies the tags from the status text and removes previous unused tags' do + expect { subject.call(status) } + .to change(Tag, :count).by(2) + .and change { status.reload.tags.map(&:name) }.from(contain_exactly('priortag')).to(contain_exactly('one', 'two')) + .and change { featured_tag.reload.statuses_count }.by(-1) + end + end + + context 'when status is not distributable' do + let(:status) { Fabricate(:status, visibility: :private, text: 'With tags #one #two', tags: [prior_tag]) } + + it 'applies the tags but does not modify featured tags' do + expect { subject.call(status) } + .to change(Tag, :count).by(2) + .and change { status.reload.tags.map(&:name) }.from(contain_exactly('priortag')).to(contain_exactly('one', 'two')) + .and(not_change { featured_tag.reload.statuses_count }) + end + end + + context 'when tags do not change' do + let(:status) { Fabricate(:status, visibility: :public, text: 'With tags #priortag', tags: [prior_tag]) } + + it 'does not modify tags or featured tags' do + expect { subject.call(status) } + .to not_change(Tag, :count) + .and not_change { status.reload.tags.map(&:name) }.from(contain_exactly('priortag')) + .and(not_change { featured_tag.reload.statuses_count }) + end end end end