BUGFIX: don't show redirect reason if you aren't redirected

Move the redirect to top page logic server-side and make sure the reason
is not shown when top is not in the navigation menu (top_menu).

app/assets/javascripts/discourse/controllers/discovery_top_controller.js

@@ -19,20 +19,5 @@ Discourse.DiscoveryTopController = Discourse.DiscoveryController.extend({
     }
   },
 
-  redirectedToTopPageReason: function() {
-    // no need for a reason if the default homepage is 'top'
-    if (Discourse.Utilities.defaultHomepage() === 'top') { return null; }
-    // check if the user is authenticated
-    if (Discourse.User.current()) {
-      if (Discourse.User.currentProp('trust_level') === 0) {
-        return I18n.t('filters.top.redirect_reasons.new_user');
-      } else if (!Discourse.User.currentProp('hasBeenSeenInTheLastMonth')) {
-        return I18n.t('filters.top.redirect_reasons.not_seen_in_a_month');
-      }
-    }
-    // no reason detected
-    return null;
-  }.property(),
-
   hasDisplayedAllTopLists: Em.computed.and('content.yearly', 'content.monthly', 'content.weekly', 'content.daily')
 });

app/assets/javascripts/discourse/models/user.js

@@ -373,16 +373,6 @@ Discourse.User = Discourse.Model.extend({
     });
   },
 
