diff --git a/app/assets/javascripts/discourse/controllers/discovery_top_controller.js b/app/assets/javascripts/discourse/controllers/discovery_top_controller.js index 6b28a1fe817..37366cc062c 100644 --- a/app/assets/javascripts/discourse/controllers/discovery_top_controller.js +++ b/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') }); diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index 4c279d563de..7f1694f0869 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/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)); diff --git a/app/assets/javascripts/discourse/templates/discovery/top.js.handlebars b/app/assets/javascripts/discourse/templates/discovery/top.js.handlebars index f78add2d5f5..ccf1cf92305 100644 --- a/app/assets/javascripts/discourse/templates/discovery/top.js.handlebars +++ b/app/assets/javascripts/discourse/templates/discovery/top.js.handlebars @@ -1,7 +1,7 @@
- {{#if redirectedToTopPageReason}} + {{#if currentUser.should_be_redirected_to_top}}
- {{redirectedToTopPageReason}} + {{currentUser.redirected_to_top_reason}}
{{/if}} {{#if content.yearly}} diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 8365c3bf6e5..2e5c3116a6d 100644 --- a/app/models/site_setting.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 4509e236147..d0ecc1b37f1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -93,7 +93,7 @@ class User < ActiveRecord::Base ALWAYS = -1 LAST_VISIT = -2 end - + GLOBAL_USERNAME_LENGTH_RANGE = 3..15 def self.username_length @@ -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 diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index b215cfa5e8c..9becf6ab25c 100644 --- a/app/serializers/current_user_serializer.rb +++ b/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 diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 914a8e76527..0f2f1b6999d 100644 --- a/app/serializers/site_serializer.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a7e2bfd6f4d..cda23859b19 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -300,7 +300,7 @@ en: uploaded_avatar_empty: "Add a custom picture" upload_title: "Upload your picture" image_is_not_a_square: "Warning: we've cropped your image; it is not square." - + change_profile_background: title: "Change Profile Background" @@ -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, your browser is too old to work on this Discourse forum. Please upgrade your browser.' diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0c3b3a20e9a..ec93a0d6419 100644 --- a/config/locales/server.en.yml +++ b/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}" diff --git a/config/site_settings.yml b/config/site_settings.yml index 0eb69d3a6a6..439820f2400 100644 --- a/config/site_settings.yml +++ b/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: diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e24ca1a1c2c..65554f4cd6e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -174,33 +174,33 @@ describe User do myself.reload.username.should == 'HanSolo' end end - + describe 'allow custom minimum username length from site settings' do before do @custom_min = User::GLOBAL_USERNAME_LENGTH_RANGE.begin - 1 SiteSetting.stubs("min_username_length").returns(@custom_min) end - + it 'should allow a shorter username than default' do result = user.change_username('a' * @custom_min) result.should_not be_false end - + it 'should not allow a shorter username than limit' do result = user.change_username('a' * (@custom_min - 1)) result.should be_false - end - + end + it 'should not allow a longer username than limit' do result = user.change_username('a' * (User::GLOBAL_USERNAME_LENGTH_RANGE.end + 1)) - result.should be_false + result.should be_false end - + it 'should use default length for validation if enforce_global_nicknames is true' do SiteSetting.stubs('enforce_global_nicknames').returns(true) - + User::username_length.begin.should == User::GLOBAL_USERNAME_LENGTH_RANGE.begin - User::username_length.end.should == User::GLOBAL_USERNAME_LENGTH_RANGE.end + User::username_length.end.should == User::GLOBAL_USERNAME_LENGTH_RANGE.end end end end @@ -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 diff --git a/test/javascripts/models/user_test.js b/test/javascripts/models/user_test.js index 13aa9b88ba5..f3a7130b773 100644 --- a/test/javascripts/models/user_test.js +++ b/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);