From 24667cedee7a32aa584b781d69c98e1518dc704b Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 18 Mar 2014 13:12:07 +1100 Subject: [PATCH] FEATURE: notify users when linked Extract out PostAlerter from observer Track outgoing post links correctly Notify users they are linked when linked --- app/models/notification.rb | 3 +- app/models/post_alert_observer.rb | 146 ++-------------------------- app/models/topic_link.rb | 14 +-- app/services/post_alerter.rb | 155 ++++++++++++++++++++++++++++++ config/locales/client.en.yml | 1 + config/locales/server.en.yml | 1 + lib/post_creator.rb | 2 + lib/post_revisor.rb | 1 + 8 files changed, 179 insertions(+), 144 deletions(-) create mode 100644 app/services/post_alerter.rb diff --git a/app/models/notification.rb b/app/models/notification.rb index c4b489ffd8e..b90ad746190 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -27,7 +27,8 @@ class Notification < ActiveRecord::Base def self.types @types ||= Enum.new( :mentioned, :replied, :quoted, :edited, :liked, :private_message, - :invited_to_private_message, :invitee_accepted, :posted, :moved_post + :invited_to_private_message, :invitee_accepted, :posted, :moved_post, + :linked ) end diff --git a/app/models/post_alert_observer.rb b/app/models/post_alert_observer.rb index 3413c3ce735..579a8744e12 100644 --- a/app/models/post_alert_observer.rb +++ b/app/models/post_alert_observer.rb @@ -1,5 +1,9 @@ class PostAlertObserver < ActiveRecord::Observer - observe :post, :post_action, :post_revision + observe :post_action, :post_revision + + def alerter + @alerter ||= PostAlerter.new + end # Dispatch to an after_save_#{class_name} method def after_save(model) @@ -13,18 +17,6 @@ class PostAlertObserver < ActiveRecord::Observer send(method_name, model) if respond_to?(method_name) end - # We need to consider new people to mention / quote when a post is edited - def after_save_post(post) - return if post.topic.private_message? - - mentioned_users = extract_mentioned_users(post) - quoted_users = extract_quoted_users(post) - - reply_to_user = post.reply_notification_target - notify_users(mentioned_users - [reply_to_user], :mentioned, post) - notify_users(quoted_users - mentioned_users - [reply_to_user], :quoted, post) - end - def after_save_post_action(post_action) # We only care about deleting post actions for now return if post_action.deleted_at.blank? @@ -38,7 +30,7 @@ class PostAlertObserver < ActiveRecord::Observer post = post_action.post return if post_action.user.blank? - create_notification(post.user, + alerter.create_notification(post.user, Notification.types[:liked], post, display_username: post_action.user.username, @@ -53,7 +45,7 @@ class PostAlertObserver < ActiveRecord::Observer return if post_revision.user_id == post.user_id return if post.topic.private_message? - create_notification(post.user, Notification.types[:edited], post, display_username: post_revision.user.username) + alerter.create_notification(post.user, Notification.types[:edited], post, display_username: post_revision.user.username) end def after_create_post(post) @@ -67,11 +59,11 @@ class PostAlertObserver < ActiveRecord::Observer next unless post.reply_to_post.user_id == user.id end - create_notification(user, Notification.types[:private_message], post) + alerter.create_notification(user, Notification.types[:private_message], post) end elsif post.post_type != Post.types[:moderator_action] # 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) + alerter.notify_post_users(post) end end @@ -81,124 +73,4 @@ class PostAlertObserver < ActiveRecord::Observer "#{action}_#{model.class.name.underscore.gsub(/.+\//, '')}" end - def unread_posts(user, topic) - Post.where('post_number > COALESCE(( - SELECT last_read_post_number FROM topic_users tu - WHERE tu.user_id = ? AND tu.topic_id = ? ),0)', - user.id, topic.id) - .where('reply_to_user_id = ? OR exists( - SELECT 1 from topic_users tu - WHERE tu.user_id = ? AND - tu.topic_id = ? AND - notification_level = ? - )', user.id, user.id, topic.id, TopicUser.notification_levels[:watching]) - .where(topic_id: topic.id) - end - - def first_unread_post(user, topic) - unread_posts(user, topic).order('post_number').first - end - - def unread_count(user, topic) - unread_posts(user, topic).count - end - - def destroy_notifications(user, type, topic) - return if user.blank? - return unless Guardian.new(user).can_see?(topic) - - user.notifications.where(notification_type: type, - topic_id: topic.id).destroy_all - end - - def create_notification(user, type, post, opts={}) - return if user.blank? - - # Make sure the user can see the post - return unless Guardian.new(user).can_see?(post) - - # skip if muted on the topic - return if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:muted] - - # Don't notify the same user about the same notification on the same post - return if user.notifications.exists?(notification_type: type, topic_id: post.topic_id, post_number: post.post_number) - - collapsed = false - - if type == Notification.types[:replied] || - type == Notification.types[:posted] - - destroy_notifications(user, Notification.types[:replied] , post.topic) - destroy_notifications(user, Notification.types[:posted] , post.topic) - collapsed = true - end - - if type == Notification.types[:private_message] - destroy_notifications(user, type, post.topic) - collapsed = true - end - - original_post = post - original_username = opts[:display_username] || post.username - - if collapsed - post = first_unread_post(user,post.topic) || post - count = unread_count(user, post.topic) - opts[:display_username] = I18n.t('embed.replies', count: count) if count > 1 - end - - UserActionObserver.log_notification(original_post, user, type) - - # 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) - 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) - end - - # TODO: Move to post-analyzer? - # Returns a list of users who were quoted in the post - def extract_quoted_users(post) - post.raw.scan(/\[quote=\"([^,]+),.+\"\]/).uniq.map do |m| - User.where("username_lower = :username and id != :id", username: m.first.strip.downcase, id: post.user_id).first - end.compact - end - - # Notify a bunch of users - def notify_users(users, type, post) - users = [users] unless users.is_a?(Array) - users.each do |u| - create_notification(u, Notification.types[type], post) - 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) - - exclude_user_ids = [] << - post.user_id << - extract_mentioned_users(post).map(&:id) << - extract_quoted_users(post).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, notification_level: TopicUser.notification_levels[:watching]) - .includes(:user).each do |tu| - create_notification(tu.user, Notification.types[:posted], post) unless exclude_user_ids.include?(tu.user_id) - end - end end diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 529ca855b5f..cc171dbde10 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -6,6 +6,7 @@ class TopicLink < ActiveRecord::Base belongs_to :user belongs_to :post belongs_to :link_topic, class_name: 'Topic' + belongs_to :link_post, class_name: 'Post' validates_presence_of :url @@ -129,6 +130,11 @@ class TopicLink < ActiveRecord::Base # Skip linking to ourselves next if topic_id == post.topic_id + reflected_post = nil + if post_number && topic_id + reflected_post = Post.where(topic_id: topic_id, post_number: post_number.to_i).first + end + added_urls << url TopicLink.create(post_id: post.id, user_id: post.user_id, @@ -136,7 +142,8 @@ class TopicLink < ActiveRecord::Base url: url, domain: parsed.host || Discourse.current_hostname, internal: internal, - link_topic_id: topic_id) + link_topic_id: topic_id, + link_post_id: reflected_post.try(:id)) # Create the reflection if we can if topic_id.present? @@ -146,11 +153,6 @@ class TopicLink < ActiveRecord::Base prefix = Discourse.base_url - reflected_post = nil - if post_number.present? - reflected_post = Post.where(topic_id: topic_id, post_number: post_number.to_i).first - end - reflected_url = "#{prefix}#{post.topic.relative_url(post.post_number)}" reflected_urls << reflected_url diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb new file mode 100644 index 00000000000..d0649dfc822 --- /dev/null +++ b/app/services/post_alerter.rb @@ -0,0 +1,155 @@ +class PostAlerter + + def after_save_post(post) + return if post.topic.private_message? + + mentioned_users = extract_mentioned_users(post) + quoted_users = extract_quoted_users(post) + linked_users = extract_linked_users(post) + + reply_to_user = post.reply_notification_target + + notified = [reply_to_user] + + notify_users(mentioned_users - notified, :mentioned, post) + + notified += mentioned_users + + notify_users(quoted_users - notified, :quoted, post) + + notified += quoted_users + + notify_users(linked_users - notified, :linked, post) + end + + def unread_posts(user, topic) + Post.where('post_number > COALESCE(( + SELECT last_read_post_number FROM topic_users tu + WHERE tu.user_id = ? AND tu.topic_id = ? ),0)', + user.id, topic.id) + .where('reply_to_user_id = ? OR exists( + SELECT 1 from topic_users tu + WHERE tu.user_id = ? AND + tu.topic_id = ? AND + notification_level = ? + )', user.id, user.id, topic.id, TopicUser.notification_levels[:watching]) + .where(topic_id: topic.id) + end + + def first_unread_post(user, topic) + unread_posts(user, topic).order('post_number').first + end + + def unread_count(user, topic) + unread_posts(user, topic).count + end + + def destroy_notifications(user, type, topic) + return if user.blank? + return unless Guardian.new(user).can_see?(topic) + + user.notifications.where(notification_type: type, + topic_id: topic.id).destroy_all + end + + def create_notification(user, type, post, opts={}) + return if user.blank? + + # Make sure the user can see the post + return unless Guardian.new(user).can_see?(post) + + # skip if muted on the topic + return if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:muted] + + # Don't notify the same user about the same notification on the same post + return if user.notifications.exists?(notification_type: type, topic_id: post.topic_id, post_number: post.post_number) + + collapsed = false + + if type == Notification.types[:replied] || + type == Notification.types[:posted] + + destroy_notifications(user, Notification.types[:replied] , post.topic) + destroy_notifications(user, Notification.types[:posted] , post.topic) + collapsed = true + end + + if type == Notification.types[:private_message] + destroy_notifications(user, type, post.topic) + collapsed = true + end + + original_post = post + original_username = opts[:display_username] || post.username + + if collapsed + post = first_unread_post(user,post.topic) || post + count = unread_count(user, post.topic) + opts[:display_username] = I18n.t('embed.replies', count: count) if count > 1 + end + + UserActionObserver.log_notification(original_post, user, type) + + # 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) + 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) + end + + # TODO: Move to post-analyzer? + # Returns a list of users who were quoted in the post + def extract_quoted_users(post) + post.raw.scan(/\[quote=\"([^,]+),.+\"\]/).uniq.map do |m| + User.where("username_lower = :username and id != :id", username: m.first.strip.downcase, id: post.user_id).first + end.compact + end + + def extract_linked_users(post) + post.topic_links.map do |link| + post = link.link_post + if !post && topic = link.link_topic + post = topic.posts(post_number: 1).first + end + post && post.user + end.compact + end + + # Notify a bunch of users + def notify_users(users, type, post) + users = [users] unless users.is_a?(Array) + users.each do |u| + create_notification(u, Notification.types[type], post) + 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) + + exclude_user_ids = [] << + post.user_id << + extract_mentioned_users(post).map(&:id) << + extract_quoted_users(post).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, notification_level: TopicUser.notification_levels[:watching]) + .includes(:user).each do |tu| + create_notification(tu.user, Notification.types[:posted], post) unless exclude_user_ids.include?(tu.user_id) + end + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 91d11383f7e..88aef45af31 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -597,6 +597,7 @@ en: invitee_accepted: " {{username}} accepted your invitation" moved_post: " {{username}} moved {{link}}" total_flagged: "total flagged posts" + linked: " {{username}} {{link}}" upload_selector: title: "Add an image" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9762dd7ffea..4612b67cb2e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -872,6 +872,7 @@ en: private_message: "%{display_username} sent you a private message: %{link}" invited_to_private_message: "%{display_username} invited you to a private message: %{link}" invitee_accepted: "%{display_username} accepted your invitation" + linked: "%{display_username} linked you in %{link}" search: within_post: "#%{post_number} by %{username}: %{excerpt}" diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 8aaf6248d4f..f945f286f23 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -64,10 +64,12 @@ class PostCreator track_topic update_topic_stats update_user_counts + publish ensure_in_allowed_users if guardian.is_staff? @post.advance_draft_sequence @post.save_reply_relationships + PostAlerter.new.after_save_post(@post) end handle_spam diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index c2a3155ecd5..b42927a6d21 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -17,6 +17,7 @@ class PostRevisor post_process_post update_topic_word_counts @post.advance_draft_sequence + PostAlerter.new.after_save_post(@post) true end