From 2de50a616d4120a9718eda7ba9bc08d35b293a0e Mon Sep 17 00:00:00 2001
From: cpradio <forums@cpradio.com>
Date: Tue, 11 Oct 2016 09:55:15 -0400
Subject: [PATCH] FEATURE: Use the top period default for users who have been
 inactive or are new

---
 app/controllers/list_controller.rb       |  8 ++--
 app/models/site_setting.rb               |  9 +---
 app/models/user_option.rb                |  2 +-
 spec/controllers/list_controller_spec.rb | 57 ++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb
index e680ce8d683..e4e037170ca 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])
+    period = ListController.best_period_for(current_user.try(:previous_visit_at), options[:category], SiteSetting.top_page_default_timeframe.to_sym)
     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)
+  def self.best_period_for(previous_visit_at, category_id=nil, default_period=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 is yearly
-    SiteSetting.top_page_default_timeframe.to_sym
+
+    default_period
   end
 
   def self.best_periods_for(date)
diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb
index dcc74ecfc31..2a2c04ac468 100644
--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@@ -97,13 +97,8 @@ class SiteSetting < ActiveRecord::Base
     ].flatten.to_set
   end
 
-  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
+  def self.min_redirected_to_top_period(duration=nil)
+    return ListController.best_period_for(duration)
   end
 
   def self.email_polling_enabled?
diff --git a/app/models/user_option.rb b/app/models/user_option.rb
index 1071026c183..99be107d215 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
+    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!
diff --git a/spec/controllers/list_controller_spec.rb b/spec/controllers/list_controller_spec.rb
index c07c6ad2f73..03768b08536 100644
--- a/spec/controllers/list_controller_spec.rb
+++ b/spec/controllers/list_controller_spec.rb
@@ -226,6 +226,63 @@ 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