From fd99e1ef56a6e38ced23715b1942a7a4f0c7d88e Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 7 Dec 2017 11:30:47 -0500 Subject: [PATCH 1/2] FEATURE: site setting enable_mentions to turn on/off mentions --- .../components/composer-editor.js.es6 | 22 ++++++++++--------- .../javascripts/discourse/lib/search.js.es6 | 20 +++++++++-------- .../discourse-markdown/mentions.js.es6 | 4 ++++ config/locales/server.en.yml | 1 + config/site_settings.yml | 3 +++ test/javascripts/lib/pretty-text-test.js.es6 | 8 ++++++- 6 files changed, 38 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 3d25c3a4f35..29e2d313aa9 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -77,16 +77,18 @@ export default Ember.Component.extend({ const $input = this.$('.d-editor-input'); const $preview = this.$('.d-editor-preview-wrapper'); - $input.autocomplete({ - template: findRawTemplate('user-selector-autocomplete'), - dataSource: term => userSearch({ - term, - topicId, - includeMentionableGroups: true - }), - key: "@", - transformComplete: v => v.username || v.name - }); + if (this.siteSettings.enable_mentions) { + $input.autocomplete({ + template: findRawTemplate('user-selector-autocomplete'), + dataSource: term => userSearch({ + term, + topicId, + includeMentionableGroups: true + }), + key: "@", + transformComplete: v => v.username || v.name + }); + } if (this._enableAdvancedEditorPreviewSync()) { this._initInputPreviewSync($input, $preview); diff --git a/app/assets/javascripts/discourse/lib/search.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6 index 266f374a65d..f64b74db943 100644 --- a/app/assets/javascripts/discourse/lib/search.js.es6 +++ b/app/assets/javascripts/discourse/lib/search.js.es6 @@ -154,13 +154,15 @@ export function applySearchAutocomplete($input, siteSettings, appEvents, options afterComplete }, options)); - $input.autocomplete(_.merge({ - template: findRawTemplate('user-selector-autocomplete'), - key: "@", - width: '100%', - treatAsTextarea: true, - transformComplete: v => v.username || v.name, - dataSource: term => userSearch({ term, includeGroups: true }), - afterComplete - }, options)); + if (Discourse.SiteSettings.enable_mentions) { + $input.autocomplete(_.merge({ + template: findRawTemplate('user-selector-autocomplete'), + key: "@", + width: '100%', + treatAsTextarea: true, + transformComplete: v => v.username || v.name, + dataSource: term => userSearch({ term, includeGroups: true }), + afterComplete + }, options)); + } }; diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/mentions.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/mentions.js.es6 index 7314bc1f98d..8484f7ba6c0 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/mentions.js.es6 +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/mentions.js.es6 @@ -38,6 +38,10 @@ function addMention(buffer, matches, state) { } export function setup(helper) { + helper.registerOptions((opts, siteSettings) => { + opts.features.mentions = !!siteSettings.enable_mentions; + }); + helper.registerPlugin(md => { const rule = { diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 07c38b521ff..7bb5823f9e5 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1283,6 +1283,7 @@ en: newuser_max_replies_per_topic: "Maximum number of replies a new user can make in a single topic until someone replies to them." max_mentions_per_post: "Maximum number of @name notifications anyone can use in a post." max_users_notified_per_group_mention: "Maximum number of users that may receive a notification if a group is mentioned (if threshold is met no notifications will be raised)" + enable_mentions: "Allow users to mention other users." create_thumbnails: "Create thumbnails and lightbox images that are too large to fit in a post." diff --git a/config/site_settings.yml b/config/site_settings.yml index 38c102a19f6..3d004c32674 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -537,6 +537,9 @@ posting: default: 1 client: true post_undo_action_window_mins: 10 + enable_mentions: + default: true + client: true max_mentions_per_post: 10 max_users_notified_per_group_mention: 100 newuser_max_replies_per_topic: 3 diff --git a/test/javascripts/lib/pretty-text-test.js.es6 b/test/javascripts/lib/pretty-text-test.js.es6 index f68de877be8..65803e8b188 100644 --- a/test/javascripts/lib/pretty-text-test.js.es6 +++ b/test/javascripts/lib/pretty-text-test.js.es6 @@ -8,6 +8,7 @@ QUnit.module("lib:pretty-text"); const rawOpts = { siteSettings: { enable_emoji: true, + enable_mentions: true, emoji_set: 'emoji_one', highlighted_languages: 'json|ruby|javascript', default_code_lang: 'auto', @@ -282,7 +283,6 @@ QUnit.test("Quotes", assert => { }); QUnit.test("Mentions", assert => { - const alwaysTrue = { mentionLookup: (function() { return "user"; }) }; assert.cookedOptions("Hello @sam", alwaysTrue, @@ -370,6 +370,12 @@ QUnit.test("Mentions", assert => { "it allows mentions within HTML tags"); }); +QUnit.test("Mentions - disabled", assert => { + assert.cookedOptions("@eviltrout", + { siteSettings : { enable_mentions: false }}, + "

@eviltrout

"); +}); + QUnit.test("Category hashtags", assert => { const alwaysTrue = { categoryHashtagLookup: (function() { return ["http://test.discourse.org/category-hashtag", "category-hashtag"]; }) }; From 74b9828731f5e9298b9a05483861e7916961931e Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 7 Dec 2017 16:16:53 -0500 Subject: [PATCH 2/2] FIX: Remove mentions filters from user and groups Additionally return no data if disabled --- .../discourse/templates/group/activity.hbs | 4 +- .../templates/user/notifications.hbs | 12 +- app/controllers/groups_controller.rb | 2 + app/controllers/user_actions_controller.rb | 3 +- app/models/user_action.rb | 5 + spec/controllers/groups_controller_spec.rb | 6 + spec/models/user_action_spec.rb | 150 +++++++++++------- test/javascripts/helpers/site-settings.js | 3 +- 8 files changed, 116 insertions(+), 69 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/group/activity.hbs b/app/assets/javascripts/discourse/templates/group/activity.hbs index 08df8ddb575..50798a89d7c 100644 --- a/app/assets/javascripts/discourse/templates/group/activity.hbs +++ b/app/assets/javascripts/discourse/templates/group/activity.hbs @@ -2,7 +2,9 @@ {{#mobile-nav class='group-activity-nav' desktopClass="pull-left nav nav-stacked" currentPath=application.currentPath}} {{group-activity-filter filter="posts" categoryId=category_id}} {{group-activity-filter filter="topics" categoryId=category_id}} - {{group-activity-filter filter="mentions" categoryId=category_id}} + {{#if siteSettings.enable_mentions}} + {{group-activity-filter filter="mentions" categoryId=category_id}} + {{/if}} {{#if showGroupMessages}} {{group-activity-filter filter="messages"}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/user/notifications.hbs b/app/assets/javascripts/discourse/templates/user/notifications.hbs index fc69bf17f8f..c732db3fbac 100644 --- a/app/assets/javascripts/discourse/templates/user/notifications.hbs +++ b/app/assets/javascripts/discourse/templates/user/notifications.hbs @@ -15,11 +15,13 @@ {{i18n 'user_action_groups.2'}} {{/link-to}} -
  • - {{#link-to 'userNotifications.mentions'}} - {{i18n 'user_action_groups.7'}} - {{/link-to}} -
  • + {{#if siteSettings.enable_mentions}} +
  • + {{#link-to 'userNotifications.mentions'}} + {{i18n 'user_action_groups.7'}} + {{/link-to}} +
  • + {{/if}}
  • {{#link-to 'userNotifications.edits'}} {{i18n 'user_action_groups.11'}} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 324847f5bca..3fde94894cb 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -98,6 +98,7 @@ class GroupsController < ApplicationController end def mentions + raise Discourse::NotFound unless SiteSetting.enable_mentions? group = find_group(:group_id) posts = group.mentioned_posts_for( guardian, @@ -107,6 +108,7 @@ class GroupsController < ApplicationController end def mentions_feed + raise Discourse::NotFound unless SiteSetting.enable_mentions? group = find_group(:group_id) @posts = group.mentioned_posts_for( guardian, diff --git a/app/controllers/user_actions_controller.rb b/app/controllers/user_actions_controller.rb index 1efe4eb6ad1..417147ac5e0 100644 --- a/app/controllers/user_actions_controller.rb +++ b/app/controllers/user_actions_controller.rb @@ -7,12 +7,13 @@ class UserActionsController < ApplicationController per_chunk = 30 user = fetch_user_from_params(include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts)) + action_types = (params[:filter] || "").split(",").map(&:to_i) opts = { user_id: user.id, user: user, offset: params[:offset].to_i, limit: per_chunk, - action_types: (params[:filter] || "").split(",").map(&:to_i), + action_types: action_types, guardian: guardian, ignore_private_messages: params[:filter] ? false : true } diff --git a/app/models/user_action.rb b/app/models/user_action.rb index 7318983f039..997f2b69165 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -209,6 +209,11 @@ SQL else builder.where("a.user_id = :user_id", user_id: user_id.to_i) builder.where("a.action_type in (:action_types)", action_types: action_types) if action_types && action_types.length > 0 + + unless SiteSetting.enable_mentions? + builder.where("a.action_type <> :mention_type", mention_type: UserAction::MENTION) + end + builder .order_by("a.created_at desc") .offset(offset.to_i) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index f73cbcc00f4..05b082a9351 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -83,6 +83,12 @@ describe GroupsController do expect(response).to be_success expect(response.content_type).to eq('application/rss+xml') end + + it 'fails when disabled' do + SiteSetting.enable_mentions = false + get :mentions_feed, params: { group_id: group.name }, format: :rss + expect(response).not_to be_success + end end end diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 181a8fb0d91..d76d6afad61 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -33,76 +33,104 @@ describe UserAction do }.merge(opts)) end - before do - # Create some test data using a helper - log_test_action - log_test_action(action_type: UserAction::GOT_PRIVATE_MESSAGE) - log_test_action(action_type: UserAction::NEW_TOPIC, target_topic_id: public_topic.id, target_post_id: public_post.id) - log_test_action(action_type: UserAction::BOOKMARK) + describe "integration" do + before do + # Create some test data using a helper + log_test_action + log_test_action(action_type: UserAction::GOT_PRIVATE_MESSAGE) + log_test_action(action_type: UserAction::NEW_TOPIC, target_topic_id: public_topic.id, target_post_id: public_post.id) + log_test_action(action_type: UserAction::BOOKMARK) + end + + def stats_for_user(viewer = nil) + UserAction.stats(user.id, Guardian.new(viewer)).map { |r| r["action_type"].to_i }.sort + end + + def stream_count(viewer = nil) + UserAction.stream(user_id: user.id, guardian: Guardian.new(viewer)).count + end + + it 'includes the events correctly' do + PostActionNotifier.enable + + mystats = stats_for_user(user) + expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE, UserAction::BOOKMARK].sort + expect(mystats).to eq(expecting) + expect(stream_count(user)).to eq(4) + + other_stats = stats_for_user + expecting = [UserAction::NEW_TOPIC] + expect(stream_count).to eq(1) + + expect(other_stats).to eq(expecting) + + public_topic.trash!(user) + expect(stats_for_user).to eq([]) + expect(stream_count).to eq(0) + + # groups + category = Fabricate(:category, read_restricted: true) + + public_topic.recover! + public_topic.category = category + public_topic.save + + expect(stats_for_user).to eq([]) + expect(stream_count).to eq(0) + + group = Fabricate(:group) + u = Fabricate(:coding_horror) + group.add(u) + group.save + + category.set_permissions(group => :full) + category.save + + expect(stats_for_user(u)).to eq([UserAction::NEW_TOPIC]) + expect(stream_count(u)).to eq(1) + + # duplicate should not exception out + log_test_action + + # recategorize belongs to the right user + category2 = Fabricate(:category) + admin = Fabricate(:admin) + public_post.revise(admin, category_id: category2.id) + + action = UserAction.stream(user_id: public_topic.user_id, guardian: Guardian.new)[0] + expect(action.acting_user_id).to eq(admin.id) + expect(action.action_type).to eq(UserAction::EDIT) + end end - def stats_for_user(viewer = nil) - UserAction.stats(user.id, Guardian.new(viewer)).map { |r| r["action_type"].to_i }.sort - end + describe "mentions" do + before do + log_test_action(action_type: UserAction::MENTION) + end - def stream_count(viewer = nil) - UserAction.stream(user_id: user.id, guardian: Guardian.new(viewer)).count - end + let(:stream) do + UserAction.stream( + user_id: user.id, + guardian: Guardian.new(user) + ) + end - it 'includes the events correctly' do - PostActionNotifier.enable + it "is returned by the stream" do + expect(stream).to be_present + end - mystats = stats_for_user(user) - expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE, UserAction::BOOKMARK].sort - expect(mystats).to eq(expecting) - expect(stream_count(user)).to eq(4) - - other_stats = stats_for_user - expecting = [UserAction::NEW_TOPIC] - expect(stream_count).to eq(1) - - expect(other_stats).to eq(expecting) - - public_topic.trash!(user) - expect(stats_for_user).to eq([]) - expect(stream_count).to eq(0) - - # groups - category = Fabricate(:category, read_restricted: true) - - public_topic.recover! - public_topic.category = category - public_topic.save - - expect(stats_for_user).to eq([]) - expect(stream_count).to eq(0) - - group = Fabricate(:group) - u = Fabricate(:coding_horror) - group.add(u) - group.save - - category.set_permissions(group => :full) - category.save - - expect(stats_for_user(u)).to eq([UserAction::NEW_TOPIC]) - expect(stream_count(u)).to eq(1) - - # duplicate should not exception out - log_test_action - - # recategorize belongs to the right user - category2 = Fabricate(:category) - admin = Fabricate(:admin) - public_post.revise(admin, category_id: category2.id) - - action = UserAction.stream(user_id: public_topic.user_id, guardian: Guardian.new)[0] - expect(action.acting_user_id).to eq(admin.id) - expect(action.action_type).to eq(UserAction::EDIT) + it "isn't returned when mentions aren't enabled" do + SiteSetting.enable_mentions = false + expect(stream).to be_blank + end end end + describe "mentions" do + it "returns the mention in the stream" do + end + end describe 'when user likes' do let(:post) { Fabricate(:post) } diff --git a/test/javascripts/helpers/site-settings.js b/test/javascripts/helpers/site-settings.js index 90150b39201..f11eb01177a 100644 --- a/test/javascripts/helpers/site-settings.js +++ b/test/javascripts/helpers/site-settings.js @@ -88,6 +88,7 @@ Discourse.SiteSettingsOriginal = { "highlighted_languages":"apache|bash|cs|cpp|css|coffeescript|diff|xml|http|ini|json|java|javascript|makefile|markdown|nginx|objectivec|ruby|perl|php|python|sql|handlebars", "enable_emoji":true, "emoji_set":"emoji_one", - "desktop_category_page_style":"categories_and_latest_topics" + "desktop_category_page_style":"categories_and_latest_topics", + "enable_mentions":true }; Discourse.SiteSettings = jQuery.extend(true, {}, Discourse.SiteSettingsOriginal);