FEATURE: Create notifications on wiki edits for watching users.

FEATURE: Create notifications on wiki edits for watching users.

  • Moves creation of notification into background job.
diff --git a/app/jobs/regular/notify_post_revision.rb b/app/jobs/regular/notify_post_revision.rb
new file mode 100644
index 0000000..d7e38a8
--- /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 8122286..8a71253 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 c4d988f..9d8bb92 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 0000000..d94d193
--- /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 845ab88..592372c 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

GitHub sha: 405ba00c

This commit has been mentioned on Discourse Meta. There might be relevant details there:

hmm, why not pluck the two columns you need instead?

1 Like

Fix broken spec in 405ba00c08a86778a7baf28e93388201162a5347.

oops I mean you just need the user_ids here, I wonder if you should just ship the full list of user ids to the async job, that cuts a lot of sidekiq jobs down, you can do slices of 50 or something

1 Like

DEV: Reduce number of jobs enqueued.