chore: Fix issues with notification actors (#1807)

app/listeners/notification_listener.rb

@@ -41,7 +41,7 @@ class NotificationListener < BaseListener
       notification_type: 'assigned_conversation_new_message',
       user: conversation.assignee,
       account: account,
-      primary_actor: conversation
+      primary_actor: message
     ).perform
   end
 

app/models/notification.rb

@@ -44,12 +44,13 @@ class Notification < ApplicationRecord
   PRIMARY_ACTORS = ['Conversation'].freeze
 
   def push_event_data
+    # Secondary actor could be nil for cases like system assigning conversation
     {
       id: id,
       notification_type: notification_type,
       primary_actor_type: primary_actor_type,
       primary_actor_id: primary_actor_id,
-      primary_actor: primary_actor&.push_event_data,
+      primary_actor: primary_actor.push_event_data,
       read_at: read_at,
       secondary_actor: secondary_actor&.push_event_data,
       user: user&.push_event_data,
@@ -59,7 +60,6 @@ class Notification < ApplicationRecord
   end
 
   # TODO: move to a data presenter
-  # rubocop:disable Metrics/CyclomaticComplexity
   def push_message_title
     case notification_type
     when 'conversation_creation'
@@ -69,19 +69,18 @@ class Notification < ApplicationRecord
     when 'assigned_conversation_new_message'
       I18n.t(
         'notifications.notification_title.assigned_conversation_new_message',
-        display_id: primary_actor.display_id,
-        content: primary_actor&.messages&.incoming&.last&.content
+        display_id: conversation.display_id,
+        content: primary_actor.content.truncate_words(10)
       )
     when 'conversation_mention'
-      I18n.t('notifications.notification_title.conversation_mention', display_id: primary_actor.conversation.display_id, name: secondary_actor.name)
+      I18n.t('notifications.notification_title.conversation_mention', display_id: conversation.display_id, name: secondary_actor.name)
     else
       ''
     end
   end
-  # rubocop:enable Metrics/CyclomaticComplexity
 
   def conversation
-    return primary_actor.conversation if ['conversation_mention'].include? notification_type
+    return primary_actor.conversation if %w[assigned_conversation_new_message conversation_mention].include? notification_type
 
     primary_actor
   end

app/views/api/v1/accounts/notifications/index.json.jbuilder

@@ -11,18 +11,19 @@ json.data do
       json.notification_type notification.notification_type
       json.push_message_title notification.push_message_title
       # TODO: front end assumes primary actor to be conversation. should fix in future
-      if notification.notification_type == 'conversation_mention'
+      if %w[assigned_conversation_new_message conversation_mention].include? notification.notification_type
         json.primary_actor_type 'Conversation'
-        json.primary_actor_id notification.primary_actor.conversation_id
-        json.primary_actor notification.primary_actor&.conversation&.push_event_data
+        json.primary_actor_id notification.conversation.id
+        json.primary_actor notification.conversation.push_event_data
       else
         json.primary_actor_type notification.primary_actor_type
         json.primary_actor_id notification.primary_actor_id
-        json.primary_actor notification.primary_actor&.push_event_data
+        json.primary_actor notification.primary_actor.push_event_data
       end
       json.read_at notification.read_at
+      # Secondary actor could be nil for cases like system assigning conversation
       json.secondary_actor notification.secondary_actor&.push_event_data
-      json.user notification.user&.push_event_data
+      json.user notification.user.push_event_data
       json.created_at notification.created_at.to_i
     end
   end

db/migrate/20210219085719_remove_old_notifications.rb

@@ -0,0 +1,5 @@
+class RemoveOldNotifications < ActiveRecord::Migration[6.0]
+  def change
+    Notification.where(notification_type: 'assigned_conversation_new_message').destroy_all
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_02_17_154129) do
+ActiveRecord::Schema.define(version: 2021_02_19_085719) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"

spec/models/notification_spec.rb

@@ -34,9 +34,11 @@ RSpec.describe Notification do
     end
 
     it 'returns appropriate title suited for the notification type assigned_conversation_new_message' do
-      notification = create(:notification, notification_type: 'assigned_conversation_new_message')
+      message = create(:message, sender: create(:user), content: Faker::Lorem.paragraphs(number: 2))
+      notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: message)
 
-      expect(notification.push_message_title).to eq "[New message] - ##{notification.primary_actor.display_id} "
+      expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} \
+#{message.content.truncate_words(10)}"
     end
 
     it 'returns appropriate title suited for the notification type conversation_mention' do