FIX: redirect to top wasn't working

This commit is contained in:
Régis Hanol 2017-10-04 22:08:41 +02:00
parent 4aa30cba2e
commit e212435545
3 changed files with 19 additions and 12 deletions

@ -3,9 +3,12 @@ module Jobs
class UpdateTopRedirection < Jobs::Base class UpdateTopRedirection < Jobs::Base
def execute(args) def execute(args)
if user = User.find_by(id: args[:user_id]) return if args[:user_id].blank? || args[:redirected_at].blank?
user.user_option.update_column(:last_redirected_to_top_at, args[:redirected_at])
end UserOption
.where(user_id: args[:user_id])
.limit(1)
.update_all(last_redirected_to_top_at: args[:redirected_at])
end end
end end

@ -77,7 +77,7 @@ class UserOption < ActiveRecord::Base
$redis.expire(key, delay) $redis.expire(key, delay)
# delay the update # delay the update
Jobs.enqueue_in(delay / 2, :update_top_redirection, user_id: self.id, redirected_at: Time.zone.now) Jobs.enqueue_in(delay / 2, :update_top_redirection, user_id: self.user_id, redirected_at: Time.zone.now)
end end
def should_be_redirected_to_top def should_be_redirected_to_top
@ -92,10 +92,10 @@ class UserOption < ActiveRecord::Base
return if user.trust_level > 0 && user.last_seen_at && user.last_seen_at > 1.month.ago return if user.trust_level > 0 && user.last_seen_at && user.last_seen_at > 1.month.ago
# top must be in the top_menu # top must be in the top_menu
return unless SiteSetting.top_menu =~ /(^|\|)top(\||$)/i return unless SiteSetting.top_menu[/\btop\b/i]
# not enough topics # not enough topics
return unless period = SiteSetting.min_redirected_to_top_period(1.days.ago) return unless period = SiteSetting.min_redirected_to_top_period(1.day.ago)
if !user.seen_before? || (user.trust_level == 0 && !redirected_to_top_yet?) if !user.seen_before? || (user.trust_level == 0 && !redirected_to_top_yet?)
update_last_redirected_to_top! update_last_redirected_to_top!

@ -56,20 +56,20 @@ describe UserOption do
let!(:user) { Fabricate(:user) } let!(:user) { Fabricate(:user) }
it "should have no reason when `SiteSetting.redirect_users_to_top_page` is disabled" do it "should have no reason when `SiteSetting.redirect_users_to_top_page` is disabled" do
SiteSetting.expects(:redirect_users_to_top_page).returns(false) SiteSetting.redirect_users_to_top_page = false
expect(user.user_option.redirected_to_top).to eq(nil) expect(user.user_option.redirected_to_top).to eq(nil)
end end
context "when `SiteSetting.redirect_users_to_top_page` is enabled" do context "when `SiteSetting.redirect_users_to_top_page` is enabled" do
before { SiteSetting.expects(:redirect_users_to_top_page).returns(true) } before { SiteSetting.redirect_users_to_top_page = true }
it "should have no reason when top is not in the `SiteSetting.top_menu`" do it "should have no reason when top is not in the `SiteSetting.top_menu`" do
SiteSetting.expects(:top_menu).returns("latest") SiteSetting.top_menu = "latest"
expect(user.user_option.redirected_to_top).to eq(nil) expect(user.user_option.redirected_to_top).to eq(nil)
end end
context "and when top is in the `SiteSetting.top_menu`" do context "and when top is in the `SiteSetting.top_menu`" do
before { SiteSetting.expects(:top_menu).returns("latest|top") } before { SiteSetting.top_menu = "latest|top" }
it "should have no reason when there are not enough topics" do it "should have no reason when there are not enough topics" do
SiteSetting.expects(:min_redirected_to_top_period).returns(nil) SiteSetting.expects(:min_redirected_to_top_period).returns(nil)
@ -87,8 +87,12 @@ describe UserOption do
end end
it "should have a reason for the first visit" do it "should have a reason for the first visit" do
expect(user.user_option.redirected_to_top).to eq(reason: I18n.t('redirected_to_top_reasons.new_user'), freeze_time do
period: :monthly) delay = SiteSetting.active_user_rate_limit_secs / 2
Jobs.expects(:enqueue_in).with(delay, :update_top_redirection, user_id: user.id, redirected_at: Time.zone.now)
expect(user.user_option.redirected_to_top).to eq(reason: I18n.t('redirected_to_top_reasons.new_user'), period: :monthly)
end
end end
it "should not have a reason for next visits" do it "should not have a reason for next visits" do