From ad3dd161e7f388793fd44e57826928812d1cd2d5 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 30 Nov 2015 17:03:47 +1100 Subject: [PATCH] FEATURE: first class group mentions built in If you allow a group to be mentioned it can be mentioned with the @ symbol. Keep in mind as a safety mechanism max_users_notified_per_group_mention is set to 100 --- .../components/composer-editor.js.es6 | 2 +- .../discourse/dialects/mention_dialect.js | 5 +- .../discourse/lib/click-track.js.es6 | 2 +- .../discourse/lib/link-mentions.js.es6 | 14 +- .../javascripts/discourse/lib/markdown.js | 1 + .../user-selector-autocomplete.raw.hbs | 3 +- .../stylesheets/common/base/compose.scss | 4 + .../stylesheets/desktop/topic-post.scss | 2 +- app/controllers/users_controller.rb | 12 +- app/mailers/user_notifications.rb | 2 +- app/models/group.rb | 44 +++--- app/models/notification.rb | 2 +- app/models/user_email_observer.rb | 4 + app/models/user_search.rb | 6 +- app/services/post_alerter.rb | 129 ++++++++++++------ config/locales/client.en.yml | 3 +- config/locales/server.en.yml | 12 ++ config/site_settings.yml | 1 + lib/autospec/rspec_runner.rb | 3 - lib/pretty_text.rb | 16 ++- spec/models/category_user_spec.rb | 2 +- spec/models/post_alert_observer_spec.rb | 3 +- spec/models/user_search_spec.rb | 8 ++ spec/services/post_alerter_spec.rb | 22 ++- test/javascripts/lib/markdown-test.js.es6 | 2 +- 25 files changed, 210 insertions(+), 94 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 49e3a49d25a..9cc3120bb16 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -53,7 +53,7 @@ export default Ember.Component.extend({ template, dataSource: term => userSearch({ term, topicId, includeGroups: true }), key: "@", - transformComplete: v => v.username || v.usernames.join(", @") + transformComplete: v => v.username || v.name }); $input.on('scroll', () => Ember.run.throttle(this, this._syncEditorAndPreviewScroll, 20)); diff --git a/app/assets/javascripts/discourse/dialects/mention_dialect.js b/app/assets/javascripts/discourse/dialects/mention_dialect.js index 9579f04afe5..20af912c6de 100644 --- a/app/assets/javascripts/discourse/dialects/mention_dialect.js +++ b/app/assets/javascripts/discourse/dialects/mention_dialect.js @@ -14,8 +14,11 @@ Discourse.Dialect.inlineRegexp({ var username = matches[1], mentionLookup = this.dialect.options.mentionLookup; - if (mentionLookup && mentionLookup(username.substr(1))) { + var type = mentionLookup && mentionLookup(username.substr(1)); + if (type === "user") { return ['a', {'class': 'mention', href: Discourse.getURL("/users/") + username.substr(1).toLowerCase()}, username]; + } else if (type === "group") { + return ['a', {'class': 'mention-group', href: Discourse.getURL("/groups/") + username.substr(1)}, username]; } else { return ['span', {'class': 'mention'}, username]; } diff --git a/app/assets/javascripts/discourse/lib/click-track.js.es6 b/app/assets/javascripts/discourse/lib/click-track.js.es6 index 65800a3750a..a0e8264b786 100644 --- a/app/assets/javascripts/discourse/lib/click-track.js.es6 +++ b/app/assets/javascripts/discourse/lib/click-track.js.es6 @@ -6,7 +6,7 @@ export default { if (Discourse.Utilities.selectedText() !== "") { return false; } var $link = $(e.currentTarget); - if ($link.hasClass('lightbox')) { return true; } + if ($link.hasClass('lightbox') || $link.hasClass('mention-group')) { return true; } var href = $link.attr('href') || $link.data('href'), $article = $link.closest('article'), diff --git a/app/assets/javascripts/discourse/lib/link-mentions.js.es6 b/app/assets/javascripts/discourse/lib/link-mentions.js.es6 index 27d4f02387e..0dfa36adba2 100644 --- a/app/assets/javascripts/discourse/lib/link-mentions.js.es6 +++ b/app/assets/javascripts/discourse/lib/link-mentions.js.es6 @@ -1,10 +1,17 @@ -function replaceSpan($e, username) { - $e.replaceWith("@" + username + ""); + } else { + $e.replaceWith("@" + username + ""); + } } const found = []; +const foundGroups = []; const checked = []; function updateFound($mentions, usernames) { @@ -14,6 +21,8 @@ function updateFound($mentions, usernames) { const username = usernames[i]; if (found.indexOf(username.toLowerCase()) !== -1) { replaceSpan($e, username); + } else if (foundGroups.indexOf(username) !== -1) { + replaceSpan($e, username, {group: true}); } else if (checked.indexOf(username) !== -1) { $e.addClass('mention-tested'); } @@ -38,6 +47,7 @@ export function linkSeenMentions($elem, siteSettings) { export function fetchUnseenMentions($elem, usernames) { return Discourse.ajax("/users/is_local_username", { data: { usernames } }).then(function(r) { found.push.apply(found, r.valid); + foundGroups.push.apply(foundGroups, r.valid_groups); checked.push.apply(checked, usernames); }); } diff --git a/app/assets/javascripts/discourse/lib/markdown.js b/app/assets/javascripts/discourse/lib/markdown.js index ee7ba257373..4b9e3ce1e8f 100644 --- a/app/assets/javascripts/discourse/lib/markdown.js +++ b/app/assets/javascripts/discourse/lib/markdown.js @@ -238,6 +238,7 @@ RSVP.EventTarget.mixin(Discourse.Markdown); Discourse.Markdown.whiteListTag('a', 'class', 'attachment'); Discourse.Markdown.whiteListTag('a', 'class', 'onebox'); Discourse.Markdown.whiteListTag('a', 'class', 'mention'); +Discourse.Markdown.whiteListTag('a', 'class', 'mention-group'); Discourse.Markdown.whiteListTag('a', 'target', '_blank'); Discourse.Markdown.whiteListTag('a', 'rel', 'nofollow'); diff --git a/app/assets/javascripts/discourse/templates/user-selector-autocomplete.raw.hbs b/app/assets/javascripts/discourse/templates/user-selector-autocomplete.raw.hbs index e0f26deae84..795c473539a 100644 --- a/app/assets/javascripts/discourse/templates/user-selector-autocomplete.raw.hbs +++ b/app/assets/javascripts/discourse/templates/user-selector-autocomplete.raw.hbs @@ -10,11 +10,10 @@ {{/each}} {{#if options.groups}} - {{#if options.users}}
{{/if}} {{#each group in options.groups}}
  • - + {{group.name}} {{max-usernames group.usernames max="3"}} diff --git a/app/assets/stylesheets/common/base/compose.scss b/app/assets/stylesheets/common/base/compose.scss index 4d2c8ad6588..4bb02e82ddd 100644 --- a/app/assets/stylesheets/common/base/compose.scss +++ b/app/assets/stylesheets/common/base/compose.scss @@ -9,6 +9,10 @@ padding: 0; margin: 0; li { + .fa-users { + color: lighten($primary, 40%); + padding: 0 2px; + } border-bottom: 1px solid dark-light-diff($primary, $secondary, 90%, -60%); a[href] { padding: 5px; diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index 625261bcc52..9c00a3b482f 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -562,7 +562,7 @@ video { position: relative; } -a.mention { +a.mention, a.mention-group { padding: 2px 4px; color: $primary; background: dark-light-diff($primary, $secondary, 90%, -60%); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5b53649d1cf..386ab9eccbf 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -205,11 +205,11 @@ class UsersController < ApplicationController usernames = [params[:username]] if usernames.blank? usernames.each(&:downcase!) - result = User.where(staged: false) - .where(username_lower: usernames) - .pluck(:username_lower) + groups = Group.where(name: users).pluck(:name) + users -= groups - render json: { valid: result } + result = User.where(staged: false).where(username_lower: users).pluck(:username_lower) + render json: {valid: result, valid_groups: groups} end def render_available_true @@ -540,7 +540,9 @@ class UsersController < ApplicationController to_render = { users: results.as_json(only: user_fields, methods: [:avatar_template]) } if params[:include_groups] == "true" - to_render[:groups] = Group.search_group(term, current_user).map { |m| { name: m.name, usernames: m.usernames.split(",") } } + to_render[:groups] = Group.search_group(term).map do |m| + {name: m.name, usernames: []} + end end render json: to_render diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 4b7b58afa52..08a6df5228a 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -203,7 +203,7 @@ class UserNotifications < ActionMailer::Base notification_type = opts[:notification_type] || Notification.types[@notification.notification_type].to_s return if user.mailing_list_mode && !@post.topic.private_message? && - ["replied", "mentioned", "quoted", "posted"].include?(notification_type) + ["replied", "mentioned", "quoted", "posted", "group_mentioned"].include?(notification_type) title = @notification.data_hash[:topic_title] allow_reply_by_email = opts[:allow_reply_by_email] unless user.suspended? diff --git a/app/models/group.rb b/app/models/group.rb index ed6371f64ad..acb7945d428 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -47,6 +47,28 @@ class Group < ActiveRecord::Base validates :alias_level, inclusion: { in: ALIAS_LEVELS.values} + scope :mentionable, lambda {|user| + + levels = [ALIAS_LEVELS[:everyone]] + + if user && user.admin? + levels = [ALIAS_LEVELS[:everyone], + ALIAS_LEVELS[:only_admins], + ALIAS_LEVELS[:mods_and_admins], + ALIAS_LEVELS[:members_mods_and_admins]] + elsif user && user.moderator? + levels = [ALIAS_LEVELS[:everyone], + ALIAS_LEVELS[:mods_and_admins], + ALIAS_LEVELS[:members_mods_and_admins]] + end + + where("alias_level in (:levels) OR + ( + alias_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in ( + SELECT group_id FROM group_users WHERE user_id = :user_id) + )", levels: levels, user_id: user && user.id ) + } + def posts_for(guardian, before_post_id=nil) user_ids = group_users.map {|gu| gu.user_id} result = Post.where(user_id: user_ids).includes(:user, :topic, :topic => :category).references(:posts, :topics, :category) @@ -165,26 +187,8 @@ class Group < ActiveRecord::Base lookup_group(name) || refresh_automatic_group!(name) end - def self.search_group(name, current_user) - levels = [ALIAS_LEVELS[:everyone]] - - if current_user.admin? - levels = [ALIAS_LEVELS[:everyone], - ALIAS_LEVELS[:only_admins], - ALIAS_LEVELS[:mods_and_admins], - ALIAS_LEVELS[:members_mods_and_admins]] - elsif current_user.moderator? - levels = [ALIAS_LEVELS[:everyone], - ALIAS_LEVELS[:mods_and_admins], - ALIAS_LEVELS[:members_mods_and_admins]] - end - - Group.where("name ILIKE :term_like AND (" + - " alias_level in (:levels)" + - " OR (alias_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in (" + - "SELECT group_id FROM group_users WHERE user_id= :user_id)" + - ")" + - ")", term_like: "#{name.downcase}%", levels: levels, user_id: current_user.id) + def self.search_group(name) + Group.where(visible: true).where("name ILIKE :term_like", term_like: "#{name.downcase}%") end def self.lookup_group(name) diff --git a/app/models/notification.rb b/app/models/notification.rb index 61da20b0cfb..bde2937907a 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -30,7 +30,7 @@ class Notification < ActiveRecord::Base @types ||= Enum.new( :mentioned, :replied, :quoted, :edited, :liked, :private_message, :invited_to_private_message, :invitee_accepted, :posted, :moved_post, - :linked, :granted_badge, :invited_to_topic, :custom + :linked, :granted_badge, :invited_to_topic, :custom, :group_mentioned ) end diff --git a/app/models/user_email_observer.rb b/app/models/user_email_observer.rb index dd9027e65de..4ab88c57db2 100644 --- a/app/models/user_email_observer.rb +++ b/app/models/user_email_observer.rb @@ -8,6 +8,10 @@ class UserEmailObserver < ActiveRecord::Observer @notification = notification end + def group_mentioned + enqueue :group_mentioned + end + def mentioned enqueue :user_mentioned end diff --git a/app/models/user_search.rb b/app/models/user_search.rb index dbf99a04203..56a40af3429 100644 --- a/app/models/user_search.rb +++ b/app/models/user_search.rb @@ -3,7 +3,7 @@ class UserSearch def initialize(term, opts={}) @term = term - @term_like = "#{term.downcase}%" + @term_like = "#{term.downcase.gsub("_", "\\_")}%" @topic_id = opts[:topic_id] @topic_allowed_users = opts[:topic_allowed_users] @searching_user = opts[:searching_user] @@ -35,12 +35,14 @@ class UserSearch users = scoped_users if @term.present? - if SiteSetting.enable_names? + if SiteSetting.enable_names? && @term !~ /[_\.-]/ query = Search.ts_query(@term, "simple") + users = users.includes(:user_search_data) .references(:user_search_data) .where("user_search_data.search_data @@ #{query}") .order(User.sql_fragment("CASE WHEN username_lower LIKE ? THEN 0 ELSE 1 END ASC", @term_like)) + else users = users.where("username_lower LIKE :term_like", term_like: @term_like) end diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index a85d66b4ba1..a8d7f20fdfc 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -2,8 +2,7 @@ class PostAlerter def self.post_created(post) alerter = PostAlerter.new - alerter.after_create_post(post) - alerter.after_save_post(post) + alerter.after_save_post(post, true) post end @@ -15,39 +14,56 @@ class PostAlerter end end - def after_create_post(post) - if post.topic.private_message? + def after_save_post(post, new_record = false) + + reply_to_user = post.reply_notification_target + + notified = [post.user].compact + + if new_record && post.topic.private_message? # If it's a private message, notify the topic_allowed_users allowed_users(post).each do |user| if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:tracking] next unless post.reply_to_post_number || post.reply_to_post.try(:user_id) == user.id end create_notification(user, Notification.types[:private_message], post) + notified += [user] end - elsif post.post_type == Post.types[:regular] - # If it's not a private message and it's not an automatic post caused by a moderator action, notify the users - notify_post_users(post) end - end - def after_save_post(post) - mentioned_users = extract_mentioned_users(post) + if new_record && reply_to_user && post.post_type == Post.types[:regular] + notify_users(reply_to_user, :replied, post) + end + + if reply_to_user + notified += [reply_to_user] + end + + mentioned_groups, mentioned_users = extract_mentions(post) + quoted_users = extract_quoted_users(post) linked_users = extract_linked_users(post) - reply_to_user = post.reply_notification_target + expand_group_mentions(mentioned_groups, post) do |group, users| + notify_users(users - notified, :group_mentioned, post, group: group) + notified += users + end - notified = [reply_to_user] - - notify_users(mentioned_users - notified, :mentioned, post) - - notified += mentioned_users + if mentioned_users + notify_users(mentioned_users - notified, :mentioned, post) + notified += mentioned_users + end notify_users(quoted_users - notified, :quoted, post) - notified += quoted_users notify_users(linked_users - notified, :linked, post) + + if new_record && post.post_type == Post.types[:regular] + # If it's not a private message and it's not an automatic post caused by a moderator action, notify the users + notify_post_users(post, notified) + end + end def unread_posts(user, topic) @@ -83,14 +99,16 @@ class PostAlerter user.reload end - NOTIFIABLE_TYPES = [:mentioned, :replied, :quoted, :posted, :linked, :private_message].map{ |t| + NOTIFIABLE_TYPES = [:mentioned, :replied, :quoted, :posted, :linked, :private_message, :group_mentioned].map{ |t| Notification.types[t] } - def create_notification(user, type, post, opts={}) + def create_notification(user, type, post, opts=nil) return if user.blank? return if user.id == Discourse::SYSTEM_USER_ID + opts ||= {} + # Make sure the user can see the post return unless Guardian.new(user).can_see?(post) @@ -143,15 +161,24 @@ class PostAlerter UserActionObserver.log_notification(original_post, user, type, opts[:acting_user_id]) + notification_data = { + topic_title: post.topic.title, + original_post_id: original_post.id, + original_username: original_username, + display_username: opts[:display_username] || post.user.username + } + + if group = opts[:group] + notification_data[:group_id] = group.id + notification_data[:group_name] = group.name + end + # Create the notification user.notifications.create(notification_type: type, topic_id: post.topic_id, post_number: post.post_number, post_action_id: opts[:post_action_id], - data: { topic_title: post.topic.title, - original_post_id: original_post.id, - original_username: original_username, - display_username: opts[:display_username] || post.user.username }.to_json) + data: notification_data.to_json) if (!existing_notification) && NOTIFIABLE_TYPES.include?(type) @@ -172,12 +199,33 @@ class PostAlerter end - # TODO: Move to post-analyzer? - # Returns a list users who have been mentioned - def extract_mentioned_users(post) - User.where(username_lower: post.raw_mentions).where("id <> ?", post.user_id) + def expand_group_mentions(groups, post) + return unless post.user && groups + + Group.mentionable(post.user).where(id: groups.map(&:id)).each do |group| + next if group.user_count >= SiteSetting.max_users_notified_per_group_mention + yield group, group.users + end + end + # TODO: Move to post-analyzer? + def extract_mentions(post) + mentions = post.raw_mentions + + return unless mentions && mentions.length > 0 + + groups = Group.where('lower(name) in (?)', mentions) + mentions -= groups.map(&:name).map(&:downcase) + + return [groups, nil] unless mentions && mentions.length > 0 + + users = User.where(username_lower: mentions).where("id <> ?", post.user_id) + + [groups,users] + end + + # TODO: Move to post-analyzer? # Returns a list of users who were quoted in the post def extract_quoted_users(post) @@ -197,7 +245,7 @@ class PostAlerter end # Notify a bunch of users - def notify_users(users, type, post) + def notify_users(users, type, post, opts=nil) users = [users] unless users.is_a?(Array) if post.topic.try(:private_message?) @@ -206,29 +254,22 @@ class PostAlerter end users.each do |u| - create_notification(u, Notification.types[type], post) + create_notification(u, Notification.types[type], post, opts) end end - # TODO: This should use javascript for parsing rather than re-doing it this way. - def notify_post_users(post) - # Is this post a reply to a user? - reply_to_user = post.reply_notification_target - notify_users(reply_to_user, :replied, post) + def notify_post_users(post, notified) - exclude_user_ids = [] << - post.user_id << - extract_mentioned_users(post).map(&:id) << - extract_quoted_users(post).map(&:id) + exclude_user_ids = notified.map(&:id) - exclude_user_ids << reply_to_user.id if reply_to_user.present? - exclude_user_ids.flatten! - - TopicUser.where(topic_id: post.topic_id) + notify = TopicUser.where(topic_id: post.topic_id) .where(notification_level: TopicUser.notification_levels[:watching]) - .where("user_id NOT IN (?)", exclude_user_ids) - .includes(:user).each do |tu| + + notify = notify.where("user_id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present? + + notify.includes(:user).each do |tu| create_notification(tu.user, Notification.types[:posted], post) end end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b5fb157b461..61302b7e8c3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -340,7 +340,7 @@ en: members: "Members" posts: "Posts" alias_levels: - title: "Who can use this group as an alias?" + title: "Who can @mention this group?" nobody: "Nobody" only_admins: "Only admins" mods_and_admins: "Only moderators and Admins" @@ -927,6 +927,7 @@ en: more: "view older notifications" total_flagged: "total flagged posts" mentioned: "

    {{username}} {{description}}

    " + group_mentioned: "

    {{username}} {{description}}

    " quoted: "

    {{username}} {{description}}

    " replied: "

    {{username}} {{description}}

    " posted: "

    {{username}} {{description}}

    " diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 7069b469ebf..4b8b3efe5d7 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1021,6 +1021,7 @@ en: newuser_max_mentions_per_post: "Maximum number of @name notifications a new user can use in a post." 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 recieve a notification if a group is mentioned (if threshold is met no notifications will be raised)" create_thumbnails: "Create thumbnails and lightbox images that are too large to fit in a post." @@ -1237,6 +1238,7 @@ en: invalid_reply_by_email_address: "Value must contain '%{reply_key}' and be different from the notification email." notification_types: + group_mentioned: "%{group_name} was mentioned in %{link}" mentioned: "%{display_username} mentioned you in %{link}" liked: "%{display_username} liked your post in %{link}" replied: "%{display_username} replied to your post in %{link}" @@ -1961,6 +1963,16 @@ en: --- %{respond_instructions} + user_group_mentioned: + subject_template: "[%{site_name}] %{topic_title}" + text_body_template: | + %{message} + + %{context} + + --- + %{respond_instructions} + user_posted: subject_template: "[%{site_name}] %{topic_title}" text_body_template: | diff --git a/config/site_settings.yml b/config/site_settings.yml index 772d4a467b6..b6eab245ad1 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -432,6 +432,7 @@ posting: client: true post_undo_action_window_mins: 10 max_mentions_per_post: 10 + max_users_notified_per_group_mention: 100 newuser_max_replies_per_topic: 3 newuser_max_mentions_per_post: 2 title_max_word_length: 30 diff --git a/lib/autospec/rspec_runner.rb b/lib/autospec/rspec_runner.rb index 08677b331b1..1ed1f86d646 100644 --- a/lib/autospec/rspec_runner.rb +++ b/lib/autospec/rspec_runner.rb @@ -9,17 +9,14 @@ module Autospec # Discourse specific watch(%r{^lib/(.+)\.rb$}) { |m| "spec/components/#{m[1]}_spec.rb" } - # Rails example watch(%r{^app/(.+)\.rb$}) { |m| "spec/#{m[1]}_spec.rb" } watch(%r{^app/(.+)(\.erb|\.haml)$}) { |m| "spec/#{m[1]}#{m[2]}_spec.rb" } watch(%r{^spec/.+_spec\.rb$}) watch(%r{^spec/support/.+\.rb$}) { "spec" } watch("app/controllers/application_controller.rb") { "spec/controllers" } - # Capybara request specs watch(%r{^app/views/(.+)/.+\.(erb|haml)$}) { |m| "spec/requests/#{m[1]}_spec.rb" } - # Fabrication watch(%r{^spec/fabricators/.+_fabricator\.rb$}) { "spec" } RELOADERS = Set.new diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 3f09835437f..8d4dc5b8ad8 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -34,10 +34,18 @@ module PrettyText UrlHelper.schemaless UrlHelper.absolute avatar_template end - def is_username_valid(username) + def mention_lookup(username) return false unless username - username = username.downcase - User.exec_sql('SELECT 1 FROM users WHERE username_lower = ?', username).values.length == 1 + if Group.exec_sql('SELECT 1 FROM groups WHERE name = ?', username).values.length == 1 + "group" + else + username = username.downcase + if User.exec_sql('SELECT 1 FROM users WHERE username_lower = ?', username).values.length == 1 + "user" + else + nil + end + end end def get_topic_info(topic_id) @@ -198,7 +206,7 @@ module PrettyText # plugin emojis context.eval("Discourse.Emoji.applyCustomEmojis();") - context.eval('opts["mentionLookup"] = function(u){return helpers.is_username_valid(u);}') + context.eval('opts["mentionLookup"] = function(u){return helpers.mention_lookup(u);}') context.eval('opts["lookupAvatar"] = function(p){return Discourse.Utilities.avatarImg({size: "tiny", avatarTemplate: helpers.avatar_template(p)});}') context.eval('opts["getTopicInfo"] = function(i){return helpers.get_topic_info(i)};') baked = context.eval('Discourse.Markdown.markdownConverter(opts).makeHtml(raw)') diff --git a/spec/models/category_user_spec.rb b/spec/models/category_user_spec.rb index 2365f611b5c..5fb965f4916 100644 --- a/spec/models/category_user_spec.rb +++ b/spec/models/category_user_spec.rb @@ -40,7 +40,7 @@ describe CategoryUser do CategoryUser.create!(user: user, category: tracked_category, notification_level: CategoryUser.notification_levels[:tracking]) watched_post = create_post(category: watched_category) - muted_post = create_post(category: muted_category) + _muted_post = create_post(category: muted_category) tracked_post = create_post(category: tracked_category) expect(Notification.where(user_id: user.id, topic_id: watched_post.topic_id).count).to eq 1 diff --git a/spec/models/post_alert_observer_spec.rb b/spec/models/post_alert_observer_spec.rb index 0b72771d60b..326d34196ec 100644 --- a/spec/models/post_alert_observer_spec.rb +++ b/spec/models/post_alert_observer_spec.rb @@ -74,8 +74,7 @@ describe PostAlertObserver do expect { Guardian.any_instance.expects(:can_see?).with(instance_of(Post)).returns(false) mention_post - PostAlerter.new.after_create_post(mention_post) - PostAlerter.new.after_save_post(mention_post) + PostAlerter.post_created(mention_post) }.not_to change(evil_trout.notifications, :count) end diff --git a/spec/models/user_search_spec.rb b/spec/models/user_search_spec.rb index efcee0b8240..7a27cebc209 100644 --- a/spec/models/user_search_spec.rb +++ b/spec/models/user_search_spec.rb @@ -35,6 +35,14 @@ describe UserSearch do UserSearch.new(*args).search end + it 'allows for correct underscore searching' do + Fabricate(:user, username: 'Under_Score') + Fabricate(:user, username: 'undertaker') + + expect(search_for("under_sc").length).to eq(1) + expect(search_for("under_").length).to eq(1) + end + # this is a seriously expensive integration test, # re-creating this entire test db is too expensive reuse it "operates correctly" do diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 5a87e50dd09..34c5e498582 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -12,7 +12,7 @@ describe PostAlerter do context "unread" do it "does not return whispers as unread posts" do op = Fabricate(:post) - whisper = Fabricate(:post, raw: 'this is a whisper post', + _whisper = Fabricate(:post, raw: 'this is a whisper post', user: Fabricate(:admin), topic: op.topic, reply_to_post_number: op.post_number, @@ -92,6 +92,26 @@ describe PostAlerter do end end + context '@group mentions' do + + it 'notifies users correctly' do + + group = Fabricate(:group, name: 'group', alias_level: Group::ALIAS_LEVELS[:everyone]) + group.add(evil_trout) + + expect { + create_post_with_alerts(raw: "Hello @group how are you?") + }.to change(evil_trout.notifications, :count).by(1) + + + group.update_columns(alias_level: Group::ALIAS_LEVELS[:members_mods_and_admins]) + + expect { + create_post_with_alerts(raw: "Hello @group you are not mentionable") + }.to change(evil_trout.notifications, :count).by(0) + end + end + context '@mentions' do let(:user) { Fabricate(:user) } diff --git a/test/javascripts/lib/markdown-test.js.es6 b/test/javascripts/lib/markdown-test.js.es6 index 1e657eb28e9..818b7551411 100644 --- a/test/javascripts/lib/markdown-test.js.es6 +++ b/test/javascripts/lib/markdown-test.js.es6 @@ -202,7 +202,7 @@ test("Quotes", function() { test("Mentions", function() { - var alwaysTrue = { mentionLookup: (function() { return true; }) }; + var alwaysTrue = { mentionLookup: (function() { return "user"; }) }; cookedOptions("Hello @sam", alwaysTrue, "

    Hello @sam

    ",