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
This commit is contained in:
parent
c0f610daf6
commit
28466eb5b2
3 changed files with 68 additions and 9 deletions
|
@ -3,14 +3,15 @@ require_dependency 'topic_list'
|
||||||
class SuggestedTopicsBuilder
|
class SuggestedTopicsBuilder
|
||||||
|
|
||||||
attr_reader :excluded_topic_ids
|
attr_reader :excluded_topic_ids
|
||||||
attr_reader :results
|
|
||||||
|
|
||||||
def initialize(topic)
|
def initialize(topic)
|
||||||
@excluded_topic_ids = [topic.id]
|
@excluded_topic_ids = [topic.id]
|
||||||
|
@category_id = topic.category_id
|
||||||
@results = []
|
@results = []
|
||||||
end
|
end
|
||||||
|
|
||||||
def add_results(results)
|
|
||||||
|
def add_results(results, priority=:low)
|
||||||
|
|
||||||
# WARNING .blank? will execute an Active Record query
|
# WARNING .blank? will execute an Active Record query
|
||||||
return unless results
|
return unless results
|
||||||
|
@ -23,16 +24,46 @@ class SuggestedTopicsBuilder
|
||||||
unless results.empty?
|
unless results.empty?
|
||||||
# Keep track of the ids we've added
|
# Keep track of the ids we've added
|
||||||
@excluded_topic_ids.concat results.map {|r| r.id}
|
@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
|
@results.concat results
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def results
|
||||||
|
@results.first(SiteSetting.suggested_topics)
|
||||||
|
end
|
||||||
|
|
||||||
def results_left
|
def results_left
|
||||||
SiteSetting.suggested_topics - @results.size
|
SiteSetting.suggested_topics - @results.size
|
||||||
end
|
end
|
||||||
|
|
||||||
def full?
|
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
|
end
|
||||||
|
|
||||||
def size
|
def size
|
||||||
|
|
|
@ -87,10 +87,10 @@ class TopicQuery
|
||||||
|
|
||||||
# When logged in we start with different results
|
# When logged in we start with different results
|
||||||
if @user
|
if @user
|
||||||
builder.add_results(unread_results(topic: topic, per_page: builder.results_left))
|
builder.add_results(unread_results(topic: topic, per_page: builder.results_left), :high)
|
||||||
builder.add_results(new_results(per_page: builder.results_left)) unless builder.full?
|
builder.add_results(new_results(topic: topic, per_page: builder.category_results_left), :high) unless builder.category_full?
|
||||||
end
|
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)
|
create_list(:suggested, {}, builder.results)
|
||||||
end
|
end
|
||||||
|
|
|
@ -3,14 +3,42 @@ require 'suggested_topics_builder'
|
||||||
|
|
||||||
describe SuggestedTopicsBuilder do
|
describe SuggestedTopicsBuilder do
|
||||||
|
|
||||||
let!(:topic) { Fabricate(:topic) }
|
let(:topic) { Fabricate(:topic) }
|
||||||
|
let(:builder) { SuggestedTopicsBuilder.new(topic) }
|
||||||
let!(:builder) { SuggestedTopicsBuilder.new(topic) }
|
|
||||||
|
|
||||||
before do
|
before do
|
||||||
SiteSetting.stubs(:suggested_topics).returns(5)
|
SiteSetting.stubs(:suggested_topics).returns(5)
|
||||||
end
|
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
|
it "has the correct defaults" do
|
||||||
builder.excluded_topic_ids.include?(topic.id).should be_true
|
builder.excluded_topic_ids.include?(topic.id).should be_true
|
||||||
builder.results_left.should == 5
|
builder.results_left.should == 5
|
||||||
|
|
Reference in a new issue