BUGFIX: the /top page now shows the first non-empty period

app/controllers/list_controller.rb

@@ -175,7 +175,7 @@ class ListController < ApplicationController
       top_options.merge!(options) if options
       top_options[:per_page] = SiteSetting.topics_per_period_in_top_page
       user = list_target_user
-      list = TopicQuery.new(user, top_options).public_send("list_top_#{period}")
+      list = TopicQuery.new(user, top_options).list_top_for(period)
       list.more_topics_url = construct_next_url_with(top_options)
       list.prev_topics_url = construct_prev_url_with(top_options)
       respond(list)
@@ -325,12 +325,22 @@ class ListController < ApplicationController
     top
   end
 
-  def self.best_period_for(date)
+  def self.best_period_for(previous_visit_at)
+    ListController.best_periods_for(previous_visit_at).each do |period|
+      return period if TopTopic.where("#{period}_score > 0").count >= SiteSetting.topics_per_period_in_top_page
+    end
+    # default period is yearly
+    :yearly
+  end
+
+  def self.best_periods_for(date)
     date ||= 1.year.ago
-    return :yearly  if date < 180.days.ago
+    periods = []
-    return :monthly if date <  35.days.ago
+    periods << :daily if date > 8.days.ago
-    return :weekly  if date <   8.days.ago
+    periods << :weekly if date > 35.days.ago
-    :daily
+    periods << :monthly if date > 180.days.ago
+    periods << :yearly
+    periods
   end
 
 end

app/models/site_setting.rb

@@ -87,10 +87,11 @@ class SiteSetting < ActiveRecord::Base
   end
 
   def self.has_enough_topics_to_redirect_to_top
-    Topic.listable_topics
+    TopTopic.periods.each do |period|
-         .visible
+      return true if TopTopic.where("#{period}_score > 0").count >= SiteSetting.topics_per_period_in_top_page
-         .where('topics.id NOT IN (SELECT COALESCE(topic_id, 0) FROM categories)')
+    end
-         .count > SiteSetting.topics_per_period_in_top_page
+    # nothing
+    false
   end
 
 end

app/models/topic.rb

@@ -108,14 +108,12 @@ class Topic < ActiveRecord::Base
   attr_accessor :include_last_poster
 
   # The regular order
-  scope :topic_list_order, lambda { order('topics.bumped_at desc') }
+  scope :topic_list_order, -> { order('topics.bumped_at desc') }
 
   # Return private message topics
-  scope :private_messages, lambda {
+  scope :private_messages, -> { where(archetype: Archetype.private_message) }
-    where(archetype: Archetype.private_message)
-  }
 
-  scope :listable_topics, lambda { where('topics.archetype <> ?', [Archetype.private_message]) }
+  scope :listable_topics, -> { where('topics.archetype <> ?', [Archetype.private_message]) }
 
   scope :by_newest, -> { order('topics.created_at desc, topics.id desc') }
 
@@ -123,16 +121,15 @@ class Topic < ActiveRecord::Base
 
   scope :created_since, lambda { |time_ago| where('created_at > ?', time_ago) }
 
-  scope :secured, lambda {|guardian=nil|
+  scope :secured, lambda { |guardian=nil|
     ids = guardian.secure_category_ids if guardian
 
     # Query conditions
-    condition =
+    condition = if ids.present?
-      if ids.present?
+      ["NOT c.read_restricted or c.id in (:cats)", cats: ids]
-        ["NOT c.read_restricted or c.id in (:cats)", cats: ids]
+    else
-      else
+      ["NOT c.read_restricted"]
-        ["NOT c.read_restricted"]
+    end
-      end
 
     where("category_id IS NULL OR category_id IN (
            SELECT c.id FROM categories c

lib/topic_query.rb

@@ -96,12 +96,6 @@ class TopicQuery
     end
   end
 
-  TopTopic.periods.each do |period|
-    define_method("list_top_#{period}") do
-      list_top_for(period)
-    end
-  end
-
   def list_topics_by(user)
     create_list(:user_topics) do |topics|
       topics.where(user_id: user.id)

spec/controllers/list_controller_spec.rb

@@ -215,28 +215,28 @@ describe ListController do
     end
   end
 
-  describe "best_period_for" do
+  describe "best_periods_for" do
 
     it "returns yearly for more than 180 days" do
-      ListController.best_period_for(nil).should == :yearly
+      ListController.best_periods_for(nil).should == [:yearly]
-      ListController.best_period_for(180.days.ago).should == :yearly
+      ListController.best_periods_for(180.days.ago).should == [:yearly]
     end
 
-    it "returns monthly when less than 180 days and more than 35 days" do
+    it "includes monthly when less than 180 days and more than 35 days" do
       (35...180).each do |date|
-        ListController.best_period_for(date.days.ago).should == :monthly
+        ListController.best_periods_for(date.days.ago).should == [:monthly, :yearly]
       end
     end
 
-    it "returns weekly when less than 35 days and more than 8 days" do
+    it "includes weekly when less than 35 days and more than 8 days" do
       (8...35).each do |date|
-        ListController.best_period_for(date.days.ago).should == :weekly
+        ListController.best_periods_for(date.days.ago).should == [:weekly, :monthly, :yearly]
       end
     end
 
-    it "returns daily when less than 8 days" do
+    it "includes daily when less than 8 days" do
       (0...8).each do |date|
-        ListController.best_period_for(date.days.ago).should == :daily
+        ListController.best_periods_for(date.days.ago).should == [:daily, :weekly, :monthly, :yearly]
       end
     end