From f0ec1fad8c120230ff86d01b42a6c049762cca9d Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Thu, 18 May 2023 11:46:44 +0200
Subject: [PATCH] FIX: Update category tag stats with new or deleted (#21531)

The old method updated only existing records, without considering that
new tags might have been created or some tags might not exist anymore.
This was usually not a problem because the stats were also updated by
other code paths.

However, the ensure consistency job should be more solid and help when
other code paths fail or after importing data.

Also, update category tag stats too should happen when updating other
category stats as well.
---
 app/models/category_tag_stat.rb       | 35 +++++++++++++++++-------
 lib/tasks/categories.rake             |  1 +
 spec/models/category_tag_stat_spec.rb | 39 +++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 spec/models/category_tag_stat_spec.rb

diff --git a/app/models/category_tag_stat.rb b/app/models/category_tag_stat.rb
index 478aa069cc2..0432c528645 100644
--- a/app/models/category_tag_stat.rb
+++ b/app/models/category_tag_stat.rb
@@ -44,23 +44,38 @@ class CategoryTagStat < ActiveRecord::Base
 
   # Recalculate all topic counts if they got out of sync
   def self.update_topic_counts
+    # Add new records or update existing records
     DB.exec <<~SQL
-      UPDATE category_tag_stats stats
-      SET topic_count = x.topic_count
-      FROM (
-        SELECT COUNT(topics.id) AS topic_count,
+      WITH stats AS (
+        SELECT topics.category_id as category_id,
                tags.id AS tag_id,
-               topics.category_id as category_id
+               COUNT(topics.id) AS topic_count
         FROM tags
         INNER JOIN topic_tags ON tags.id = topic_tags.tag_id
         INNER JOIN topics ON topics.id = topic_tags.topic_id
                AND topics.deleted_at IS NULL
                AND topics.category_id IS NOT NULL
-        GROUP BY tags.id, topics.category_id
-      ) x
-      WHERE stats.tag_id = x.tag_id
-        AND stats.category_id = x.category_id
-        AND x.topic_count <> stats.topic_count
+        GROUP BY topics.category_id, tags.id
+      )
+      INSERT INTO category_tag_stats(category_id, tag_id, topic_count)
+      SELECT category_id, tag_id, topic_count FROM stats
+      ON CONFLICT (category_id, tag_id) DO
+      UPDATE SET topic_count = EXCLUDED.topic_count
+    SQL
+
+    # Delete old records
+    DB.exec <<~SQL
+      DELETE FROM category_tag_stats
+      WHERE (category_id, tag_id) NOT IN (
+        SELECT topics.category_id as category_id,
+               tags.id AS tag_id
+        FROM tags
+        INNER JOIN topic_tags ON tags.id = topic_tags.tag_id
+        INNER JOIN topics ON topics.id = topic_tags.topic_id
+               AND topics.deleted_at IS NULL
+               AND topics.category_id IS NOT NULL
+        GROUP BY topics.category_id, tags.id
+      )
     SQL
   end
 end
diff --git a/lib/tasks/categories.rake b/lib/tasks/categories.rake
index 1e33a62166b..703af6834ed 100644
--- a/lib/tasks/categories.rake
+++ b/lib/tasks/categories.rake
@@ -24,6 +24,7 @@ task "categories:move_topics", %i[from_category to_category] => [:environment] d
 
     puts "Updating category stats..."
     Category.update_stats
+    CategoryTagStat.update_topic_counts
   end
 
   puts "", "Done!", ""
diff --git a/spec/models/category_tag_stat_spec.rb b/spec/models/category_tag_stat_spec.rb
new file mode 100644
index 00000000000..6a154848dcb
--- /dev/null
+++ b/spec/models/category_tag_stat_spec.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+describe CategoryTagStat do
+  fab!(:category) { Fabricate(:category) }
+  fab!(:tag) { Fabricate(:tag) }
+  fab!(:topic) { Fabricate(:topic, category: category, tags: [tag]) }
+
+  describe "#update_topic_counts" do
+    it "creates new records" do
+      CategoryTagStat.destroy_all
+
+      expect { CategoryTagStat.update_topic_counts }.to change { CategoryTagStat.count }.by(1)
+      category_tag_stat = CategoryTagStat.last
+      expect(category_tag_stat.category_id).to eq(category.id)
+      expect(category_tag_stat.tag_id).to eq(tag.id)
+      expect(category_tag_stat.topic_count).to eq(1)
+    end
+
+    it "updates existing records" do
+      CategoryTagStat.last.update(topic_count: 10)
+
+      expect { CategoryTagStat.update_topic_counts }.not_to change { CategoryTagStat.count }
+      category_tag_stat = CategoryTagStat.last
+      expect(category_tag_stat.category_id).to eq(category.id)
+      expect(category_tag_stat.tag_id).to eq(tag.id)
+      expect(category_tag_stat.topic_count).to eq(1)
+    end
+
+    it "deletes old records" do
+      CategoryTagStat.last.update(tag_id: Fabricate(:tag).id)
+
+      expect { CategoryTagStat.update_topic_counts }.not_to change { CategoryTagStat.count }
+      category_tag_stat = CategoryTagStat.last
+      expect(category_tag_stat.category_id).to eq(category.id)
+      expect(category_tag_stat.tag_id).to eq(tag.id)
+      expect(category_tag_stat.topic_count).to eq(1)
+    end
+  end
+end