From 71f66cd679fdeba28567c10bee064dc1b97dfd0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 24 May 2018 17:27:43 +0200 Subject: [PATCH] FIX: ensure PostAlerter is always run in sidekiq --- app/jobs/regular/notify_category_change.rb | 15 +++++ app/jobs/regular/post_alert.rb | 9 ++- app/models/topic.rb | 19 ++----- app/services/post_action_notifier.rb | 3 - app/services/post_alerter.rb | 65 ++++++++++------------ lib/post_jobs_enqueuer.rb | 6 +- lib/post_revisor.rb | 2 +- 7 files changed, 60 insertions(+), 59 deletions(-) create mode 100644 app/jobs/regular/notify_category_change.rb diff --git a/app/jobs/regular/notify_category_change.rb b/app/jobs/regular/notify_category_change.rb new file mode 100644 index 00000000000..ed8a55c7f7a --- /dev/null +++ b/app/jobs/regular/notify_category_change.rb @@ -0,0 +1,15 @@ +require_dependency "post_alerter" + +module Jobs + class NotifyCategoryChange < Jobs::Base + def execute(args) + post = Post.find_by(id: args[:post_id]) + + if post&.topic + post_alerter = PostAlerter.new + post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids])) + post_alerter.notify_first_post_watchers(post, post_alerter.category_watchers(post.topic)) + end + end + end +end diff --git a/app/jobs/regular/post_alert.rb b/app/jobs/regular/post_alert.rb index 6fb5fb06164..6f5fc2523c9 100644 --- a/app/jobs/regular/post_alert.rb +++ b/app/jobs/regular/post_alert.rb @@ -4,9 +4,12 @@ module Jobs class PostAlert < Jobs::Base def execute(args) - # maybe it was removed by the time we are making the post - post = Post.where(id: args[:post_id]).first - PostAlerter.post_created(post, args[:options] || {}) if post && post.topic + post = Post.find_by(id: args[:post_id]) + if post&.topic + opts = args[:options] || {} + new_record = true == args[:new_record] + PostAlerter.new(opts).after_save_post(post, new_record) + end end end diff --git a/app/models/topic.rb b/app/models/topic.rb index ede59422bd9..78cc39584ef 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -675,19 +675,9 @@ SQL CategoryUser.auto_watch(category_id: new_category.id, topic_id: self.id) CategoryUser.auto_track(category_id: new_category.id, topic_id: self.id) - post = self.ordered_posts.first - - if post - post_alerter = PostAlerter.new - - post_alerter.notify_post_users( - post, - [post.user, post.last_editor].uniq - ) - - post_alerter.notify_first_post_watchers( - post, post_alerter.category_watchers(self) - ) + if post = self.ordered_posts.first + notified_user_ids = [post.user_id, post.last_editor_id].uniq + Jobs.enqueue(:notify_category_change, post_id: post.id, notified_user_ids: notified_user_ids) end end @@ -788,8 +778,7 @@ SQL last_post = posts.order('post_number desc').where('not hidden AND posts.deleted_at IS NULL').first if last_post - # ensure all the notifications are out - PostAlerter.new.after_save_post(last_post) + Jobs.enqueue(:post_alert, post_id: last_post.id) add_small_action(user, "invited_group", group.name) group_id = group.id diff --git a/app/services/post_action_notifier.rb b/app/services/post_action_notifier.rb index a25524b5435..cc42feec420 100644 --- a/app/services/post_action_notifier.rb +++ b/app/services/post_action_notifier.rb @@ -41,7 +41,6 @@ class PostActionNotifier end def self.post_action_deleted(post_action) - return if @disabled # We only care about deleting post actions for now @@ -69,7 +68,6 @@ class PostActionNotifier end def self.post_action_created(post_action) - return if @disabled # We only notify on likes for now @@ -89,7 +87,6 @@ class PostActionNotifier end def self.after_create_post_revision(post_revision) - return if @disabled post = post_revision.post diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 240d817c590..f05754357d3 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -134,11 +134,9 @@ class PostAlerter user_ids -= [post.user_id] users = User.where(id: user_ids) - Scheduler::Defer.later("Notify First Post Watchers") do - DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) - users.each do |user| - create_notification(user, Notification.types[:watching_first_post], post) - end + DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) + users.each do |user| + create_notification(user, Notification.types[:watching_first_post], post) end end @@ -406,6 +404,7 @@ class PostAlerter DiscourseEvent.trigger(:post_notification_alert, user, payload) end end + created.id ? created : nil end @@ -486,11 +485,9 @@ class PostAlerter users = [users] unless users.is_a?(Array) users = users.reject { |u| u.staged? } if post.topic&.private_message? - Scheduler::Defer.later("Notify Users") do - DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) - users.each do |u| - create_notification(u, Notification.types[type], post, opts) - end + DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) + users.each do |u| + create_notification(u, Notification.types[type], post, opts) end users @@ -499,28 +496,26 @@ class PostAlerter def notify_pm_users(post, reply_to_user, notified) return unless post.topic - Scheduler::Defer.later("Notify PM Users") do - # users that aren't part of any mentioned groups - users = directly_targeted_users(post).reject { |u| notified.include?(u) } - DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) - users.each do |user| - notification_level = TopicUser.get(post.topic, user)&.notification_level - if reply_to_user == user || notification_level == TopicUser.notification_levels[:watching] || user.staged? - create_notification(user, Notification.types[:private_message], post) - end + # users that aren't part of any mentioned groups + users = directly_targeted_users(post).reject { |u| notified.include?(u) } + DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) + users.each do |user| + notification_level = TopicUser.get(post.topic, user)&.notification_level + if reply_to_user == user || notification_level == TopicUser.notification_levels[:watching] || user.staged? + create_notification(user, Notification.types[:private_message], post) end + end - # users that are part of all mentionned groups - users = indirectly_targeted_users(post).reject { |u| notified.include?(u) } - DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) - users.each do |user| - case TopicUser.get(post.topic, user)&.notification_level - when TopicUser.notification_levels[:watching] - # only create a notification when watching the group - create_notification(user, Notification.types[:private_message], post) - when TopicUser.notification_levels[:tracking] - notify_group_summary(user, post) - end + # users that are part of all mentionned groups + users = indirectly_targeted_users(post).reject { |u| notified.include?(u) } + DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) + users.each do |user| + case TopicUser.get(post.topic, user)&.notification_level + when TopicUser.notification_levels[:watching] + # only create a notification when watching the group + create_notification(user, Notification.types[:private_message], post) + when TopicUser.notification_levels[:tracking] + notify_group_summary(user, post) end end end @@ -575,12 +570,10 @@ class PostAlerter exclude_user_ids = notified.map(&:id) notify = notify.where("id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present? - Scheduler::Defer.later("Notify Post Users") do - DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post) - notify.pluck(:id).each do |user_id| - user = User.find_by(id: user_id) - create_notification(user, Notification.types[:posted], post) - end + DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post) + notify.pluck(:id).each do |user_id| + user = User.find_by(id: user_id) + create_notification(user, Notification.types[:posted], post) end end diff --git a/lib/post_jobs_enqueuer.rb b/lib/post_jobs_enqueuer.rb index 898743e44a5..2c4ac7218bc 100644 --- a/lib/post_jobs_enqueuer.rb +++ b/lib/post_jobs_enqueuer.rb @@ -26,7 +26,11 @@ class PostJobsEnqueuer private def enqueue_post_alerts - Jobs.enqueue(:post_alert, post_id: @post.id, options: @opts[:post_alert_options]) + Jobs.enqueue(:post_alert, + post_id: @post.id, + new_record: true, + options: @opts[:post_alert_options], + ) end def feature_topic_users diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index efa718787d5..81ab76494a0 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -577,7 +577,7 @@ class PostRevisor def alert_users return if @editor.id == Discourse::SYSTEM_USER_ID - PostAlerter.new.after_save_post(@post) + Jobs.enqueue(:post_alert, post_id: @post.id) end def publish_changes