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
This commit is contained in:
Sam 2015-11-30 17:03:47 +11:00
parent 5a7831265a
commit ad3dd161e7
25 changed files with 210 additions and 94 deletions

View File

@ -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));

View File

@ -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];
}

View File

@ -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'),

View File

@ -1,10 +1,17 @@
function replaceSpan($e, username) {
$e.replaceWith("<a href='" +
function replaceSpan($e, username, opts) {
if (opts && opts.group) {
$e.replaceWith("<a href='" +
Discourse.getURL("/groups/") + username +
"' class='mention-group'>@" + username + "</a>");
} else {
$e.replaceWith("<a href='" +
Discourse.getURL("/users/") + username.toLowerCase() +
"' class='mention'>@" + username + "</a>");
}
}
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);
});
}

View File

@ -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');

View File

@ -10,11 +10,10 @@
</li>
{{/each}}
{{#if options.groups}}
{{#if options.users}}<hr>{{/if}}
{{#each group in options.groups}}
<li>
<a href>
<i class='icon-group'></i>
<i class='fa fa-users'></i>
<span class='username'>{{group.name}}</span>
<span class='name'>{{max-usernames group.usernames max="3"}}</span>
</a>

View File

@ -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;

View File

@ -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%);

View File

@ -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

View File

@ -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?

View File

@ -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)

View File

@ -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

View File

@ -8,6 +8,10 @@ class UserEmailObserver < ActiveRecord::Observer
@notification = notification
end
def group_mentioned
enqueue :group_mentioned
end
def mentioned
enqueue :user_mentioned
end

View File

@ -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

View File

@ -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

View File

@ -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: "<i title='mentioned' class='fa fa-at'></i><p><span>{{username}}</span> {{description}}</p>"
group_mentioned: "<i title='group mentioned' class='fa fa-at'></i><p><span>{{username}}</span> {{description}}</p>"
quoted: "<i title='quoted' class='fa fa-quote-right'></i><p><span>{{username}}</span> {{description}}</p>"
replied: "<i title='replied' class='fa fa-reply'></i><p><span>{{username}}</span> {{description}}</p>"
posted: "<i title='replied' class='fa fa-reply'></i><p><span>{{username}}</span> {{description}}</p>"

View File

@ -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: |

View File

@ -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

View File

@ -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

View File

@ -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)')

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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) }

View File

@ -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,
"<p>Hello <a class=\"mention\" href=\"/users/sam\">@sam</a></p>",