diff --git a/app/assets/javascripts/discourse/routes/discovery-categories.js.es6 b/app/assets/javascripts/discourse/routes/discovery-categories.js.es6 index 37be18bc3..bdab235a9 100644 --- a/app/assets/javascripts/discourse/routes/discovery-categories.js.es6 +++ b/app/assets/javascripts/discourse/routes/discovery-categories.js.es6 @@ -31,11 +31,14 @@ const DiscoveryCategoriesRoute = Discourse.Route.extend(OpenComposer, { }, setupController(controller, model) { - model.set("loadingTopics", true); + // 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)); + TopicList.find("latest") + .then(result => model.set("topicList", result)) + .finally(() => model.set("loadingTopics", false)); + } controller.set("model", model); diff --git a/app/assets/stylesheets/common/base/_topic-list.scss b/app/assets/stylesheets/common/base/_topic-list.scss index 0dec7467d..ca65f1d1f 100644 --- a/app/assets/stylesheets/common/base/_topic-list.scss +++ b/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 { diff --git a/app/assets/stylesheets/desktop/topic-list.scss b/app/assets/stylesheets/desktop/topic-list.scss index 0df2a15fe..abf868895 100644 --- a/app/assets/stylesheets/desktop/topic-list.scss +++ b/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 // -------------------------------------------------- diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 463e0084a..fd5af6cd4 100644 --- a/app/controllers/categories_controller.rb +++ b/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 diff --git a/app/models/category.rb b/app/models/category.rb index a52cdeff3..bc841d62b 100644 --- a/app/models/category.rb +++ b/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') diff --git a/app/models/category_list.rb b/app/models/category_list.rb index 38f9fc14f..153fa3569 100644 --- a/app/models/category_list.rb +++ b/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 diff --git a/app/serializers/category_detailed_serializer.rb b/app/serializers/category_detailed_serializer.rb index d7a454b19..9faaf484a 100644 --- a/app/serializers/category_detailed_serializer.rb +++ b/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 diff --git a/spec/models/category_list_spec.rb b/spec/models/category_list_spec.rb index e047945f2..a94d07d4a 100644 --- a/spec/models/category_list_spec.rb +++ b/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