FIX: properly send push notifications on assign

FIX: properly send push notifications on assign

Previously we would rely on a quirk in core where whisper small actions
would get a notification, this behavior was removed and now we must
be explicit, it provides a much more consistent assign notification

From 2d5359055dfc7e96f74d14752f1f78c4657caead Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Tue, 4 Dec 2018 17:55:58 +1100
Subject: [PATCH] FIX: properly send push notifications on assign

Previously we would rely on a quirk in core where whisper small actions
would get a notification, this behavior was removed and now we must
be explicit, it provides a much more consistent assign notification

diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index f4f71c8..86a5b40 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -18,6 +18,7 @@ en:
     already_claimed: "That topic has already been claimed."
     flag_assigned: "Sorry, that flag's topic is assigned to another user"
     flag_unclaimed: "You must claim that topic before acting on the flag"
+    topic_assigned_excerpt: "@%{username} assigned you the topic '%{title}'"
   assign_mailer:
     title: "Assign Mailer"
     subject_template: "[%{email_prefix}] %{assignee_name} assigned you to '%{topic_title}'!"
diff --git a/lib/topic_assigner.rb b/lib/topic_assigner.rb
index 14751af..fb88e0a 100644
--- a/lib/topic_assigner.rb
+++ b/lib/topic_assigner.rb
@@ -181,7 +181,7 @@ SQL
 
     post_type = SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper]
 
-    unless silent
+    if !silent
       @topic.add_moderator_post(
         @assigned_by,
         nil,
@@ -191,7 +191,7 @@ SQL
         custom_fields: { "action_code_who" => assign_to.username }
       )
 
-      unless @assigned_by.id == assign_to.id
+      if @assigned_by.id != assign_to.id
 
         Notification.create!(
           notification_type: Notification.types[:custom],
@@ -207,6 +207,19 @@ SQL
       end
     end
 
+    # we got to send a push notification as well
+    # what we created here is a whisper and notification will not raise a push
+    alerter = PostAlerter.new(first_post)
+    # TODO: remove June 2019
+    if alerter.respond_to?(:create_notification_alert) && @assigned_by.id != assign_to.id
+      alerter.create_notification_alert(
+        user: assign_to,
+        post: first_post,
+        notification_type: Notification.types[:custom],
+        excerpt: I18n.t("discourse_assign.topic_assigned_excerpt", title: @topic.title, username: @assigned_by.username)
+      )
+    end
+
     true
   end
 
diff --git a/spec/lib/topic_assigner_spec.rb b/spec/lib/topic_assigner_spec.rb
index f6bfe1b..6a522a7 100644
--- a/spec/lib/topic_assigner_spec.rb
+++ b/spec/lib/topic_assigner_spec.rb
@@ -27,10 +27,18 @@ RSpec.describe TopicAssigner do
     let(:post) { Fabricate(:post) }
     let(:topic) { post.topic }
     let(:moderator) { Fabricate(:moderator) }
-    let(:assigner) { TopicAssigner.new(topic, moderator) }
+    let(:moderator2) { Fabricate(:moderator) }
+    let(:assigner) { TopicAssigner.new(topic, moderator2) }
+    let(:assigner_self) { TopicAssigner.new(topic, moderator) }
 
     it "can assign and unassign correctly" do
-      assigner.assign(moderator)
+
+      messages = MessageBus.track_publish("/notification-alert/#{moderator.id}") do
+        assigner.assign(moderator)
+      end
+
+      expect(messages.length).to eq(1)
+      expect(messages.first.data[:excerpt]).to eq("@#{moderator2.username} assigned you the topic '#{topic.title}'")
 
       expect(TopicQuery.new(
         moderator, assigned: moderator.username
@@ -55,7 +63,7 @@ RSpec.describe TopicAssigner do
       )
 
       expect do
-        assigner.assign(moderator)
+        assigner_self.assign(moderator)
       end.to_not change { TopicUser.last.notifications_reason_id }
     end

GitHub

2 Likes

Should we do this now?

Sure I say once 2.3 is released.

1 Like