diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 58d894e2e..26d504da2 100644 --- a/app/controllers/list_controller.rb +++ b/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 - return :monthly if date < 35.days.ago - return :weekly if date < 8.days.ago - :daily + periods = [] + periods << :daily if date > 8.days.ago + periods << :weekly if date > 35.days.ago + periods << :monthly if date > 180.days.ago + periods << :yearly + periods end end diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index f1636335b..252213e31 100644 --- a/app/models/site_setting.rb +++ b/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 - .visible - .where('topics.id NOT IN (SELECT COALESCE(topic_id, 0) FROM categories)') - .count > SiteSetting.topics_per_period_in_top_page + TopTopic.periods.each do |period| + return true if TopTopic.where("#{period}_score > 0").count >= SiteSetting.topics_per_period_in_top_page + end + # nothing + false end end diff --git a/app/models/topic.rb b/app/models/topic.rb index 9360b9847..154f20303 100644 --- a/app/models/topic.rb +++ b/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 { - where(archetype: Archetype.private_message) - } + scope :private_messages, -> { 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 = - if ids.present? - ["NOT c.read_restricted or c.id in (:cats)", cats: ids] - else - ["NOT c.read_restricted"] - end + condition = if ids.present? + ["NOT c.read_restricted or c.id in (:cats)", cats: ids] + else + ["NOT c.read_restricted"] + end where("category_id IS NULL OR category_id IN ( SELECT c.id FROM categories c diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 4c90ba463..6a14e37ee 100644 --- a/lib/topic_query.rb +++ b/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) diff --git a/spec/controllers/list_controller_spec.rb b/spec/controllers/list_controller_spec.rb index c4f681dd2..9eb21789e 100644 --- a/spec/controllers/list_controller_spec.rb +++ b/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_period_for(180.days.ago).should == :yearly + ListController.best_periods_for(nil).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