-  hasNotBeenSeenInTheLastMonth: function() {
-    return moment().diff(moment(this.get("last_seen_at")), "month", true) >= 1.0;
-  }.property("last_seen_at"),
-
-  shouldBeRedirectToTopPage: function() {
-    if (this.get("trust_level") > 0) { return false; }
-    var duration = Discourse.SiteSettings.redirect_new_users_to_top_page_duration;
-    return moment().diff(moment(this.get("created_at")), "days", true) < duration;
-  }.property("trust_level", "created_at"),
-
   /**
     Homepage of the user
 
@@ -390,18 +380,8 @@ Discourse.User = Discourse.Model.extend({
     @type {String}
   **/
   homepage: function() {
-    // when there are enough topics, /top is the default for
-    //   - new users
-    //   - long-time-no-see user (ie. > 1 month)
-    if (Discourse.Site.currentProp("has_enough_topic_to_redirect_to_top_page")) {
-      if (Discourse.SiteSettings.top_menu.indexOf("top") >= 0) {
-        if (this.get("shouldBeRedirectToTopPage") || this.get("hasNotBeenSeenInTheLastMonth")) {
-          return "top";
-        }
-      }
-    }
-    return Discourse.Utilities.defaultHomepage();
-  }.property("shouldBeRedirectToTopPage", "hasNotBeenSeenInTheLastMonth"),
+    return this.get("should_be_redirected_to_top") ? "top" : Discourse.Utilities.defaultHomepage();
+  }.property("should_be_redirected_to_top"),
 
   updateMutedCategories: function() {
     this.set("mutedCategories", Discourse.Category.findByIds(this.muted_category_ids));

app/assets/javascripts/discourse/templates/discovery/top.js.handlebars

@@ -1,7 +1,7 @@
 <div class="top-lists">
-  {{#if redirectedToTopPageReason}}
+  {{#if currentUser.should_be_redirected_to_top}}
     <div class="alert alert-info">
-      {{redirectedToTopPageReason}}
+      {{currentUser.redirected_to_top_reason}}
     </div>
   {{/if}}
   {{#if content.yearly}}

app/models/site_setting.rb

@@ -98,6 +98,13 @@ class SiteSetting < ActiveRecord::Base
     use_https? ? "https" : "http"
   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
+  end
+
 end
 
 # == Schema Information

app/models/user.rb

@@ -553,6 +553,23 @@ class User < ActiveRecord::Base
     @lq ||= LeaderRequirements.new(self)
   end
 
+  def should_be_redirected_to_top
+    redirected_to_top_reason.present?
+  end
+
+  def redirected_to_top_reason
+    # top must be in the top_menu
+    return unless SiteSetting.top_menu =~ /top/i
+    # there should be enough topics
+    return unless SiteSetting.has_enough_topics_to_redirect_to_top
+    # new users
+    return I18n.t('redirected_to_top_reasons.new_user') if trust_level == 0 &&
+      created_at > SiteSetting.redirect_new_users_to_top_page_duration.days.ago
+    # long-time-no-see user
+    return I18n.t('redirected_to_top_reasons.not_seen_in_a_month') if last_seen_at < 1.month.ago
+    nil
+  end
+
   protected
 
   def cook

app/serializers/current_user_serializer.rb

@@ -17,7 +17,9 @@ class CurrentUserSerializer < BasicUserSerializer
              :can_edit,
              :can_invite_to_forum,
              :no_password,
-             :can_delete_account
+             :can_delete_account,
+             :should_be_redirected_to_top,
+             :redirected_to_top_reason
 
   def include_site_flagged_posts_count?
     object.staff?
@@ -63,4 +65,8 @@ class CurrentUserSerializer < BasicUserSerializer
     true
   end
 
+  def include_redirected_to_top_reason?
+    object.should_be_redirected_to_top
+  end
+
 end

app/serializers/site_serializer.rb

@@ -9,8 +9,7 @@ class SiteSerializer < ApplicationSerializer
              :top_menu_items,
              :anonymous_top_menu_items,
              :uncategorized_category_id, # this is hidden so putting it here
-             :is_readonly,
-             :has_enough_topic_to_redirect_to_top_page
+             :is_readonly
 
   has_many :categories, serializer: BasicCategorySerializer, embed: :objects
   has_many :post_action_types, embed: :objects
@@ -51,12 +50,4 @@ class SiteSerializer < ApplicationSerializer
     Discourse.readonly_mode?
   end
 
-  def has_enough_topic_to_redirect_to_top_page
-    Topic.listable_topics
-         .visible
-         .includes(:category).references(:category)
-         .where('COALESCE(categories.topic_id, 0) <> topics.id')
-         .count > SiteSetting.topics_per_period_in_top_page
-  end
-
 end

config/locales/client.en.yml

@@ -1205,9 +1205,6 @@ en:
         this_week: "This week"
         today: "Today"
         other_periods: "see more top topics"
-        redirect_reasons:
-          new_user: "Welcome! As a new visitor, this will be your homepage until you've had some time to look around at the forum. Here's some of the most popular topics from the past."
-          not_seen_in_a_month: "Welcome back! We haven't seen you in a while. These are the top discussion topics since you've been away."
 
     browser_update: 'Unfortunately, <a href="http://www.discourse.org/faq/#browser">your browser is too old to work on this Discourse forum</a>. Please <a href="http://browsehappy.com">upgrade your browser</a>.'
 

config/locales/server.en.yml

@@ -871,6 +871,10 @@ en:
   most_recent_poster: "Most Recent Poster"
   frequent_poster: "Frequent Poster"
 
+  redirected_to_top_reasons:
+    new_user: "Welcome! As a new visitor, this will be your homepage until you've had some time to look around at the forum. Here's some of the most popular topics from the past."
+    not_seen_in_a_month: "Welcome back! We haven't seen you in a while. These are the top discussion topics since you've been away."
+
   move_posts:
     new_topic_moderator_post:
       one: "I moved a post to a new topic: %{topic_link}"

config/site_settings.yml

@@ -71,7 +71,6 @@ basic:
   topics_per_period_in_top_page:
     default: 50
   redirect_new_users_to_top_page_duration:
-    client: true
     default: 7
 
 users:

spec/models/user_spec.rb

@@ -1108,4 +1108,69 @@ describe User do
       end
     end
   end
+
+  describe "should_be_redirected_to_top" do
+    let!(:user) { Fabricate(:user) }
+
+    it "should be redirected to top when there is a reason to" do
+      user.expects(:redirected_to_top_reason).returns("42")
+      user.should_be_redirected_to_top.should == true
+    end
+
+    it "should not be redirected to top when there is no reason to" do
+      user.expects(:redirected_to_top_reason).returns(nil)
+      user.should_be_redirected_to_top.should == false
+    end
+
+  end
+
+  describe "redirected_to_top_reason" do
+    let!(:user) { Fabricate(:user) }
+
+    it "should have no reason when top is not in the top_menu" do
+      SiteSetting.expects(:top_menu).returns("latest")
+      user.redirected_to_top_reason.should == nil
+    end
+
+    it "should have no reason when there isn't enough topics" do
+      SiteSetting.expects(:top_menu).returns("latest|top")
+      SiteSetting.expects(:has_enough_topics_to_redirect_to_top).returns(false)
+      user.redirected_to_top_reason.should == nil
+    end
+
+    describe "new users" do
+      before do
+        user.expects(:trust_level).returns(0)
+        user.stubs(:last_seen_at).returns(1.day.ago)
+        SiteSetting.expects(:top_menu).returns("latest|top")
+        SiteSetting.expects(:has_enough_topics_to_redirect_to_top).returns(true)
+        SiteSetting.expects(:redirect_new_users_to_top_page_duration).returns(7)
+      end
+
+      it "should have a reason for newly created user" do
+        user.expects(:created_at).returns(5.days.ago)
+        user.redirected_to_top_reason.should == I18n.t('redirected_to_top_reasons.new_user')
+      end
+
+      it "should not have a reason for newly created user" do
+        user.expects(:created_at).returns(10.days.ago)
+        user.redirected_to_top_reason.should == nil
+      end
+    end
+
+    describe "old users" do
+      before do
+        user.stubs(:trust_level).returns(1)
+        SiteSetting.expects(:top_menu).returns("latest|top")
+        SiteSetting.expects(:has_enough_topics_to_redirect_to_top).returns(true)
+      end
+
+      it "should have a reason for long-time-no-see users" do
+        user.expects(:last_seen_at).returns(2.months.ago)
+        user.redirected_to_top_reason.should == I18n.t('redirected_to_top_reasons.not_seen_in_a_month')
+      end
+    end
+
+  end
+
 end

test/javascripts/models/user_test.js

@@ -26,63 +26,17 @@ test("isAllowedToUploadAFile", function() {
   ok(user.isAllowedToUploadAFile("image"), "moderator can always upload a file");
 });
 
-test("homepage when top is disabled", function() {
-  var newUser = Discourse.User.create({ trust_level: 0, last_seen_at: moment() }),
-      oldUser = Discourse.User.create({ trust_level: 1, last_seen_at: moment() }),
-      defaultHomepage = Discourse.Utilities.defaultHomepage();
+test("homepage", function() {
+  var user = Discourse.User.create({ should_be_redirected_to_top: false });
+  var defaultHomepage = Discourse.Utilities.defaultHomepage();
 
-  Discourse.SiteSettings.top_menu = "latest";
+  equal(user.get("homepage"), defaultHomepage, "user's homepage is default when not redirected");
 
-  equal(newUser.get("homepage"), defaultHomepage, "new user's homepage is default when top is disabled");
-  equal(oldUser.get("homepage"), defaultHomepage, "old user's homepage is default when top is disabled");
+  user.set("should_be_redirected_to_top", true);
 
-  oldUser.set("last_seen_at", moment().subtract('month', 2));
-  equal(oldUser.get("homepage"), defaultHomepage, "long-time-no-see old user's homepage is default when top is disabled");
+  equal(user.get("homepage"), "top", "user's homepage is top when redirected");
 });
 
-test("homepage when top is enabled and not enough topics", function() {
-  var newUser = Discourse.User.create({ trust_level: 0, last_seen_at: moment() }),
-      oldUser = Discourse.User.create({ trust_level: 1, last_seen_at: moment() }),
-      defaultHomepage = Discourse.Utilities.defaultHomepage();
-
-  Discourse.SiteSettings.top_menu = "latest|top";
-  Discourse.Site.currentProp("has_enough_topic_to_redirect_to_top_page", false);
-
-  equal(newUser.get("homepage"), defaultHomepage, "new user's homepage is default");
-  equal(oldUser.get("homepage"), defaultHomepage, "old user's homepage is default");
-
-  oldUser.set("last_seen_at", moment().subtract('month', 2));
-  equal(oldUser.get("homepage"), defaultHomepage, "long-time-no-see old user's homepage is default");
-});
-
-test("homepage when top is enabled and has enough topics", function() {
-  var newUser = Discourse.User.create({ trust_level: 0, last_seen_at: moment(), created_at: moment().subtract("day", 6) }),
-      oldUser = Discourse.User.create({ trust_level: 1, last_seen_at: moment(), created_at: moment().subtract("month", 2) }),
-      defaultHomepage = Discourse.Utilities.defaultHomepage();
-
-  Discourse.SiteSettings.top_menu = "latest|top";
-  Discourse.SiteSettings.redirect_new_users_to_top_page_duration = 7;
-  Discourse.Site.currentProp("has_enough_topic_to_redirect_to_top_page", true);
-
-  equal(newUser.get("homepage"), "top", "new user's homepage is top when top is enabled");
-  equal(oldUser.get("homepage"), defaultHomepage, "old user's homepage is default when top is enabled");
-
-  oldUser.set("last_seen_at", moment().subtract('month', 2));
-  equal(oldUser.get("homepage"), "top", "long-time-no-see old user's homepage is top when top is enabled");
-});
-
-test("new user's homepage when top is enabled, there's enough topics and duration is over", function() {
-  var newUser = Discourse.User.create({ trust_level: 0, last_seen_at: moment(), created_at: moment().subtract("month", 1) }),
-      defaultHomepage = Discourse.Utilities.defaultHomepage();
-
-  Discourse.SiteSettings.top_menu = "latest|top";
-  Discourse.SiteSettings.redirect_new_users_to_top_page_duration = 7;
-  Discourse.Site.currentProp("has_enough_topic_to_redirect_to_top_page", true);
-
-  equal(newUser.get("homepage"), defaultHomepage, "new user's homepage is default when redirect duration is over");
-});
-
-
 asyncTestDiscourse("findByUsername", function() {
   expect(3);