FIX: /categories page on mobile

app/assets/javascripts/discourse/routes/discovery-categories.js.es6

@@ -31,11 +31,14 @@ const DiscoveryCategoriesRoute = Discourse.Route.extend(OpenComposer, {
   },
 
   setupController(controller, model) {
+    // only load latest topics in desktop view
+    if (!this.site.mobileView) {
       model.set("loadingTopics", true);
 
       TopicList.find("latest")
                .then(result => model.set("topicList", result))
                .finally(() => model.set("loadingTopics", false));
+    }
 
     controller.set("model", model);
 

app/assets/stylesheets/common/base/_topic-list.scss

@@ -107,64 +107,6 @@ html.anon .topic-list a.title:visited:not(.badge-notification) {color: dark-ligh
 
 }
 
-.navigation-categories {
-  .topic-list {
-    width: 48%;
-    float: left;
-  }
-  .main-link {
-    width: 100%;
-    .discourse-tag {
-      font-size: 12px;
-    }
-  }
-  .topic-stats {
-    text-align: right;
-    a {
-      color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 40%));
-    }
-  }
-  .num.posts {
-    text-align: right;
-    a {
-      padding: 0;
-    }
-  }
-  .posts-map.posts {
-    margin-bottom: 10px;
-    width: 100%;
-    .badge-posts {
-      font-weight: bold;
-    }
-  }
-  .topic-list-latest {
-    margin-left: 4%;
-    .new-posts {
-      margin-left: 5px;
-    }
-  }
-  .topic-list.categories {
-    th.stats {
-      width: 20%;
-    }
-    .stats {
-      vertical-align: top;
-      text-align: center;
-      .value {
-        font-size: 1.2em;
-      }
-    }
-  }
-  .no-topics, .more-topics {
-    border-bottom: none;
-    td {
-      padding-right: 0 !important;
-      color: $primary;
-    }
-  }
-}
-
-
 .topic-list.categories {
 
   .category .badge-notification {

app/assets/stylesheets/desktop/topic-list.scss

@@ -236,6 +236,63 @@
   margin: 20px 0;
 }
 
+.navigation-categories {
+  .topic-list {
+    width: 48%;
+    float: left;
+  }
+  .main-link {
+    width: 100%;
+    .discourse-tag {
+      font-size: 12px;
+    }
+  }
+  .topic-stats {
+    text-align: right;
+    a {
+      color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 40%));
+    }
+  }
+  .num.posts {
+    text-align: right;
+    a {
+      padding: 0;
+    }
+  }
+  .posts-map.posts {
+    margin-bottom: 10px;
+    width: 100%;
+    .badge-posts {
+      font-weight: bold;
+    }
+  }
+  .topic-list-latest {
+    margin-left: 4%;
+    .new-posts {
+      margin-left: 5px;
+    }
+  }
+  .topic-list.categories {
+    th.stats {
+      width: 20%;
+    }
+    .stats {
+      vertical-align: top;
+      text-align: center;
+      .value {
+        font-size: 1.2em;
+      }
+    }
+  }
+  .no-topics, .more-topics {
+    border-bottom: none;
+    td {
+      padding-right: 0 !important;
+      color: $primary;
+    }
+  }
+}
+
 // Misc. stuff
 // --------------------------------------------------
 

app/controllers/categories_controller.rb

@@ -16,7 +16,10 @@ class CategoriesController < ApplicationController
 
     @description = SiteSetting.site_description
 
-    category_options = { is_homepage: current_homepage == "categories".freeze }
+    category_options = {
+      is_homepage: current_homepage == "categories".freeze,
+      include_topics: view_context.mobile_view? || params[:include_topics]
+    }
 
     @category_list = CategoryList.new(guardian, category_options)
     @category_list.draft_key = Draft::NEW_TOPIC

app/models/category.rb

@@ -87,7 +87,7 @@ class Category < ActiveRecord::Base
 
   # permission is just used by serialization
   # we may consider wrapping this in another spot
