From 405ba00c08a86778a7baf28e93388201162a5347 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 8 May 2019 15:18:56 +0800 Subject: [PATCH] FEATURE: Create notifications on wiki edits for watching users. * Moves creation of notification into background job. --- app/jobs/regular/notify_post_revision.rb | 20 ++++++++ app/models/topic_user.rb | 14 ++++-- app/services/post_action_notifier.rb | 28 +++++++---- notify_post_revision.rb | 20 ++++++++ spec/services/post_action_notifier_spec.rb | 58 +++++++++++++++++++++- 5 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 app/jobs/regular/notify_post_revision.rb create mode 100644 notify_post_revision.rb diff --git a/app/jobs/regular/notify_post_revision.rb b/app/jobs/regular/notify_post_revision.rb new file mode 100644 index 00000000000..d7e38a841da --- /dev/null +++ b/app/jobs/regular/notify_post_revision.rb @@ -0,0 +1,20 @@ +module Jobs + class NotifyPostRevision < Jobs::Base + def execute(args) + user = User.find_by(id: args[:user_id]) + raise Discourse::InvalidParameters.new(:user_id) unless user + + post_revision = PostRevision.find_by(id: args[:post_revision_id]) + raise Discourse::InvalidParameters.new(:post_revision_id) unless post_revision + + PostActionNotifier.alerter.create_notification( + user, + Notification.types[:edited], + post_revision.post, + display_username: post_revision.user.username, + acting_user_id: post_revision.try(:user_id), + revision_number: post_revision.number + ) + end + end +end diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index 8122286406b..8a7125340de 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -7,11 +7,19 @@ class TopicUser < ActiveRecord::Base # used for serialization attr_accessor :post_action_data - scope :tracking, lambda { |topic_id| + scope :level, lambda { |topic_id, level| where(topic_id: topic_id) - .where("COALESCE(topic_users.notification_level, :regular) >= :tracking", + .where("COALESCE(topic_users.notification_level, :regular) >= :level", regular: TopicUser.notification_levels[:regular], - tracking: TopicUser.notification_levels[:tracking]) + level: TopicUser.notification_levels[level]) + } + + scope :tracking, lambda { |topic_id| + level(topic_id, :tracking) + } + + scope :watching, lambda { |topic_id| + level(topic_id, :watching) } # Class methods diff --git a/app/services/post_action_notifier.rb b/app/services/post_action_notifier.rb index c4d988f25dc..9d8bb923f45 100644 --- a/app/services/post_action_notifier.rb +++ b/app/services/post_action_notifier.rb @@ -93,19 +93,29 @@ class PostActionNotifier return unless post return if post_revision.user.blank? - return if post_revision.user_id == post.user_id return if post.topic.blank? return if post.topic.private_message? return if SiteSetting.disable_edit_notifications && post_revision.user_id == Discourse::SYSTEM_USER_ID - alerter.create_notification( - post.user, - Notification.types[:edited], - post, - display_username: post_revision.user.username, - acting_user_id: post_revision.try(:user_id), - revision_number: post_revision.number - ) + if post_revision.user_id != post.user_id + Jobs.enqueue(:notify_post_revision, + user_id: post.user_id, + post_revision_id: post_revision.id + ) + end + + if post.wiki && post.is_first_post? + TopicUser.watching(post.topic_id) + .where.not(user_id: post_revision.user_id) + .where(topic: post.topic) + .find_each do |topic_user| + + Jobs.enqueue(:notify_post_revision, + user_id: topic_user.user_id, + post_revision_id: post_revision.id + ) + end + end end def self.after_post_unhide(post, flaggers) diff --git a/notify_post_revision.rb b/notify_post_revision.rb new file mode 100644 index 00000000000..d94d193533c --- /dev/null +++ b/notify_post_revision.rb @@ -0,0 +1,20 @@ +module Jobs + class NotifyPostRevision < Jobs::Base + def execute(args) + user = User.find_by(id: args[:user_id]) + raise Discourse::InvalidParameters.new(:user_id) unless user + + post_revision = PostRevision.find_by(id: args[:post_revision_id]) + raise Discourse::InvalidParameters.new(:post_revision_id) unless post_revision + + PostActionNotifier.alerter.create_notification( + user, + Notification.types[:edited], + post_revision.post, + display_username: post_revision.user.username, + acting_user_id: post_revision&.user_id, + revision_number: post_revision.number + ) + end + end +end diff --git a/spec/services/post_action_notifier_spec.rb b/spec/services/post_action_notifier_spec.rb index 845ab88313e..592372c2c5e 100644 --- a/spec/services/post_action_notifier_spec.rb +++ b/spec/services/post_action_notifier_spec.rb @@ -6,6 +6,7 @@ describe PostActionNotifier do before do PostActionNotifier.enable + Jobs.run_immediately! end fab!(:evil_trout) { Fabricate(:evil_trout) } @@ -15,7 +16,62 @@ describe PostActionNotifier do it 'notifies a user of the revision' do expect { post.revise(evil_trout, raw: "world is the new body of the message") - }.to change(post.user.notifications, :count).by(1) + }.to change { post.reload.user.notifications.count }.by(1) + end + + it 'notifies watching users of revision when post is wiki-ed and first post in topic' do + SiteSetting.editing_grace_period_max_diff = 1 + + post.update!(wiki: true) + user = post.user + user2 = Fabricate(:user) + user3 = Fabricate(:user) + + TopicUser.change(user2.id, post.topic, + notification_level: TopicUser.notification_levels[:watching] + ) + + TopicUser.change(user3.id, post.topic, + notification_level: TopicUser.notification_levels[:tracking] + ) + + expect do + post.revise(Fabricate(:user), raw: "I made some changes to the wiki!") + end.to change { Notification.count }.by(2) + + edited_notification_type = Notification.types[:edited] + + expect(Notification.exists?( + user: user, + notification_type: edited_notification_type + )).to eq(true) + + expect(Notification.exists?( + user: user2, + notification_type: edited_notification_type + )).to eq(true) + + expect do + post.revise(user, raw: "I made some changes to the wiki again!") + end.to change { + Notification.where(notification_type: edited_notification_type).count + }.by(1) + + expect(Notification.where( + user: user2, + notification_type: edited_notification_type + ).count).to eq(2) + + expect do + post.revise(user2, raw: "I changed the wiki totally") + end.to change { + Notification.where(notification_type: edited_notification_type).count + }.by(1) + + expect(Notification.where( + user: user, + notification_type: edited_notification_type + ).count).to eq(2) end it 'stores the revision number with the notification' do