Include pinned topics in category list.

- removes an (n+1) query for user data
- supports the preload store for the data to avoid a second request
- fix a bug where uncategorizes was reporting (0, 0, 0) for topics by week, month, year

app/assets/javascripts/discourse/models/category_list.js

@@ -31,7 +31,10 @@ Discourse.CategoryList.reopenClass({
         var uncategorized = Discourse.Category.uncategorizedInstance();
         uncategorized.setProperties({
           topics: c.topics,
-          featured_users: c.featured_users
+          featured_users: c.featured_users,
+          topics_week: c.topics_week,
+          topics_month: c.topics_month,
+          topics_year: c.topics_year
         });
         categories.pushObject(uncategorized);
       } else {
@@ -43,8 +46,16 @@ Discourse.CategoryList.reopenClass({
 
   list: function(filter) {
     var route = this;
+    var finder = null;
+    if (filter === 'categories') {
+      finder = PreloadStore.getAndRemove("categories_list", function() {
+        return Discourse.ajax("/categories.json")
+      });
+    } else {
+      finder = Discourse.ajax("/" + filter + ".json")
+    }
 
-    return Discourse.ajax("/" + filter + ".json").then(function(result) {
+    return finder.then(function(result) {
       var categoryList = Discourse.TopicList.create();
       categoryList.set('can_create_category', result.category_list.can_create_category);
       categoryList.set('can_create_topic', result.category_list.can_create_topic);

app/controllers/categories_controller.rb

@@ -9,6 +9,8 @@ class CategoriesController < ApplicationController
   def index
     @list = CategoryList.new(guardian)
     discourse_expires_in 1.minute
+
+    store_preloaded("categories_list", MultiJson.dump(CategoryListSerializer.new(@list, scope: guardian)))
     respond_to do |format|
       format.html { render }
       format.json { render_serialized(@list, CategoryListSerializer) }

app/models/category.rb

@@ -62,6 +62,8 @@ class Category < ActiveRecord::Base
                          topics_week = (#{topics_week})")
   end
 
+  attr_accessor :displayable_topics
+
   # Internal: Generate the text of post prompting to enter category
   # description.
   def self.post_template

app/models/category_featured_topic.rb

@@ -16,22 +16,14 @@ class CategoryFeaturedTopic < ActiveRecord::Base
     return if c.blank?
 
     CategoryFeaturedTopic.transaction do
-      exec_sql "DELETE FROM category_featured_topics WHERE category_id = :category_id", category_id: c.id
+      CategoryFeaturedTopic.delete_all(category_id: c.id)
-      exec_sql "INSERT INTO category_featured_topics (category_id, topic_id, created_at, updated_at)
+      query = TopicQuery.new(nil, per_page: SiteSetting.category_featured_topics)
-                SELECT :category_id,
+      results = query.list_category(c)
-                       ft.id,
+      if results.present?
-                       CURRENT_TIMESTAMP,
+        results.topic_ids.each_with_index do |topic_id, idx|
-                       CURRENT_TIMESTAMP
+          c.category_featured_topics.create(topic_id: topic_id, rank: idx)
-                FROM topics AS ft
+        end
-                WHERE ft.category_id = :category_id
+      end
-                  AND ft.visible
-                  AND ft.deleted_at IS NULL
-                  AND ft.archetype <> :private_message
-                ORDER BY ft.bumped_at DESC
-                LIMIT :featured_limit",
-                category_id: c.id,
-                private_message: Archetype.private_message,
-                featured_limit: SiteSetting.category_featured_topics
     end
   end
 

app/models/category_list.rb

@@ -3,19 +3,59 @@ class CategoryList
 
   attr_accessor :categories, :topic_users, :uncategorized
 
-  def initialize(guardian)
+  def initialize(guardian=nil)
-    guardian ||= Guardian.new
+    @guardian = guardian || Guardian.new
 
+    find_relevant_topics
+    find_categories
+    add_uncategorized
+    find_user_data
+  end
+
+  private
+
+    # Retrieve a list of all the topics we'll need
+    def find_relevant_topics
+      @topics_by_category_id = {}
+      category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank)
+      @topics_by_id = {}
+
+      @all_topics = Topic.where(id: category_featured_topics.map(&:topic_id))
+      @all_topics.each do |t|
+        @topics_by_id[t.id] = t
+      end
+
+      category_featured_topics.each do |cft|
+        @topics_by_category_id[cft.category_id] ||= []
+        @topics_by_category_id[cft.category_id] << cft.topic_id
+      end
+    end
+
+    # Find a list of all categories to associate the topics with
+    def find_categories
       @categories = Category
-                    .includes(featured_topics: [:category])
                       .includes(:featured_users)
-                    .where('topics.visible' => true)
+                      .secured(@guardian)
-                    .secured(guardian)
+                      .order('COALESCE(categories.topics_week, 0) DESC')
-                    .order('categories.topics_week desc, categories.topics_month desc, categories.topics_year desc')
+                      .order('COALESCE(categories.topics_month, 0) DESC')
-
+                      .order('COALESCE(categories.topics_year, 0) DESC')
 
       @categories = @categories.to_a
+      @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]
+            topic.category = c
+            c.displayable_topics << topic
+          end
+        end
+      end
+    end
 
+    # Add the uncategorized "magic" category
+    def add_uncategorized
       # Support for uncategorized topics
       uncategorized_topics = Topic
                         .listable_topics
@@ -44,7 +84,7 @@ class CategoryList
         # Find the appropriate place to insert it:
         insert_at = nil
         @categories.each_with_index do |c, idx|
-        if totals['topics_week'].to_i > (c.topics_week || 0)
+          if uncategorized.topics_week > (c.topics_week || 0)
             insert_at = idx
             break
           end
@@ -53,31 +93,28 @@ class CategoryList
         @categories.insert(insert_at || @categories.size, uncategorized)
       end
 
-    unless guardian.can_create?(Category)
+      if @all_topics.present?
+        uncategorized.displayable_topics = uncategorized_topics
+        @all_topics << uncategorized_topics
+        @all_topics.flatten!
+      end
+    end
+
+    # Remove any empty topics unless we can create them (so we can see the controls)
+    def prune_empty
+      unless @guardian.can_create?(Category)
         # Remove categories with no featured topics unless we have the ability to edit one
-      @categories.delete_if { |c| c.featured_topics.blank? }
+        @categories.delete_if { |c| c.displayable_topics.blank? }
-    else
-      # Show all categories to people who have the ability to edit and delete categories
-      if @categories.size > 0
-        @categories.insert(@categories.size, *Category.where('id not in (?)', @categories.map(&:id).compact).to_a)
-      else
-        @categories = Category.all.to_a
       end
     end
 
     # Get forum topic user records if appropriate
-    if guardian.current_user
+    def find_user_data
-      topics = []
+      if @guardian.current_user && @all_topics.present?
-      @categories.each { |c| topics << c.featured_topics }
+        topic_lookup = TopicUser.lookup_for(@guardian.current_user, @all_topics)
-      topics << @uncategorized
-
-      topics.flatten! if topics.present?
-      topics.compact! if topics.present?
-
-      topic_lookup = TopicUser.lookup_for(guardian.current_user, topics)
 
         # Attach some data for serialization to each topic
-      topics.each { |ft| ft.user_data = topic_lookup[ft.id] }
+        @all_topics.each { |ft| ft.user_data = topic_lookup[ft.id] }
       end
     end
 end

app/models/topic_list.rb

@@ -41,6 +41,11 @@ class TopicList
     return @topics
   end
 
+  def topic_ids
+    return [] if @topics_input.blank?
+    @topics_input.map {|t| t.id}
+  end
+
   def filter_summary
     @filter_summary ||= get_summary
   end

app/serializers/category_detailed_serializer.rb

@@ -13,7 +13,7 @@ class CategoryDetailedSerializer < ApplicationSerializer
              :is_uncategorized
 
   has_many :featured_users, serializer: BasicUserSerializer
-  has_many :featured_topics, serializer: CategoryTopicSerializer, embed: :objects, key: :topics
+  has_many :displayable_topics, serializer: CategoryTopicSerializer, embed: :objects, key: :topics
 
   def topics_week
     object.topics_week || 0
@@ -35,4 +35,8 @@ class CategoryDetailedSerializer < ApplicationSerializer
     is_uncategorized
   end
 
+  def include_displayable_topics?
+    return displayable_topics.present?
+  end
+
 end

app/views/categories/index.html.erb

@@ -2,9 +2,11 @@
   <div class="category">
     <h2><a href="/category/<%= c.slug.blank? ? c.id : c.slug %>"><%= c.name %></a></h2>
     <div class="topic-list">
-      <% c.topics.limit(6).each do |t| %>
+      <%- if c.displayable_topics.present? %>
+        <% c.displayable_topics.each do |t| %>
           <a href="<%= t.relative_url %>"><%= t.title %></a> <span title='<%= t 'posts' %>'>(<%= t.posts_count %>)</span><br/>
         <% end %>
+      <%- end %>
     </div>
   </div>
 <% end %>

db/migrate/20130528174147_add_rank_to_category_featured_topics.rb

@@ -0,0 +1,6 @@
+class AddRankToCategoryFeaturedTopics < ActiveRecord::Migration
+  def change
+    add_column :category_featured_topics, :rank, :integer, default: 0, null: false
+    add_index :category_featured_topics, [:category_id, :rank]
+  end
+end