-  attr_accessor :permission, :subcategory_ids, :notification_level, :has_children
+  attr_accessor :displayable_topics, :permission, :subcategory_ids, :notification_level, :has_children
 
   @topic_id_cache = DistributedCache.new('category_topic_ids')
 

app/models/category_list.rb

@@ -1,3 +1,5 @@
+require_dependency 'pinned_check'
+
 class CategoryList
   include ActiveModel::Serialization
 
@@ -11,7 +13,13 @@ class CategoryList
     @guardian = guardian || Guardian.new
     @options = options
 
+    find_relevant_topics if options[:include_topics]
     find_categories
+
+    prune_empty
+    find_user_data
+    sort_unpinned
+    trim_results
   end
 
   def preload_key
@@ -20,16 +28,25 @@ class CategoryList
 
   private
 
-    # Find a list of all categories to associate the topics with
+    def find_relevant_topics
+      @topics_by_id = {}
+      @topics_by_category_id = {}
+
+      category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank)
+
+      @all_topics = Topic.where(id: category_featured_topics.map(&:topic_id))
+      @all_topics.each { |t| @topics_by_id[t.id] = t }
+
+      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
+
     def find_categories
       @categories = Category.includes(:topic_only_relative_url, subcategories: [:topic_only_relative_url]).secured(@guardian)
       @categories = @categories.where(suppress_from_homepage: false) if @options[:is_homepage]
 
-      unless SiteSetting.allow_uncategorized_topics
-        # TODO: also make sure the uncategorized is empty
-        @categories = @categories.where("id <> #{SiteSetting.uncategorized_category_id}")
-      end
-
       if SiteSetting.fixed_category_positions
         @categories = @categories.order(:position, :id)
       else
@@ -64,7 +81,60 @@ class CategoryList
       end
 
       @categories.each { |c| c.subcategory_ids = subcategories[c.id] }
-
       @categories.delete_if { |c| to_delete.include?(c) }
+
+      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? && @guardian.can_see?(topic)
+                # topic.category is very slow under rails 4.2
+                topic.association(:category).target = c
+                c.displayable_topics << topic
               end
+            end
+          end
+        end
+      end
+    end
+
+    def prune_empty
+      return if SiteSetting.allow_uncategorized_topics
+      @categories.delete_if { |c| c.uncategorized? && c.displayable_topics.blank? }
+    end
+
+    # Attach some data for serialization to each topic
+    def find_user_data
+      if @guardian.current_user && @all_topics.present?
+        topic_lookup = TopicUser.lookup_for(@guardian.current_user, @all_topics)
+        @all_topics.each { |ft| ft.user_data = topic_lookup[ft.id] }
+      end
+    end
+
+    # Put unpinned topics at the end of the list
+    def sort_unpinned
+      if @guardian.current_user && @all_topics.present?
+        @categories.each do |c|
+          next if c.displayable_topics.blank? || c.displayable_topics.size <= SiteSetting.category_featured_topics
+          unpinned = []
+          c.displayable_topics.each do |t|
+            unpinned << t if t.pinned_at && PinnedCheck.unpinned?(t, t.user_data)
+          end
+          unless unpinned.empty?
+            c.displayable_topics = (c.displayable_topics - unpinned) + unpinned
+          end
+        end
+      end
+    end
+
+    def trim_results
+      @categories.each do |c|
+        next if c.displayable_topics.blank?
+        c.displayable_topics = c.displayable_topics[0, SiteSetting.category_featured_topics]
+      end
+    end
+
 end

app/serializers/category_detailed_serializer.rb

@@ -10,6 +10,12 @@ class CategoryDetailedSerializer < BasicCategorySerializer
              :is_uncategorized,
              :subcategory_ids
 
