Merge 486448f0be into 9a2be25199

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

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