From 28466eb5b29c70437d08f5beb4a9f400b5ef40c2 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 28 Aug 2013 10:51:49 +1000
Subject: [PATCH] group the "suggested topics" by category correctly.

in the past new topics were not prioritizing current category and
new topics in a category were not being inserted before other unread topics
in other categories
---
 lib/suggested_topics_builder.rb               | 37 +++++++++++++++++--
 lib/topic_query.rb                            |  6 +--
 .../suggested_topics_builder_spec.rb          | 34 +++++++++++++++--
 3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/lib/suggested_topics_builder.rb b/lib/suggested_topics_builder.rb
index 35ca744c005..26a83dcff9c 100644
--- a/lib/suggested_topics_builder.rb
+++ b/lib/suggested_topics_builder.rb
@@ -3,14 +3,15 @@ require_dependency 'topic_list'
 class SuggestedTopicsBuilder
 
   attr_reader :excluded_topic_ids
-  attr_reader :results
 
   def initialize(topic)
     @excluded_topic_ids = [topic.id]
+    @category_id = topic.category_id
     @results = []
   end
 
-  def add_results(results)
+
+  def add_results(results, priority=:low)
 
     # WARNING .blank? will execute an Active Record query
     return unless results
@@ -23,16 +24,46 @@ class SuggestedTopicsBuilder
     unless results.empty?
       # Keep track of the ids we've added
       @excluded_topic_ids.concat results.map {|r| r.id}
+      splice_results(results,priority)
+    end
+  end
+
+  def splice_results(results, priority)
+    if  @category_id &&
+        priority == :high &&
+        non_category_index = @results.index{|r| r.category_id != @category_id}
+
+      category_results, non_category_results = results.partition{|r| r.category_id == @category_id}
+
+      @results.insert non_category_index, *category_results
+      @results.concat non_category_results
+    else
       @results.concat results
     end
   end
 
+  def results
+    @results.first(SiteSetting.suggested_topics)
+  end
+
   def results_left
     SiteSetting.suggested_topics - @results.size
   end
 
   def full?
-    results_left == 0
+    results_left <= 0
+  end
+
+  def category_results_left
+    SiteSetting.suggested_topics - @results.count{|r| r.category_id == @category_id}
+  end
+
+  def category_full?
+    if @category_id
+
+    else
+      full?
+    end
   end
 
   def size
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index 6bd63830a74..766fb8178c1 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -87,10 +87,10 @@ class TopicQuery
 
     # When logged in we start with different results
     if @user
-      builder.add_results(unread_results(topic: topic, per_page: builder.results_left))
-      builder.add_results(new_results(per_page: builder.results_left)) unless builder.full?
+      builder.add_results(unread_results(topic: topic, per_page: builder.results_left), :high)
+      builder.add_results(new_results(topic: topic, per_page: builder.category_results_left), :high) unless builder.category_full?
     end
-    builder.add_results(random_suggested(topic, builder.results_left)) unless builder.full?
+    builder.add_results(random_suggested(topic, builder.results_left), :low) unless builder.full?
 
     create_list(:suggested, {}, builder.results)
   end
diff --git a/spec/components/suggested_topics_builder_spec.rb b/spec/components/suggested_topics_builder_spec.rb
index ed35220ab1d..71187f70998 100644
--- a/spec/components/suggested_topics_builder_spec.rb
+++ b/spec/components/suggested_topics_builder_spec.rb
@@ -3,14 +3,42 @@ require 'suggested_topics_builder'
 
 describe SuggestedTopicsBuilder do
 
-  let!(:topic) { Fabricate(:topic) }
-
-  let!(:builder) { SuggestedTopicsBuilder.new(topic) }
+  let(:topic) { Fabricate(:topic) }
+  let(:builder) { SuggestedTopicsBuilder.new(topic) }
 
   before do
     SiteSetting.stubs(:suggested_topics).returns(5)
   end
 
+  context "splicing category results" do
+
+    def fake_topic(topic_id, category_id)
+      build(:topic, id: topic_id, category_id: category_id)
+    end
+
+    let(:builder) do
+      SuggestedTopicsBuilder.new(fake_topic(1,1))
+    end
+
+    it "prioritizes category correctly" do
+      builder.splice_results([fake_topic(2,2)], :high)
+      builder.splice_results([fake_topic(3,1)], :high)
+      builder.splice_results([fake_topic(4,1)], :high)
+
+      builder.results.map(&:id).should == [3,4,2]
+
+      # we have 2 items in category 1
+      builder.category_results_left.should == 3
+    end
+
+    it "inserts using default approach for non high priority" do
+      builder.splice_results([fake_topic(2,2)], :high)
+      builder.splice_results([fake_topic(3,1)], :low)
+
+      builder.results.map(&:id).should == [2,3]
+    end
+  end
+
   it "has the correct defaults" do
     builder.excluded_topic_ids.include?(topic.id).should be_true
     builder.results_left.should == 5