+  has_many :displayable_topics, serializer: ListableTopicSerializer, embed: :objects, key: :topics
+
+  def include_displayable_topics?
+    displayable_topics.present?
+  end
+
   def is_uncategorized
     object.id == SiteSetting.uncategorized_category_id
   end

spec/models/category_list_spec.rb

@@ -5,9 +5,10 @@ describe CategoryList do
 
   let(:user) { Fabricate(:user) }
   let(:admin) { Fabricate(:admin) }
-  let(:category_list) { CategoryList.new(Guardian.new user) }
+  let(:category_list) { CategoryList.new(Guardian.new(user), include_topics: true) }
 
   context "security" do
+
     it "properly hide secure categories" do
       cat = Fabricate(:category)
       Fabricate(:topic, category: cat)
@@ -19,6 +20,33 @@ describe CategoryList do
       expect(CategoryList.new(Guardian.new user).categories.count).to eq(1)
       expect(CategoryList.new(Guardian.new nil).categories.count).to eq(1)
     end
+
+    it "doesn't show topics that you can't view" do
+      public_cat = Fabricate(:category) # public category
+      Fabricate(:topic, category: public_cat)
+
+      private_cat = Fabricate(:category) # private category
+      Fabricate(:topic, category: private_cat)
+      private_cat.set_permissions(admins: :full)
+      private_cat.save
+
+      secret_subcat = Fabricate(:category, parent_category_id: public_cat.id) # private subcategory
+      Fabricate(:topic, category: secret_subcat)
+      secret_subcat.set_permissions(admins: :full)
+      secret_subcat.save
+
+      CategoryFeaturedTopic.feature_topics
+
+      expect(CategoryList.new(Guardian.new(admin), include_topics: true).categories.find { |x| x.name == public_cat.name }.displayable_topics.count).to eq(2)
+      expect(CategoryList.new(Guardian.new(admin), include_topics: true).categories.find { |x| x.name == private_cat.name }.displayable_topics.count).to eq(1)
+
+      expect(CategoryList.new(Guardian.new(user), include_topics: true).categories.find { |x| x.name == public_cat.name }.displayable_topics.count).to eq(1)
+      expect(CategoryList.new(Guardian.new(user), include_topics: true).categories.find { |x| x.name == private_cat.name }).to eq(nil)
+
+      expect(CategoryList.new(Guardian.new(nil), include_topics: true).categories.find { |x| x.name == public_cat.name }.displayable_topics.count).to eq(1)
+      expect(CategoryList.new(Guardian.new(nil), include_topics: true).categories.find { |x| x.name == private_cat.name }).to eq(nil)
+    end
+
   end
 
   context "with a category" do
@@ -34,7 +62,27 @@ describe CategoryList do
         expect(category.id).to eq(topic_category.id)
         expect(category.featured_topics.include?(topic)).to eq(true)
       end
+    end
 
+    context "with pinned topics in a category" do
+      let!(:topic1) { Fabricate(:topic, category: topic_category, bumped_at: 8.minutes.ago) }
+      let!(:topic2) { Fabricate(:topic, category: topic_category, bumped_at: 5.minutes.ago) }
+      let!(:topic3) { Fabricate(:topic, category: topic_category, bumped_at: 2.minutes.ago) }
+      let!(:pinned) { Fabricate(:topic, category: topic_category, pinned_at: 10.minutes.ago, bumped_at: 10.minutes.ago) }
+      let(:category) { category_list.categories.find{|c| c.id == topic_category.id} }
+
+      before do
+        SiteSetting.stubs(:category_featured_topics).returns(2)
+      end
+
+      it "returns pinned topic first" do
+        expect(category.displayable_topics.map(&:id)).to eq([pinned.id, topic3.id])
+      end
+
+      it "returns topics in bumped_at order if pinned was unpinned" do
+        PinnedCheck.stubs(:unpinned?).returns(true)
+        expect(category.displayable_topics.map(&:id)).to eq([topic3.id, topic2.id])
+      end
     end
 
   end