Prefer your own topics in Suggested

lib/suggested_topics_builder.rb

@@ -0,0 +1,43 @@
+require_dependency 'topic_list'
+
+class SuggestedTopicsBuilder
+
+  attr_reader :excluded_topic_ids
+  attr_reader :results
+
+  def initialize(topic)
+    @excluded_topic_ids = [topic.id]
+    @results = []
+  end
+
+  def add_results(results)
+
+    return if results.blank?
+
+    # Only add results if we don't have those topic ids already
+    results = results.where('topics.id NOT IN (?)', @excluded_topic_ids)
+                     .where(closed: false, archived: false, visible: true)
+
+    return if results.blank?
+
+    # Keep track of the ids we've added
+    @excluded_topic_ids << results.map {|r| r.id}
+    @excluded_topic_ids.flatten!
+
+    @results << results
+    @results.flatten!
+  end
+
+  def results_left
+    SiteSetting.suggested_topics - @results.size
+  end
+
+  def full?
+    results_left == 0
+  end
+
+  def size
+    @results.size
+  end
+
+end

lib/topic_query.rb

@@ -3,6 +3,7 @@
 # found.
 #
 require_dependency 'topic_list'
+require_dependency 'suggested_topics_builder'
 
 class TopicQuery
 
@@ -84,49 +85,16 @@ class TopicQuery
 
   # Return a list of suggested topics for a topic
   def list_suggested_for(topic)
+    builder = SuggestedTopicsBuilder.new(topic)
 
-    exclude_topic_ids = [topic.id]
+    # When logged in we start with different results
-
+    if @user.present?
-    # If not logged in, return some random results, preferably in this category
+      builder.add_results(unread_results(per_page: builder.results_left))
-    if @user.blank?
+      builder.add_results(new_results(per_page: builder.results_left)) unless builder.full?
-      return TopicList.new(:suggested, @user, random_suggested_results_for(topic, SiteSetting.suggested_topics, exclude_topic_ids))
     end
+    builder.add_results(random_suggested(topic, builder.results_left)) unless builder.full?
 
-    results = unread_results(per_page: SiteSetting.suggested_topics)
+    TopicList.new(:suggested, @user, builder.results)
-                .where('topics.id NOT IN (?)', exclude_topic_ids)
-                .where(closed: false, archived: false, visible: true)
-                .all
-
-    results_left = SiteSetting.suggested_topics - results.size
-
-    # If we don't have enough results, go to new posts
-    if results_left > 0
-      exclude_topic_ids << results.map {|t| t.id}
-      exclude_topic_ids.flatten!
-
-      results << new_results(per_page: results_left)
-                  .where('topics.id NOT IN (?)', exclude_topic_ids)
-                  .where(closed: false, archived: false, visible: true)
-                  .all
-
-      results.flatten!
-
-      results_left = SiteSetting.suggested_topics - results.size
-
-      # If we STILL don't have enough results, find random topics
-      if results_left > 0
-        exclude_topic_ids << results.map {|t| t.id}
-        exclude_topic_ids.flatten!
-
-        results << random_suggested_results_for(topic, results_left, exclude_topic_ids)
-                    .where(closed: false, archived: false, visible: true)
-                    .all
-
-        results.flatten!
-      end
-    end
-
-    TopicList.new(:suggested, @user, results)
   end
 
   # The latest view of topics
@@ -214,7 +182,10 @@ class TopicQuery
   end
 
   def unread_results(list_opts={})
+    list_opts[:unordered] ||= true
     TopicQuery.unread_filter(default_list(list_opts))
+      .order('CASE WHEN topics.user_id = tu.user_id THEN 1 ELSE 2 END')
+      .order(TopicQuery.order_nocategory_with_pinned_sql)
   end
 
   protected
@@ -277,17 +248,15 @@ class TopicQuery
       result
     end
 
+    def random_suggested(topic, count)
+      result = default_list(unordered: true, per_page: count)
 
-    def random_suggested_results_for(topic, count, exclude_topic_ids)
+      # If we are in a category, prefer it for the random results
-      results = default_list(unordered: true, per_page: count)
-                 .where('topics.id NOT IN (?)', exclude_topic_ids)
-                 .where(closed: false, archived: false, visible: true)
-
       if topic.category_id.present?
-        return results.order("CASE WHEN topics.category_id = #{topic.category_id.to_i} THEN 0 ELSE 1 END, RANDOM()")
+        result = result.order("CASE WHEN topics.category_id = #{topic.category_id.to_i} THEN 0 ELSE 1 END, RANDOM()")
       end
 
-      results.order("RANDOM()")
+      result.order("RANDOM()")
     end
 
 end

spec/components/suggested_topics_builder_spec.rb

@@ -0,0 +1,68 @@
+require 'spec_helper'
+require 'suggested_topics_builder'
+
+describe SuggestedTopicsBuilder do
+
+  let!(:topic) { Fabricate(:topic) }
+
+  let!(:builder) { SuggestedTopicsBuilder.new(topic) }
+
+  before do
+    SiteSetting.stubs(:suggested_topics).returns(5)
+  end
+
+  it "has the correct defaults" do
+    builder.excluded_topic_ids.include?(topic.id).should be_true
+    builder.results_left.should == 5
+    builder.size.should == 0
+    builder.should_not be_full
+  end
+
+  it "returns full correctly" do
+    builder.stubs(:results_left).returns(0)
+    builder.should be_full
+  end
+
+  context "adding results" do
+
+    it "adds nothing with nil results" do
+      builder.add_results(nil)
+      builder.results_left.should == 5
+      builder.size.should == 0
+      builder.should_not be_full
+    end
+
+    context "adding topics" do
+      let!(:other_topic) { Fabricate(:topic) }
+
+      before do
+        # Add all topics
+        builder.add_results(Topic)
+      end
+
+      it "added the result correctly" do
+        builder.size.should == 1
+        builder.results_left.should == 4
+        builder.should_not be_full
+        builder.excluded_topic_ids.include?(topic.id).should be_true
+        builder.excluded_topic_ids.include?(other_topic.id).should be_true
+      end
+
+    end
+
+    context "adding invalid status topics" do
+      let!(:archived_topic) { Fabricate(:topic, archived: true)}
+      let!(:closed_topic) { Fabricate(:topic, closed: true)}
+      let!(:invisible_topic) { Fabricate(:topic, visible: false)}
+
+      it "doesn't add archived, closed or invisible topics" do
+        builder.add_results(Topic)
+        builder.size.should == 0
+        builder.should_not be_full
+      end
+    end
+
+  end
+
+
+end