FIX: Update topic_count when updating visibility (#11946)

Updating a topic's visibility did not increase or decrease the
topic_count of a category, but Category.update_stats does ignore
unlisted topics which resulted in inconsistencies when deleting
such topics.

app/models/topic.rb

@@ -132,7 +132,7 @@ class Topic < ActiveRecord::Base
 
   def trash!(trashed_by = nil)
     if deleted_at.nil?
-      update_category_topic_count_by(-1)
+      update_category_topic_count_by(-1) if visible?
       CategoryTagStat.topic_deleted(self) if self.tags.present?
       DiscourseEvent.trigger(:topic_trashed, self)
     end
@@ -142,7 +142,7 @@ class Topic < ActiveRecord::Base
 
   def recover!(recovered_by = nil)
     unless deleted_at.nil?
-      update_category_topic_count_by(1)
+      update_category_topic_count_by(1) if visible?
       CategoryTagStat.topic_recovered(self) if self.tags.present?
       DiscourseEvent.trigger(:topic_recovered, self)
     end
@@ -1648,8 +1648,9 @@ class Topic < ActiveRecord::Base
   def update_category_topic_count_by(num)
     if category_id.present?
       Category
-        .where(['id = ?', category_id])
-        .update_all("topic_count = topic_count " + (num > 0 ? '+' : '') + "#{num}")
+        .where('id = ?', category_id)
+        .where('topic_id != ? OR topic_id IS NULL', self.id)
+        .update_all("topic_count = topic_count + #{num.to_i}")
     end
   end
 

app/services/topic_status_updater.rb

@@ -46,6 +46,10 @@ TopicStatusUpdater = Struct.new(:topic, :user) do
       UserProfile.remove_featured_topic_from_all_profiles(topic)
     end
 
+    if status.visible?
+      topic.update_category_topic_count_by(status.enabled? ? 1 : -1)
+    end
+
     if @topic_timer
       if status.manually_closing_topic? || status.closing_topic?
         topic.delete_topic_timer(TopicTimer.types[:close])

spec/models/topic_spec.rb

@@ -1112,6 +1112,8 @@ describe Topic do
     end
 
     context 'visibility' do
+      let(:category) { Fabricate(:category_with_definition) }
+
       context 'disable' do
         it 'should not be visible and have correct counts' do
           topic.update_status('visible', false, @user)
@@ -1121,6 +1123,14 @@ describe Topic do
           expect(topic.bumped_at).to eq_time(@original_bumped_at)
         end
 
+        it 'decreases topic_count of topic category' do
+          topic.update!(category: category)
+          Category.update_stats
+
+          expect { topic.update_status('visible', false, @user) }
+            .to change { category.reload.topic_count }.by(-1)
+        end
+
         it 'removes itself as featured topic on user profiles' do
           user.user_profile.update(featured_topic_id: topic.id)
           expect(user.user_profile.featured_topic).to eq(topic)
@@ -1142,6 +1152,13 @@ describe Topic do
           expect(topic.moderator_posts_count).to eq(1)
           expect(topic.bumped_at).to eq_time(@original_bumped_at)
         end
+
+        it 'increases topic_count of topic category' do
+          topic.update!(category: category, visible: false)
+
+          expect { topic.update_status('visible', true, @user) }
+            .to change { category.reload.topic_count }.by(1)
+        end
       end
     end
 
@@ -2144,6 +2161,11 @@ describe Topic do
         topic = Fabricate(:topic, category: category, deleted_at: 1.day.ago)
         expect { topic.trash!(moderator) }.to_not change { category.reload.topic_count }
       end
+
+      it "doesn't subtract 1 if topic is unlisted" do
+        topic = Fabricate(:topic, category: category, visible: false)
+        expect { topic.trash!(moderator) }.to_not change { category.reload.topic_count }
+      end
     end
 
     it "trashes topic embed record" do
@@ -2169,6 +2191,11 @@ describe Topic do
         topic = Fabricate(:topic, category: category)
         expect { topic.recover! }.to_not change { category.reload.topic_count }
       end
+
+      it "doesn't add 1 if topic is not visible" do
+        topic = Fabricate(:topic, category: category, visible: false)
+        expect { topic.recover! }.to_not change { category.reload.topic_count }
+      end
     end
 
     it "recovers topic embed record" do