diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index e680ce8d683..080c9029e43 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -327,14 +327,20 @@ class ListController < ApplicationController exclude_category_ids.pluck(:id) end - def self.best_period_for(previous_visit_at, category_id=nil) + def self.best_period_with_topics_for(previous_visit_at, category_id=nil) best_periods_for(previous_visit_at).each do |period| top_topics = TopTopic.where("#{period}_score > 0") top_topics = top_topics.joins(:topic).where("topics.category_id = ?", category_id) if category_id - return period if top_topics.count >= SiteSetting.topics_per_period_in_top_page + top_topics = top_topics.limit(SiteSetting.topics_per_period_in_top_page) + return period if top_topics.count == SiteSetting.topics_per_period_in_top_page end - # default period is yearly - SiteSetting.top_page_default_timeframe.to_sym + + false + end + + def self.best_period_for(previous_visit_at, category_id=nil) + best_period_with_topics_for(previous_visit_at, category_id) || + SiteSetting.top_page_default_timeframe.to_sym end def self.best_periods_for(date) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 70b62602eec..ef88e14516d 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -99,8 +99,8 @@ class SiteSetting < ActiveRecord::Base end def self.min_redirected_to_top_period(duration) - period = ListController.best_period_for(duration) - return period if TopTopic.topics_per_period(period) >= SiteSetting.topics_per_period_in_top_page + period = ListController.best_period_with_topics_for(duration) + return period if period # not enough topics nil diff --git a/app/models/top_topic.rb b/app/models/top_topic.rb index d0564c9f7e0..6172f7cf0a0 100644 --- a/app/models/top_topic.rb +++ b/app/models/top_topic.rb @@ -4,12 +4,6 @@ class TopTopic < ActiveRecord::Base belongs_to :topic - def self.topics_per_period(period) - DistributedMemoizer.memoize("#{Discourse.current_hostname}_topics_per_period_#{period}", 1.day) do - TopTopic.where("#{period}_score > 0").count - end.to_i - end - # The top topics we want to refresh often def self.refresh_daily! transaction do diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb index 78589dc4ade..d1905b2c085 100644 --- a/spec/models/site_setting_spec.rb +++ b/spec/models/site_setting_spec.rb @@ -76,7 +76,11 @@ describe SiteSetting do before do SiteSetting.topics_per_period_in_top_page = 2 SiteSetting.top_page_default_timeframe = 'daily' - TopTopic.stubs(:topics_per_period).with(:daily).returns(3) + + 2.times do + TopTopic.create!(daily_score: 2.5) + end + TopTopic.refresh! end @@ -91,7 +95,6 @@ describe SiteSetting do before do SiteSetting.topics_per_period_in_top_page = 20 SiteSetting.top_page_default_timeframe = 'daily' - TopTopic.stubs(:topics_per_period).with(:daily).returns(1) TopTopic.refresh! end