Refactor find_relevant_topics and find_categories

This breaks down two private methods into smaller methods for
readability. This also moves logic into the Category model class.

app/models/category.rb

@@ -63,6 +63,9 @@ class Category < ActiveRecord::Base
       scoped_to_permissions(guardian, [:create_post, :full])
     end
   }
+
+  scope :featured_users, ->{ includes(:featured_users) }
+
   delegate :post_template, to: 'self.class'
 
   # permission is just used by serialization
@@ -148,6 +151,14 @@ SQL
                          posts_week = (#{posts_week})")
   end
 
+  def self.ordered_list(guardian)
+    self.secured(guardian)
+    .order('position asc')
+    .order('COALESCE(categories.posts_week, 0) DESC')
+    .order('COALESCE(categories.posts_month, 0) DESC')
+    .order('COALESCE(categories.posts_year, 0) DESC')
+  end
+
   def visible_posts
     query = Post.joins(:topic)
                 .where(['topics.category_id = ?', self.id])
@@ -331,8 +342,8 @@ SQL
   end
 
   def self.query_category(slug, parent_category_id)
-    self.where(slug: slug, parent_category_id: parent_category_id).includes(:featured_users).first ||
-    self.where(id: slug.to_i, parent_category_id: parent_category_id).includes(:featured_users).first
+    self.where(slug: slug, parent_category_id: parent_category_id).featured_users.first ||
+    self.where(id: slug.to_i, parent_category_id: parent_category_id).featured_users.first
   end
 
   def has_children?

app/models/category_list.rb

@@ -35,16 +35,23 @@ class CategoryList
 
     # Retrieve a list of all the topics we'll need
     def find_relevant_topics
+      @topics_by_id = {}
       @topics_by_category_id = {}
       category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank)
-      @topics_by_id = {}
 
+      build_topics_by_id(category_featured_topics)
+      build_topics_by_category_id(category_featured_topics)
+    end
+
+    def build_topics_by_id(category_featured_topics)
       @all_topics = Topic.where(id: category_featured_topics.map(&:topic_id))
       @all_topics.each do |t|
         t.include_last_poster = true if include_latest_posts? # hint for serialization
         @topics_by_id[t.id] = t
       end
+    end
 
+    def build_topics_by_category_id(category_featured_topics)
       category_featured_topics.each do |cft|
         @topics_by_category_id[cft.category_id] ||= []
         @topics_by_category_id[cft.category_id] << cft.topic_id
@@ -53,21 +60,26 @@ class CategoryList
 
     # Find a list of all categories to associate the topics with
     def find_categories
-      @categories = Category
-                        .includes(:featured_users, subcategories: [:topic_only_relative_url])
-                        .secured(@guardian)
-                        .order('position asc')
-                        .order('COALESCE(categories.posts_week, 0) DESC')
-                        .order('COALESCE(categories.posts_month, 0) DESC')
-                        .order('COALESCE(categories.posts_year, 0) DESC')
-                        .to_a
+      @categories = Category.featured_users.ordered_list(@guardian).to_a
+      subcategories = {}
+      to_delete = Set.new
 
       if latest_post_only?
         @categories  = @categories.includes(:latest_post => {:topic => :last_poster} )
       end
 
-      subcategories = {}
-      to_delete = Set.new
+      build_subcategories
+
+      if subcategories.present?
+        @categories.each { |c| c.subcategory_ids = subcategories[c.id] }
+        @categories.delete_if {|c| to_delete.include?(c) }
+      end
+
+      set_all_topics if latest_post_only?
+      set_displayable_category_topics if @topics_by_category_id
+    end
+
+    def build_subcategories
       @categories.each do |c|
         if c.parent_category_id.present?
           subcategories[c.parent_category_id] ||= []
@@ -75,44 +87,36 @@ class CategoryList
           to_delete << c
         end
       end
+    end
 
-      if subcategories.present?
-        @categories.each do |c|
-          c.subcategory_ids = subcategories[c.id]
-        end
-        @categories.delete_if {|c| to_delete.include?(c) }
-      end
-
-      if latest_post_only?
-        @all_topics = []
-        @categories.each do |c|
-          if c.latest_post && c.latest_post.topic
-            c.displayable_topics = [c.latest_post.topic]
-            topic = c.latest_post.topic
-            topic.include_last_poster = true # hint for serialization
-            @all_topics << topic
-          end
+    def set_all_topics
+      @all_topics = []
+      @categories.each do |c|
+        if c.latest_post && c.latest_post.topic
+          c.displayable_topics = [c.latest_post.topic]
+          topic = c.latest_post.topic
+          topic.include_last_poster = true # hint for serialization
+          @all_topics << topic
         end
       end
+    end
 
-      if @topics_by_category_id
-        @categories.each do |c|
-          topics_in_cat = @topics_by_category_id[c.id]
-          if topics_in_cat.present?
-            c.displayable_topics = []
-            topics_in_cat.each do |topic_id|
-              topic = @topics_by_id[topic_id]
-              if topic.present?
-                topic.category = c
-                c.displayable_topics << topic
-              end
+    def set_displayable_category_topics
+      @categories.each do |c|
+        topics_in_cat = @topics_by_category_id[c.id]
+        if topics_in_cat.present?
+          c.displayable_topics = []
+          topics_in_cat.each do |topic_id|
+            topic = @topics_by_id[topic_id]
+            if topic.present?
+              topic.category = c
+              c.displayable_topics << topic
             end
           end
         end
       end
     end
 
-
     # Remove any empty categories unless we can create them (so we can see the controls)
     def prune_empty
       if !@guardian.can_create?(Category)