Merge pull request #178 from danneu/update-stats-fix

Category year/month/week stats shouldn't include deleted topics.

app/models/category.rb

@@ -17,7 +17,6 @@ class Category < ActiveRecord::Base
   after_save :invalidate_site_cache
   after_destroy :invalidate_site_cache
 
-
   def uncategorized_validator
     return errors.add(:name, I18n.t(:is_reserved)) if name == SiteSetting.uncategorized_name
     return errors.add(:slug, I18n.t(:is_reserved)) if slug == SiteSetting.uncategorized_name
@@ -27,23 +26,21 @@ class Category < ActiveRecord::Base
     order('topic_count desc')
   end
   
+  # Recalculates `topics_year`, `topics_month`, and `topics_week`
+  # for each Category.
   def self.update_stats
-    exec_sql "UPDATE categories
-              SET topics_week = (SELECT COUNT(*)
-                                FROM topics as ft
-                                WHERE ft.category_id = categories.id
-                                  AND ft.created_at > (CURRENT_TIMESTAMP - INTERVAL '1 WEEK')
-                                  AND ft.visible),
-                  topics_month = (SELECT COUNT(*)
-                                FROM topics as ft
-                                WHERE ft.category_id = categories.id
-                                  AND ft.created_at > (CURRENT_TIMESTAMP - INTERVAL '1 MONTH')
-                                  AND ft.visible),
-                  topics_year = (SELECT COUNT(*)
-                                FROM topics as ft
-                                WHERE ft.category_id = categories.id
-                                  AND ft.created_at > (CURRENT_TIMESTAMP - INTERVAL '1 YEAR')
-                                  AND ft.visible)"
+    topics = Topic
+               .select("COUNT(*)")
+               .where("topics.category_id = categories.id")
+               .visible
+
+    topics_year = topics.created_since(1.year.ago).to_sql
+    topics_month = topics.created_since(1.month.ago).to_sql
+    topics_week = topics.created_since(1.week.ago).to_sql
+    
+    Category.update_all("topics_year = (#{topics_year}), 
+                         topics_month = (#{topics_month}), 
+                         topics_week = (#{topics_week})")
   end
 
   # Use the first paragraph of the topic's first post as the excerpt

app/models/topic.rb

@@ -168,6 +168,10 @@ class Topic < ActiveRecord::Base
     where(visible: true)
   end
   
+  def self.created_since(time_ago)
+    where("created_at > ?", time_ago)
+  end
+  
   def private_message?
     self.archetype == Archetype.private_message
   end

spec/fabricators/topic_fabricator.rb

@@ -3,6 +3,10 @@ Fabricator(:topic) do
   title { sequence(:title) { |i| "Test topic #{i}" } }
 end
 
+Fabricator(:deleted_topic, from: :topic) do
+  deleted_at Time.now 
+end
+
 Fabricator(:topic_allowed_user) do 
 end
 

spec/models/category_spec.rb

@@ -158,13 +158,15 @@ describe Category do
 
   describe 'update_stats' do
     
-    # We're going to test with one topic. That's enough for stats!
     before do
       @category = Fabricate(:category)
+    end
     
-      # Create a non-invisible category to make sure count is 1
-      @topic = Fabricate(:topic, user: @category.user, category: @category)     
+    context 'with regular topics' do
 
+      before do
+        @category.topics << Fabricate(:topic, 
+                                      user: @category.user)     
         Category.update_stats
         @category.reload
       end
@@ -183,5 +185,29 @@ describe Category do
     
     end
     
-end
+    context 'with deleted topics' do
 
+      before do
+        @category.topics << Fabricate(:deleted_topic, 
+                                      user: @category.user)
+        Category.update_stats
+        @category.reload
+      end
+
+      it 'does not count deleted topics for topics_week' do
+        @category.topics_week.should == 0
+      end
+
+      it 'does not count deleted topics for topics_month' do
+        @category.topics_month.should == 0
+      end
+
+      it 'does not count deleted topics for topics_year' do
+        @category.topics_year.should == 0
+      end
+
+    end
+
+  end
+
+end