From 8348a411249c2815011638b936365487bb62f147 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Thu, 20 Aug 2020 00:35:04 +0530 Subject: [PATCH] FEATURE: add `regular_categories` field in site setting & user option. (#10477) Like "default watching" and "default tracking" categories option now the "regular" categories support is added. It will be useful for sites that are muted by default. The user option will be displayed only if `mute_all_categories_by_default` site setting is enabled. --- .../admin/mixins/setting-component.js | 1 + .../app/controllers/preferences/categories.js | 8 +++- .../javascripts/discourse/app/models/user.js | 40 ++++++++++++------- .../app/templates/preferences/categories.hbs | 14 ++++++- .../admin/site_settings_controller.rb | 4 ++ app/models/category_user.rb | 1 + app/models/user.rb | 2 +- app/serializers/user_serializer.rb | 5 +++ app/services/user_updater.rb | 1 + config/locales/client.en.yml | 2 + config/locales/server.en.yml | 1 + config/site_settings.yml | 3 ++ lib/site_settings/validations.rb | 25 ++++++++++-- lib/topic_query.rb | 3 +- spec/components/topic_query_spec.rb | 5 +++ spec/models/user_spec.rb | 4 ++ spec/serializers/site_serializer_spec.rb | 12 +++++- .../web_hook_user_serializer_spec.rb | 2 +- test/javascripts/fixtures/user_fixtures.js | 3 ++ 19 files changed, 109 insertions(+), 27 deletions(-) diff --git a/app/assets/javascripts/admin/mixins/setting-component.js b/app/assets/javascripts/admin/mixins/setting-component.js index 44609e88cff..3b1ec456689 100644 --- a/app/assets/javascripts/admin/mixins/setting-component.js +++ b/app/assets/javascripts/admin/mixins/setting-component.js @@ -173,6 +173,7 @@ export default Mixin.create({ "default_categories_tracking", "default_categories_muted", "default_categories_watching_first_post", + "default_categories_regular", "default_tags_watching", "default_tags_tracking", "default_tags_muted", diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/categories.js b/app/assets/javascripts/discourse/app/controllers/preferences/categories.js index 13baaf6fd29..b11b85399f2 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/categories.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/categories.js @@ -9,6 +9,7 @@ export default Controller.extend({ this.saveAttrNames = [ "muted_category_ids", + "regular_category_ids", "watched_category_ids", "tracked_category_ids", "watched_first_post_category_ids" @@ -19,10 +20,13 @@ export default Controller.extend({ "model.watchedCategories", "model.watchedFirstPostCategories", "model.trackedCategories", + "model.regularCategories", "model.mutedCategories" ) - selectedCategories(watched, watchedFirst, tracked, muted) { - return [].concat(watched, watchedFirst, tracked, muted).filter(t => t); + selectedCategories(watched, watchedFirst, tracked, regular, muted) { + return [] + .concat(watched, watchedFirst, tracked, regular, muted) + .filter(t => t); }, @discourseComputed diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 2267727bd58..33ef2829ae2 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -332,25 +332,27 @@ const User = RestModel.extend({ var updatedState = {}; - ["muted", "watched", "tracked", "watched_first_post"].forEach(s => { - if (fields === undefined || fields.includes(s + "_category_ids")) { - let prop = - s === "watched_first_post" - ? "watchedFirstPostCategories" - : s + "Categories"; - let cats = this.get(prop); - if (cats) { - let cat_ids = cats.map(c => c.get("id")); - updatedState[s + "_category_ids"] = cat_ids; + ["muted", "regular", "watched", "tracked", "watched_first_post"].forEach( + s => { + if (fields === undefined || fields.includes(s + "_category_ids")) { + let prop = + s === "watched_first_post" + ? "watchedFirstPostCategories" + : s + "Categories"; + let cats = this.get(prop); + if (cats) { + let cat_ids = cats.map(c => c.get("id")); + updatedState[s + "_category_ids"] = cat_ids; - // HACK: denote lack of categories - if (cats.length === 0) { - cat_ids = [-1]; + // HACK: denote lack of categories + if (cats.length === 0) { + cat_ids = [-1]; + } + data[s + "_category_ids"] = cat_ids; } - data[s + "_category_ids"] = cat_ids; } } - }); + ); [ "muted_tags", @@ -704,6 +706,14 @@ const User = RestModel.extend({ this.set("mutedCategories", Category.findByIds(this.muted_category_ids)); }, + @observes("regular_category_ids") + updateRegularCategories() { + this.set( + "regularCategories", + Category.findByIds(this.regular_category_ids) + ); + }, + @observes("tracked_category_ids") updateTrackedCategories() { this.set( diff --git a/app/assets/javascripts/discourse/app/templates/preferences/categories.hbs b/app/assets/javascripts/discourse/app/templates/preferences/categories.hbs index 7b36d0f3ffd..e76d19a27ad 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/categories.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/categories.hbs @@ -37,7 +37,17 @@
{{i18n "user.watched_first_post_categories_instructions"}}
- {{#unless siteSettings.mute_all_categories_by_default}} + {{#if siteSettings.mute_all_categories_by_default}} +
+ + {{category-selector + categories=model.regularCategories + blocklist=selectedCategories + onChange=(action (mut model.regularCategories)) + }} +
+
{{i18n "user.regular_categories_instructions"}}
+ {{else}}
{{#if canSee}} @@ -50,7 +60,7 @@ }}
{{i18n (if hideMutedTags "user.muted_categories_instructions" "user.muted_categories_instructions_dont_hide")}}
- {{/unless}} + {{/if}} {{plugin-outlet name="user-preferences-categories" args=(hash model=model save=(action "save"))}} diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index 46b74093308..3b7ac11b41d 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -54,6 +54,8 @@ class Admin::SiteSettingsController < Admin::AdminController notification_level = NotificationLevels.all[:muted] when "default_categories_watching_first_post" notification_level = NotificationLevels.all[:watching_first_post] + when "default_categories_regular" + notification_level = NotificationLevels.all[:regular] end CategoryUser.where(category_id: (previous_category_ids - new_category_ids), notification_level: notification_level).delete_all @@ -131,6 +133,8 @@ class Admin::SiteSettingsController < Admin::AdminController notification_level = NotificationLevels.all[:muted] when "default_categories_watching_first_post" notification_level = NotificationLevels.all[:watching_first_post] + when "default_categories_regular" + notification_level = NotificationLevels.all[:regular] end user_ids = CategoryUser.where(category_id: previous_category_ids - new_category_ids, notification_level: notification_level).distinct.pluck(:user_id) diff --git a/app/models/category_user.rb b/app/models/category_user.rb index afcd56ce193..04fe4691b02 100644 --- a/app/models/category_user.rb +++ b/app/models/category_user.rb @@ -207,6 +207,7 @@ class CategoryUser < ActiveRecord::Base SiteSetting.default_categories_watching.split("|"), SiteSetting.default_categories_tracking.split("|"), SiteSetting.default_categories_watching_first_post.split("|"), + SiteSetting.default_categories_regular.split("|") ].flatten.map { |id| [id.to_i, self.notification_levels[:regular]] } notification_levels += SiteSetting.default_categories_muted.split("|").map { |id| [id.to_i, self.notification_levels[:muted]] } diff --git a/app/models/user.rb b/app/models/user.rb index 440ed6519d8..7f64e5a48b3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1448,7 +1448,7 @@ class User < ActiveRecord::Base values = [] - %w{watching watching_first_post tracking muted}.each do |s| + %w{watching watching_first_post tracking regular muted}.each do |s| category_ids = SiteSetting.get("default_categories_#{s}").split("|").map(&:to_i) category_ids.each do |category_id| next if category_id == 0 diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 0192d19ceaa..3b95db221f0 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -35,6 +35,7 @@ class UserSerializer < UserCardSerializer private_attributes :locale, :muted_category_ids, + :regular_category_ids, :watched_tags, :watching_first_post_tags, :tracked_tags, @@ -219,6 +220,10 @@ class UserSerializer < UserCardSerializer CategoryUser.lookup(object, :muted).pluck(:category_id) end + def regular_category_ids + CategoryUser.lookup(object, :regular).pluck(:category_id) + end + def tracked_category_ids CategoryUser.lookup(object, :tracking).pluck(:category_id) end diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 50e2e007612..913e5ee7cd0 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -6,6 +6,7 @@ class UserUpdater watched_first_post_category_ids: :watching_first_post, watched_category_ids: :watching, tracked_category_ids: :tracking, + regular_category_ids: :regular, muted_category_ids: :muted } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9cc2bc82345..4ed06ab689e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -984,6 +984,8 @@ en: muted_categories: "Muted" muted_categories_instructions: "You will not be notified of anything about new topics in these categories, and they will not appear on the categories or latest pages." muted_categories_instructions_dont_hide: "You will not be notified of anything about new topics in these categories." + regular_categories: "Regular" + regular_categories_instructions: "These category topics will be displayed in the `latest` and `top` topics list." no_category_access: "As a moderator you have limited category access, save is disabled." delete_account: "Delete My Account" delete_account_confirm: "Are you sure you want to permanently delete your account? This action cannot be undone!" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 61f2214efe8..66515d8118c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2172,6 +2172,7 @@ en: default_categories_tracking: "List of categories that are tracked by default." default_categories_muted: "List of categories that are muted by default." default_categories_watching_first_post: "List of categories in which first post in each new topic will be watched by default." + default_categories_regular: "List of categories that are normal by default. Useful when `mute_all_categories_by_default` site setting is enabled." mute_all_categories_by_default: "Set the default notification level of all the categories to muted. Require users opt-in to categories for them to appear in 'latest' and 'categories' pages. If you wish to amend the defaults for anonymous users set 'default_categories_' settings." default_tags_watching: "List of tags that are watched by default." diff --git a/config/site_settings.yml b/config/site_settings.yml index fa5dd1611ab..e33e84ea0bc 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2174,6 +2174,9 @@ user_preferences: default_categories_watching_first_post: type: category_list default: "" + default_categories_regular: + type: category_list + default: "" mute_all_categories_by_default: default: false client: true diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb index 55da27c8b50..c49b83910b0 100644 --- a/lib/site_settings/validations.rb +++ b/lib/site_settings/validations.rb @@ -23,7 +23,8 @@ module SiteSettings::Validations default_categories_selected = [ SiteSetting.default_categories_tracking.split("|"), SiteSetting.default_categories_muted.split("|"), - SiteSetting.default_categories_watching_first_post.split("|") + SiteSetting.default_categories_watching_first_post.split("|"), + SiteSetting.default_categories_regular.split("|") ].flatten.map(&:to_i).to_set validate_default_categories(category_ids, default_categories_selected) @@ -35,7 +36,8 @@ module SiteSettings::Validations default_categories_selected = [ SiteSetting.default_categories_watching.split("|"), SiteSetting.default_categories_muted.split("|"), - SiteSetting.default_categories_watching_first_post.split("|") + SiteSetting.default_categories_watching_first_post.split("|"), + SiteSetting.default_categories_regular.split("|") ].flatten.map(&:to_i).to_set validate_default_categories(category_ids, default_categories_selected) @@ -47,7 +49,8 @@ module SiteSettings::Validations default_categories_selected = [ SiteSetting.default_categories_watching.split("|"), SiteSetting.default_categories_tracking.split("|"), - SiteSetting.default_categories_watching_first_post.split("|") + SiteSetting.default_categories_watching_first_post.split("|"), + SiteSetting.default_categories_regular.split("|") ].flatten.map(&:to_i).to_set validate_default_categories(category_ids, default_categories_selected) @@ -59,7 +62,21 @@ module SiteSettings::Validations default_categories_selected = [ SiteSetting.default_categories_watching.split("|"), SiteSetting.default_categories_tracking.split("|"), - SiteSetting.default_categories_muted.split("|") + SiteSetting.default_categories_muted.split("|"), + SiteSetting.default_categories_regular.split("|") + ].flatten.map(&:to_i).to_set + + validate_default_categories(category_ids, default_categories_selected) + end + + def validate_default_categories_regular(new_val) + category_ids = validate_category_ids(new_val) + + default_categories_selected = [ + SiteSetting.default_categories_watching.split("|"), + SiteSetting.default_categories_tracking.split("|"), + SiteSetting.default_categories_muted.split("|"), + SiteSetting.default_categories_watching_first_post.split("|") ].flatten.map(&:to_i).to_set validate_default_categories(category_ids, default_categories_selected) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 7c073b963cd..df2e60f42e2 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -891,7 +891,8 @@ class TopicQuery category_ids = [ SiteSetting.default_categories_watching.split("|"), SiteSetting.default_categories_tracking.split("|"), - SiteSetting.default_categories_watching_first_post.split("|") + SiteSetting.default_categories_watching_first_post.split("|"), + SiteSetting.default_categories_regular.split("|") ].flatten.map(&:to_i) category_ids << category_id if category_id.present? && category_ids.exclude?(category_id) diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index fdfd67dabbd..86e18d1fac4 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -338,6 +338,11 @@ describe TopicQuery do expect(TopicQuery.new.list_latest.topics.map(&:id)).to include(topic.id) end + it 'should include default regular category topics in latest list for anonymous users' do + SiteSetting.default_categories_regular = category.id.to_s + expect(TopicQuery.new.list_latest.topics.map(&:id)).to include(topic.id) + end + it 'should include topics when filtered by category' do topic_query = TopicQuery.new(user, category: topic.category_id) expect(topic_query.list_latest.topics.map(&:id)).to include(topic.id) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cea4a75126a..2d770a05dd2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1645,6 +1645,7 @@ describe User do fab!(:category1) { Fabricate(:category) } fab!(:category2) { Fabricate(:category) } fab!(:category3) { Fabricate(:category) } + fab!(:category4) { Fabricate(:category) } before do SiteSetting.default_email_digest_frequency = 1440 # daily @@ -1666,6 +1667,7 @@ describe User do SiteSetting.default_categories_tracking = category1.id.to_s SiteSetting.default_categories_muted = category2.id.to_s SiteSetting.default_categories_watching_first_post = category3.id.to_s + SiteSetting.default_categories_regular = category4.id.to_s end it "has overriden preferences" do @@ -1688,6 +1690,7 @@ describe User do expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([category1.id]) expect(CategoryUser.lookup(user, :muted).pluck(:category_id)).to eq([category2.id]) expect(CategoryUser.lookup(user, :watching_first_post).pluck(:category_id)).to eq([category3.id]) + expect(CategoryUser.lookup(user, :regular).pluck(:category_id)).to eq([category4.id]) end it "does not set category preferences for staged users" do @@ -1696,6 +1699,7 @@ describe User do expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([]) expect(CategoryUser.lookup(user, :muted).pluck(:category_id)).to eq([]) expect(CategoryUser.lookup(user, :watching_first_post).pluck(:category_id)).to eq([]) + expect(CategoryUser.lookup(user, :regular).pluck(:category_id)).to eq([]) end end diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb index 6574b3e4b2c..22a55ced834 100644 --- a/spec/serializers/site_serializer_spec.rb +++ b/spec/serializers/site_serializer_spec.rb @@ -4,9 +4,9 @@ require 'rails_helper' describe SiteSerializer do let(:guardian) { Guardian.new } + let(:category) { Fabricate(:category) } it "includes category custom fields only if its preloaded" do - category = Fabricate(:category) category.custom_fields["enable_marketplace"] = true category.save_custom_fields @@ -18,4 +18,14 @@ describe SiteSerializer do data = MultiJson.dump(described_class.new(Site.new(guardian), scope: guardian, root: false)) expect(data).to include("enable_marketplace") end + + it "returns correct notification level for categories" do + SiteSetting.mute_all_categories_by_default = true + SiteSetting.default_categories_regular = category.id.to_s + + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + categories = serialized[:categories] + expect(categories[0][:notification_level]).to eq(0) + expect(categories[-1][:notification_level]).to eq(1) + end end diff --git a/spec/serializers/web_hook_user_serializer_spec.rb b/spec/serializers/web_hook_user_serializer_spec.rb index ef6934d0041..75ca07f5b04 100644 --- a/spec/serializers/web_hook_user_serializer_spec.rb +++ b/spec/serializers/web_hook_user_serializer_spec.rb @@ -23,7 +23,7 @@ RSpec.describe WebHookUserSerializer do it 'should only include the required keys' do count = serializer.as_json.keys.count - difference = count - 49 + difference = count - 50 expect(difference).to eq(0), lambda { message = (difference < 0 ? diff --git a/test/javascripts/fixtures/user_fixtures.js b/test/javascripts/fixtures/user_fixtures.js index 287bb901b8a..d88adf77926 100644 --- a/test/javascripts/fixtures/user_fixtures.js +++ b/test/javascripts/fixtures/user_fixtures.js @@ -179,6 +179,7 @@ export default { skip_new_user_tips: false, enable_quoting: true, muted_category_ids: [], + regular_category_ids: [], tracked_category_ids: [], watched_category_ids: [3], watched_first_post_category_ids: [], @@ -2499,6 +2500,7 @@ export default { can_delete_all_posts: true, locale: null, muted_category_ids: [], + regular_category_ids: [], watched_tags: [], watching_first_post_tags: [], tracked_tags: [], @@ -2857,6 +2859,7 @@ export default { associated_accounts: [], locale: "en_US", muted_category_ids: [], + regular_category_ids: [], watched_tags: [], watching_first_post_tags: [], tracked_tags: [],