diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index e4e037170ca..e680ce8d683 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -171,7 +171,7 @@ class ListController < ApplicationController def top(options=nil) options ||= {} - period = ListController.best_period_for(current_user.try(:previous_visit_at), options[:category], SiteSetting.top_page_default_timeframe.to_sym) + period = ListController.best_period_for(current_user.try(:previous_visit_at), options[:category]) send("top_#{period}", options) end @@ -327,14 +327,14 @@ class ListController < ApplicationController exclude_category_ids.pluck(:id) end - def self.best_period_for(previous_visit_at, category_id=nil, default_period=nil) + def self.best_period_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 end - - default_period + # default period is yearly + 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 2a2c04ac468..dcc74ecfc31 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -97,8 +97,13 @@ class SiteSetting < ActiveRecord::Base ].flatten.to_set end - def self.min_redirected_to_top_period(duration=nil) - return ListController.best_period_for(duration) + def self.min_redirected_to_top_period + TopTopic.sorted_periods.each do |p| + period = p[0] + return period if TopTopic.topics_per_period(period) >= SiteSetting.topics_per_period_in_top_page + end + # not enough topics + nil end def self.email_polling_enabled? diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 99be107d215..1071026c183 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -90,7 +90,7 @@ class UserOption < ActiveRecord::Base # top must be in the top_menu return unless SiteSetting.top_menu =~ /(^|\|)top(\||$)/i # not enough topics - return unless period = SiteSetting.min_redirected_to_top_period(1.days.ago) + return unless period = SiteSetting.min_redirected_to_top_period if !user.seen_before? || (user.trust_level == 0 && !redirected_to_top_yet?) update_last_redirected_to_top! diff --git a/spec/controllers/list_controller_spec.rb b/spec/controllers/list_controller_spec.rb index 03768b08536..c07c6ad2f73 100644 --- a/spec/controllers/list_controller_spec.rb +++ b/spec/controllers/list_controller_spec.rb @@ -226,63 +226,6 @@ describe ListController do end end - describe "best_period_for" do - - context "has_top_topics_and_not_seen_recently" do - - SiteSetting.topics_per_period_in_top_page = 2 - SiteSetting.top_page_default_timeframe = 'daily' - - let!(:t1) { Fabricate(:topic) } - let!(:t2) { Fabricate(:topic) } - let!(:t3) { Fabricate(:topic) } - - before do - TopTopic.refresh! - end - - it "should_return_a_time_period" do - expect(ListController.best_period_for(nil, nil, SiteSetting.top_page_default_timeframe.to_sym)).not_to eq(nil) - end - - end - - context "has_top_topics_and_seen_recently" do - - SiteSetting.topics_per_period_in_top_page = 2 - SiteSetting.top_page_default_timeframe = 'daily' - - let!(:t1) { Fabricate(:topic) } - let!(:t2) { Fabricate(:topic) } - let!(:t3) { Fabricate(:topic) } - - before do - TopTopic.refresh! - end - - it "should_return_a_time_period" do - expect(ListController.best_period_for(1.month.ago, nil, SiteSetting.top_page_default_timeframe.to_sym)).not_to eq(nil) - end - - end - - context "does_not_have_top_topics" do - - SiteSetting.topics_per_period_in_top_page = 2 - SiteSetting.top_page_default_timeframe = 'daily' - - before do - TopTopic.refresh! - end - - it "should_not_return_a_time_period" do - expect(ListController.best_period_for(1.month.ago, nil, nil)).to eq(nil) - end - - end - - end - describe "best_periods_for" do it "returns yearly for more than 180 days" do