From ac0053f491e6604f943ecb7d10f400d7d1d80d4a Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 16 Jul 2018 18:10:22 +1000 Subject: [PATCH] FEATURE: navigate to first post and auto bump category settings ### navigate_to_first_post_after_read setting for categories When enabled on categories logged on users will return to OP after reading the entire category. (useful for documentation categories) ### num_auto_bump_daily Set a number of topics that will automatically bump daily on a category. - Every 15 minutes we will check if any category has this setting - Categories with the setting are shuffled - We exclude pinned, closed, category description and archived topics - Maximum of 1 topic for the list of categories is bumped till limit reached per category - We always try to bump oldest first - Limit is elastic using a RateLimiter that ensures that we only bump N per day Also some minor organisation on category settings Froze strings on category.rb --- .../discourse/models/category.js.es6 | 5 +- .../javascripts/discourse/models/topic.js.es6 | 17 +- .../components/edit-category-settings.hbs | 64 +++++--- app/assets/stylesheets/common/base/modal.scss | 8 +- app/controllers/categories_controller.rb | 1 + app/jobs/scheduled/periodical_updates.rb | 8 + app/models/category.rb | 150 ++++++++++++------ app/serializers/basic_category_serializer.rb | 3 +- config/locales/client.en.yml | 6 +- ..._to_first_post_after_read_to_categories.rb | 5 + spec/models/category_spec.rb | 40 +++++ spec/requests/categories_controller_spec.rb | 9 +- test/javascripts/models/topic-test.js.es6 | 17 ++ 13 files changed, 241 insertions(+), 92 deletions(-) create mode 100644 db/migrate/20180716062405_add_navigate_to_first_post_after_read_to_categories.rb diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index 8a5997f4a14..0afa3fd9bc3 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -115,7 +115,10 @@ const Category = RestModel.extend({ default_view: this.get("default_view"), subcategory_list_style: this.get("subcategory_list_style"), default_top_period: this.get("default_top_period"), - minimum_required_tags: this.get("minimum_required_tags") + minimum_required_tags: this.get("minimum_required_tags"), + navigate_to_first_post_after_read: this.get( + "navigate_to_first_post_after_read" + ) }, type: id ? "PUT" : "POST" }); diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6 index a47192b7e71..68be7f9b016 100644 --- a/app/assets/javascripts/discourse/models/topic.js.es6 +++ b/app/assets/javascripts/discourse/models/topic.js.es6 @@ -212,11 +212,18 @@ const Topic = RestModel.extend({ }.property("url", "last_read_post_number"), lastUnreadUrl: function() { - const postNumber = Math.min( - this.get("last_read_post_number") + 1, - this.get("highest_post_number") - ); - return this.urlForPostNumber(postNumber); + const highest = this.get("highest_post_number"); + const lastRead = this.get("last_read_post_number"); + + if (highest <= lastRead) { + if (this.get("category.navigate_to_first_post_after_read")) { + return this.urlForPostNumber(1); + } else { + return this.urlForPostNumber(lastRead + 1); + } + } else { + return this.urlForPostNumber(lastRead + 1); + } }.property("url", "last_read_post_number", "highest_post_number"), lastPostUrl: function() { diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs index e08271dfff4..778ddcbfda7 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs @@ -1,3 +1,19 @@ +{{#if showPositionInput}} +
+ +
+{{/if}} + +{{#unless showPositionInput}} +
+ {{i18n 'category.position_disabled'}} + {{i18n 'category.position_disabled_click'}} +
+{{/unless}} +
-
+
+
+ +
+
@@ -92,6 +115,13 @@
+
+ +
+ {{#if siteSettings.topic_featured_link_enabled}}
@@ -129,29 +159,6 @@ {{plugin-outlet name="category-email-in" args=(hash category=category)}} {{/if}} -{{#if showPositionInput}} -
- -
-{{/if}} - -{{#unless emailInEnabled}} -
- {{i18n 'category.email_in_disabled'}} - {{i18n 'category.email_in_disabled_click'}} -
-{{/unless}} - -{{#unless showPositionInput}} -
- {{i18n 'category.position_disabled'}} - {{i18n 'category.position_disabled_click'}} -
-{{/unless}} - {{#if siteSettings.tagging_enabled}}
{{plugin-outlet name="category-custom-settings" args=(hash category=category)}} + +{{#unless emailInEnabled}} +
+ {{i18n 'category.email_in_disabled'}} + {{i18n 'category.email_in_disabled_click'}} +
+{{/unless}} diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index aadbf7134b3..465a64c32fc 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -369,12 +369,8 @@ } .edit-category-modal { - .future-date-input, - .num-featured-topics-fields, - .position-fields { - input[type="text"] { - width: 50px; - } + input[type="number"] { + width: 50px; } .subcategory-list-style-field { diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index b9e86e72760..0489bed9aa9 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -283,6 +283,7 @@ class CategoriesController < ApplicationController :subcategory_list_style, :default_top_period, :minimum_required_tags, + :navigate_to_first_post_after_read, custom_fields: [params[:custom_fields].try(:keys)], permissions: [*p.try(:keys)], allowed_tags: [], diff --git a/app/jobs/scheduled/periodical_updates.rb b/app/jobs/scheduled/periodical_updates.rb index ca54bf3b181..5025f999711 100644 --- a/app/jobs/scheduled/periodical_updates.rb +++ b/app/jobs/scheduled/periodical_updates.rb @@ -49,6 +49,14 @@ module Jobs SiteSetting.min_new_topics_time = last_new_topic.created_at.to_i end + auto_bumps = CategoryCustomField.where(name: Category::NUM_AUTO_BUMP_DAILY).pluck(:id) + + if (auto_bumps.length > 0) + auto_bumps.shuffle.each do |category_id| + break if Category.find_by(id: category_id)&.auto_bump_topic! + end + end + nil end diff --git a/app/models/category.rb b/app/models/category.rb index 49f8f4526d3..e3fc9bc7ded 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_dependency 'distributed_cache' class Category < ActiveRecord::Base @@ -9,9 +11,11 @@ class Category < ActiveRecord::Base REQUIRE_TOPIC_APPROVAL = 'require_topic_approval' REQUIRE_REPLY_APPROVAL = 'require_reply_approval' + NUM_AUTO_BUMP_DAILY = 'num_auto_bump_daily' register_custom_field_type(REQUIRE_TOPIC_APPROVAL, :boolean) register_custom_field_type(REQUIRE_REPLY_APPROVAL, :boolean) + register_custom_field_type(NUM_AUTO_BUMP_DAILY, :integer) belongs_to :topic, dependent: :destroy belongs_to :topic_only_relative_url, @@ -365,6 +369,51 @@ class Category < ActiveRecord::Base custom_fields[REQUIRE_REPLY_APPROVAL] end + def num_auto_bump_daily + custom_fields[NUM_AUTO_BUMP_DAILY] + end + + def num_auto_bump_daily=(v) + custom_fields[NUM_AUTO_BUMP_DAILY] = v + end + + def auto_bump_limiter + RateLimiter.new(nil, "auto_bump_limit_#{self.id}", num_auto_bump_daily.to_i, 86400) + end + + def clear_auto_bump_cache! + auto_bump_limiter.clear! + end + + # will automatically bump a single topic + # if number of automatically bumped topics is smaller than threshold + def auto_bump_topic! + return false if num_auto_bump_daily.blank? + + limiter = auto_bump_limiter + return false if !limiter.can_perform? + + id = Topic + .visible + .listable_topics + .where(category_id: self.id) + .where('id <> ?', self.topic_id) + .where('bumped_at < ?', 1.day.ago) + .where('pinned_at IS NULL AND NOT closed AND NOT archived') + .order('bumped_at ASC') + .limit(1) + .pluck(:id).first + + if id + Topic.where(id: id).update_all(bumped_at: Time.zone.now) + limiter.performed! + true + else + false + end + + end + def allowed_tags=(tag_names_arg) DiscourseTagging.add_or_create_tags_by_name(self, tag_names_arg, unlimited: true) end @@ -459,12 +508,10 @@ class Category < ActiveRecord::Base def url url = @@url_cache[self.id] unless url - url = "#{Discourse.base_uri}/c" + url = +"#{Discourse.base_uri}/c" url << "/#{parent_category.slug}" if parent_category_id url << "/#{slug}" - url.freeze - - @@url_cache[self.id] = url + @@url_cache[self.id] = -url end url @@ -545,53 +592,54 @@ end # # Table name: categories # -# id :integer not null, primary key -# name :string(50) not null -# color :string(6) default("AB9364"), not null -# topic_id :integer -# topic_count :integer default(0), not null -# created_at :datetime not null -# updated_at :datetime not null -# user_id :integer not null -# topics_year :integer default(0) -# topics_month :integer default(0) -# topics_week :integer default(0) -# slug :string not null -# description :text -# text_color :string(6) default("FFFFFF"), not null -# read_restricted :boolean default(FALSE), not null -# auto_close_hours :float -# post_count :integer default(0), not null -# latest_post_id :integer -# latest_topic_id :integer -# position :integer -# parent_category_id :integer -# posts_year :integer default(0) -# posts_month :integer default(0) -# posts_week :integer default(0) -# email_in :string -# email_in_allow_strangers :boolean default(FALSE) -# topics_day :integer default(0) -# posts_day :integer default(0) -# allow_badges :boolean default(TRUE), not null -# name_lower :string(50) not null -# auto_close_based_on_last_post :boolean default(FALSE) -# topic_template :text -# contains_messages :boolean -# sort_order :string -# sort_ascending :boolean -# uploaded_logo_id :integer -# uploaded_background_id :integer -# topic_featured_link_allowed :boolean default(TRUE) -# all_topics_wiki :boolean default(FALSE), not null -# show_subcategory_list :boolean default(FALSE) -# num_featured_topics :integer default(3) -# default_view :string(50) -# subcategory_list_style :string(50) default("rows_with_featured_topics") -# default_top_period :string(20) default("all") -# mailinglist_mirror :boolean default(FALSE), not null -# suppress_from_latest :boolean default(FALSE) -# minimum_required_tags :integer default(0) +# id :integer not null, primary key +# name :string(50) not null +# color :string(6) default("AB9364"), not null +# topic_id :integer +# topic_count :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# user_id :integer not null +# topics_year :integer default(0) +# topics_month :integer default(0) +# topics_week :integer default(0) +# slug :string not null +# description :text +# text_color :string(6) default("FFFFFF"), not null +# read_restricted :boolean default(FALSE), not null +# auto_close_hours :float +# post_count :integer default(0), not null +# latest_post_id :integer +# latest_topic_id :integer +# position :integer +# parent_category_id :integer +# posts_year :integer default(0) +# posts_month :integer default(0) +# posts_week :integer default(0) +# email_in :string +# email_in_allow_strangers :boolean default(FALSE) +# topics_day :integer default(0) +# posts_day :integer default(0) +# allow_badges :boolean default(TRUE), not null +# name_lower :string(50) not null +# auto_close_based_on_last_post :boolean default(FALSE) +# topic_template :text +# contains_messages :boolean +# sort_order :string +# sort_ascending :boolean +# uploaded_logo_id :integer +# uploaded_background_id :integer +# topic_featured_link_allowed :boolean default(TRUE) +# all_topics_wiki :boolean default(FALSE), not null +# show_subcategory_list :boolean default(FALSE) +# num_featured_topics :integer default(3) +# default_view :string(50) +# subcategory_list_style :string(50) default("rows_with_featured_topics") +# default_top_period :string(20) default("all") +# mailinglist_mirror :boolean default(FALSE), not null +# suppress_from_latest :boolean default(FALSE) +# minimum_required_tags :integer default(0) +# navigate_to_first_post_after_read :boolean default(FALSE), not null # # Indexes # diff --git a/app/serializers/basic_category_serializer.rb b/app/serializers/basic_category_serializer.rb index 4149a32cf26..6be2df27ff1 100644 --- a/app/serializers/basic_category_serializer.rb +++ b/app/serializers/basic_category_serializer.rb @@ -25,7 +25,8 @@ class BasicCategorySerializer < ApplicationSerializer :default_view, :subcategory_list_style, :default_top_period, - :minimum_required_tags + :minimum_required_tags, + :navigate_to_first_post_after_read has_one :uploaded_logo, embed: :object, serializer: CategoryUploadSerializer has_one :uploaded_background, embed: :object, serializer: CategoryUploadSerializer diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f99b5fec6d6..e14586abc51 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2275,7 +2275,7 @@ en: show_subcategory_list: "Show subcategory list above topics in this category." num_featured_topics: "Number of topics shown on the categories page:" subcategory_num_featured_topics: "Number of featured topics on parent category's page:" - all_topics_wiki: "Make new topics wikis by default." + all_topics_wiki: "Make new topics wikis by default" subcategory_list_style: "Subcategory List Style:" sort_order: "Topic List Sort By:" default_view: "Default Topic List:" @@ -2286,12 +2286,14 @@ en: require_topic_approval: "Require moderator approval of all new topics" require_reply_approval: "Require moderator approval of all new replies" this_year: "this year" - position: "position" + position: "Position:" default_position: "Default Position" position_disabled: "Categories will be displayed in order of activity. To control the order of categories in lists, " position_disabled_click: 'enable the "fixed category positions" setting.' minimum_required_tags: 'Minimum number of tags required in a topic:' parent: "Parent Category" + num_auto_bump_daily: 'Number of open topics to automatically bump daily:' + navigate_to_first_post_after_read: 'Navigate to first post after topics are read' notifications: watching: title: "Watching" diff --git a/db/migrate/20180716062405_add_navigate_to_first_post_after_read_to_categories.rb b/db/migrate/20180716062405_add_navigate_to_first_post_after_read_to_categories.rb new file mode 100644 index 00000000000..03a6cd1896a --- /dev/null +++ b/db/migrate/20180716062405_add_navigate_to_first_post_after_read_to_categories.rb @@ -0,0 +1,5 @@ +class AddNavigateToFirstPostAfterReadToCategories < ActiveRecord::Migration[5.2] + def change + add_column :categories, :navigate_to_first_post_after_read, :bool, null: false, default: false + end +end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 688035ebc86..54828be487a 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -684,4 +684,44 @@ describe Category do it { expect(category.reload.require_reply_approval?).to eq(true) } end end + + describe 'auto bump' do + before do + RateLimiter.enable + end + + after do + RateLimiter.disable + end + + it 'should correctly automatically bump topics' do + freeze_time 1.second.ago + category = Fabricate(:category) + category.clear_auto_bump_cache! + + _post1 = create_post(category: category) + _post2 = create_post(category: category) + _post3 = create_post(category: category) + + time = 1.month.from_now + freeze_time time + + expect(category.auto_bump_topic!).to eq(false) + expect(Topic.where(bumped_at: time).count).to eq(0) + + category.num_auto_bump_daily = 2 + category.save! + + expect(category.auto_bump_topic!).to eq(true) + expect(Topic.where(bumped_at: time).count).to eq(1) + + expect(category.auto_bump_topic!).to eq(true) + expect(Topic.where(bumped_at: time).count).to eq(2) + + expect(category.auto_bump_topic!).to eq(false) + expect(Topic.where(bumped_at: time).count).to eq(2) + + end + end + end diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index dbc7c250357..5c6baa466ac 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -297,24 +297,31 @@ describe CategoriesController do expect(UserHistory.count).to eq(5) # 2 + 3 (bootstrap mode) end - it 'updates per-category approval settings correctly' do + it 'updates per-category settings correctly' do category.custom_fields[Category::REQUIRE_TOPIC_APPROVAL] = false category.custom_fields[Category::REQUIRE_REPLY_APPROVAL] = false + category.custom_fields[Category::NUM_AUTO_BUMP_DAILY] = 0 + + category.navigate_to_first_post_after_read = false category.save! put "/categories/#{category.id}.json", params: { name: category.name, color: category.color, text_color: category.text_color, + navigate_to_first_post_after_read: true, custom_fields: { require_reply_approval: true, require_topic_approval: true, + num_auto_bump_daily: 10 } } category.reload expect(category.require_topic_approval?).to eq(true) expect(category.require_reply_approval?).to eq(true) + expect(category.num_auto_bump_daily).to eq(10) + expect(category.navigate_to_first_post_after_read).to eq(true) end end end diff --git a/test/javascripts/models/topic-test.js.es6 b/test/javascripts/models/topic-test.js.es6 index de606ef6b38..a3e89726cb9 100644 --- a/test/javascripts/models/topic-test.js.es6 +++ b/test/javascripts/models/topic-test.js.es6 @@ -32,6 +32,23 @@ QUnit.test("visited", assert => { ); }); +QUnit.test("lastUnreadUrl", assert => { + const category = Em.Object.create({ + navigate_to_first_post_after_read: true + }); + + const topic = Topic.create({ + id: 101, + highest_post_number: 10, + last_read_post_number: 10, + slug: "hello" + }); + + topic.set("category", category); + + assert.equal(topic.get("lastUnreadUrl"), "/t/hello/101/1"); +}); + QUnit.test("has details", assert => { const topic = Topic.create({ id: 1234 }); const topicDetails = topic.get("details");