From e2124355459efbadc8cae374a5c3bb2a48111e07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Wed, 4 Oct 2017 22:08:41 +0200
Subject: [PATCH] FIX: redirect to top wasn't working
---
app/jobs/regular/update_top_redirection.rb | 9 ++++++---
app/models/user_option.rb | 6 +++---
spec/models/user_option_spec.rb | 16 ++++++++++------
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/app/jobs/regular/update_top_redirection.rb b/app/jobs/regular/update_top_redirection.rb
index 776cdfca7de..592dc6b8402 100644
--- a/app/jobs/regular/update_top_redirection.rb
+++ b/app/jobs/regular/update_top_redirection.rb
@@ -3,9 +3,12 @@ module Jobs
class UpdateTopRedirection < Jobs::Base
def execute(args)
- if user = User.find_by(id: args[:user_id])
- user.user_option.update_column(:last_redirected_to_top_at, args[:redirected_at])
- end
+ return if args[:user_id].blank? || args[:redirected_at].blank?
+
+ UserOption
+ .where(user_id: args[:user_id])
+ .limit(1)
+ .update_all(last_redirected_to_top_at: args[:redirected_at])
end
end
diff --git a/app/models/user_option.rb b/app/models/user_option.rb
index cf8d1359971..6ce2f12ef5e 100644
--- a/app/models/user_option.rb
+++ b/app/models/user_option.rb
@@ -77,7 +77,7 @@ class UserOption < ActiveRecord::Base
$redis.expire(key, delay)
# 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
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
# 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
- 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?)
update_last_redirected_to_top!
diff --git a/spec/models/user_option_spec.rb b/spec/models/user_option_spec.rb
index d494896cef5..832de55276c 100644
--- a/spec/models/user_option_spec.rb
+++ b/spec/models/user_option_spec.rb
@@ -56,20 +56,20 @@ describe UserOption do
let!(:user) { Fabricate(:user) }
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)
end
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
- SiteSetting.expects(:top_menu).returns("latest")
+ SiteSetting.top_menu = "latest"
expect(user.user_option.redirected_to_top).to eq(nil)
end
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
SiteSetting.expects(:min_redirected_to_top_period).returns(nil)
@@ -87,8 +87,12 @@ describe UserOption do
end
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'),
- period: :monthly)
+ freeze_time do
+ 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
it "should not have a reason for next visits" do