Merge pull request #4487 from cpradio/use-top-default-for-new-users

FEATURE: Use the top period default for users who have been inactive or are new

app/models/site_setting.rb

@@ -97,11 +97,10 @@ class SiteSetting < ActiveRecord::Base
     ].flatten.to_set
   end
 
-  def self.min_redirected_to_top_period
+  def self.min_redirected_to_top_period(duration)
-    TopTopic.sorted_periods.each do |p|
+    period = ListController.best_period_for(duration)
-      period = p[0]
     return period if TopTopic.topics_per_period(period) >= SiteSetting.topics_per_period_in_top_page
-    end
+
     # not enough topics
     nil
   end

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
+    return unless period = SiteSetting.min_redirected_to_top_period(1.days.ago)
 
     if !user.seen_before? || (user.trust_level == 0 && !redirected_to_top_yet?)
       update_last_redirected_to_top!

spec/models/site_setting_spec.rb

@@ -69,6 +69,40 @@ describe SiteSetting do
     end
   end
 
+  describe "min_redirected_to_top_period" do
+
+    context "has_enough_top_topics" 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)
+        TopTopic.refresh!
+      end
+
+      it "should_return_a_time_period" do
+        expect(SiteSetting.min_redirected_to_top_period(1.days.ago)).to eq(:daily)
+      end
+
+    end
+
+    context "does_not_have_enough_top_topics" 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
+
+      it "should_return_a_time_period" do
+        expect(SiteSetting.min_redirected_to_top_period(1.days.ago)).to eq(nil)
+      end
+
+    end
+
+  end
+
   describe "scheme" do
     before do
       SiteSetting.force_https = true