From a43ed88699acb300048973c06f847b9ffc39f593 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 12 Jul 2013 14:38:20 -0400 Subject: [PATCH] Prefer your own topics in Suggested --- lib/suggested_topics_builder.rb | 43 ++++++++++++ lib/topic_query.rb | 63 +++++------------ .../suggested_topics_builder_spec.rb | 68 +++++++++++++++++++ 3 files changed, 127 insertions(+), 47 deletions(-) create mode 100644 lib/suggested_topics_builder.rb create mode 100644 spec/components/suggested_topics_builder_spec.rb diff --git a/lib/suggested_topics_builder.rb b/lib/suggested_topics_builder.rb new file mode 100644 index 00000000000..b57adaf40a5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 3b5c98c844a..eaaedd72956 100644 --- a/lib/topic_query.rb +++ b/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] - - # If not logged in, return some random results, preferably in this category - if @user.blank? - return TopicList.new(:suggested, @user, random_suggested_results_for(topic, SiteSetting.suggested_topics, exclude_topic_ids)) + # When logged in we start with different results + if @user.present? + builder.add_results(unread_results(per_page: builder.results_left)) + builder.add_results(new_results(per_page: builder.results_left)) unless builder.full? end + builder.add_results(random_suggested(topic, builder.results_left)) unless builder.full? - results = unread_results(per_page: SiteSetting.suggested_topics) - .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) + TopicList.new(:suggested, @user, builder.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) - results = default_list(unordered: true, per_page: count) - .where('topics.id NOT IN (?)', exclude_topic_ids) - .where(closed: false, archived: false, visible: true) - + # If we are in a category, prefer it for the random results 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 diff --git a/spec/components/suggested_topics_builder_spec.rb b/spec/components/suggested_topics_builder_spec.rb new file mode 100644 index 00000000000..ed35220ab1d --- /dev/null +++ b/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