diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index f05e1a4a632..b8353663509 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -433,24 +433,24 @@ class ListController < ApplicationController end def self.best_period_with_topics_for(previous_visit_at, category_id = nil, default_period = SiteSetting.top_page_default_timeframe) - best_periods_for(previous_visit_at, default_period.to_sym).each do |period| + best_periods_for(previous_visit_at, default_period.to_sym).find do |period| top_topics = TopTopic.where("#{period}_score > 0") top_topics = top_topics.joins(:topic).where("topics.category_id = ?", category_id) if category_id top_topics = top_topics.limit(SiteSetting.topics_per_period_in_top_page) - return period if top_topics.count == SiteSetting.topics_per_period_in_top_page + top_topics.count == SiteSetting.topics_per_period_in_top_page end - - nil end def self.best_periods_for(date, default_period = :all) - date ||= 1.year.ago + return [default_period, :all].uniq unless date + periods = [] - periods << default_period if :all != default_period - periods << :daily if :daily != default_period && date > 8.days.ago - periods << :weekly if :weekly != default_period && date > 35.days.ago - periods << :monthly if :monthly != default_period && date > 180.days.ago - periods << :yearly if :yearly != default_period + periods << :daily if date > (1.week + 1.day).ago + periods << :weekly if date > (1.month + 1.week).ago + periods << :monthly if date > (3.months + 3.weeks).ago + periods << :quarterly if date > (1.year + 1.month).ago + periods << :yearly if date > 3.years.ago + periods << :all periods end diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 987116c47ac..587c6807d09 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -566,52 +566,22 @@ RSpec.describe ListController do end describe "best_periods_for" do - it "returns yearly for more than 180 days" do - expect(ListController.best_periods_for(nil, :all)).to eq([:yearly]) - expect(ListController.best_periods_for(180.days.ago, :all)).to eq([:yearly]) + it "works" do + expect(ListController.best_periods_for(nil)).to eq([:all]) + expect(ListController.best_periods_for(5.years.ago)).to eq([:all]) + expect(ListController.best_periods_for(2.years.ago)).to eq([:yearly, :all]) + expect(ListController.best_periods_for(6.months.ago)).to eq([:quarterly, :yearly, :all]) + expect(ListController.best_periods_for(2.months.ago)).to eq([:monthly, :quarterly, :yearly, :all]) + expect(ListController.best_periods_for(2.weeks.ago)).to eq([:weekly, :monthly, :quarterly, :yearly, :all]) + expect(ListController.best_periods_for(2.days.ago)).to eq([:daily, :weekly, :monthly, :quarterly, :yearly, :all]) end - it "includes monthly when less than 180 days and more than 35 days" do - (35...180).each do |date| - expect(ListController.best_periods_for(date.days.ago, :all)).to eq([:monthly, :yearly]) - end - end - - it "includes weekly when less than 35 days and more than 8 days" do - (8...35).each do |date| - expect(ListController.best_periods_for(date.days.ago, :all)).to eq([:weekly, :monthly, :yearly]) - end - end - - it "includes daily when less than 8 days" do - (0...8).each do |date| - expect(ListController.best_periods_for(date.days.ago, :all)).to eq([:daily, :weekly, :monthly, :yearly]) - end - end - - it "returns default even for more than 180 days" do - expect(ListController.best_periods_for(nil, :monthly)).to eq([:monthly, :yearly]) - expect(ListController.best_periods_for(180.days.ago, :monthly)).to eq([:monthly, :yearly]) - end - - it "returns default even when less than 180 days and more than 35 days" do - (35...180).each do |date| - expect(ListController.best_periods_for(date.days.ago, :weekly)).to eq([:weekly, :monthly, :yearly]) - end - end - - it "returns default even when less than 35 days and more than 8 days" do - (8...35).each do |date| - expect(ListController.best_periods_for(date.days.ago, :daily)).to eq([:daily, :weekly, :monthly, :yearly]) - end - end - - it "doesn't return default when set to all" do - expect(ListController.best_periods_for(nil, :all)).to eq([:yearly]) - end - - it "doesn't return value twice when matches default" do - expect(ListController.best_periods_for(nil, :yearly)).to eq([:yearly]) + it "supports default period" do + expect(ListController.best_periods_for(nil, :yearly)).to eq([:yearly, :all]) + expect(ListController.best_periods_for(nil, :quarterly)).to eq([:quarterly, :all]) + expect(ListController.best_periods_for(nil, :monthly)).to eq([:monthly, :all]) + expect(ListController.best_periods_for(nil, :weekly)).to eq([:weekly, :all]) + expect(ListController.best_periods_for(nil, :daily)).to eq([:daily, :all]) end end