From cc180b77ce03eadd0868e9f94cc5d628a57d8222 Mon Sep 17 00:00:00 2001
From: Subin T P <subinthattaparambil@gmail.com>
Date: Tue, 14 Jan 2020 00:50:18 +0530
Subject: [PATCH] Include only incoming/outgoing messages in reporting (#429)

* Skip activity messages from reporting

* Makes reportable? as access list for reporting
---
 app/listeners/reporting_listener.rb       | 10 +++---
 app/models/message.rb                     |  4 +++
 spec/listeners/reporting_listener_spec.rb | 39 +++++++++++++++++++++++
 3 files changed, 48 insertions(+), 5 deletions(-)
 create mode 100644 spec/listeners/reporting_listener_spec.rb

diff --git a/app/listeners/reporting_listener.rb b/app/listeners/reporting_listener.rb
index 8032aeab5..14027783f 100644
--- a/app/listeners/reporting_listener.rb
+++ b/app/listeners/reporting_listener.rb
@@ -25,10 +25,10 @@ class ReportingListener < BaseListener
 
   def message_created(event)
     message, account, timestamp = extract_message_and_account(event)
-    if message.outgoing?
-      ::Reports::UpdateAccountIdentity.new(account, timestamp).incr_outgoing_messages_count
-    else
-      ::Reports::UpdateAccountIdentity.new(account, timestamp).incr_incoming_messages_count
-    end
+
+    return unless message.reportable?
+
+    identity = ::Reports::UpdateAccountIdentity.new(account, timestamp)
+    message.outgoing? ? identity.incr_outgoing_messages_count : identity.incr_incoming_messages_count
   end
 end
diff --git a/app/models/message.rb b/app/models/message.rb
index a40c76efd..8221fffe8 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -66,6 +66,10 @@ class Message < ApplicationRecord
     data
   end
 
+  def reportable?
+    incoming? || outgoing?
+  end
+
   private
 
   def dispatch_event
diff --git a/spec/listeners/reporting_listener_spec.rb b/spec/listeners/reporting_listener_spec.rb
new file mode 100644
index 000000000..856a730c7
--- /dev/null
+++ b/spec/listeners/reporting_listener_spec.rb
@@ -0,0 +1,39 @@
+require 'rails_helper'
+describe ReportingListener do
+  let(:listener) { described_class.instance }
+  let!(:account) { create(:account) }
+  let(:report_identity) { Reports::UpdateAccountIdentity.new(account, Time.zone.now) }
+  let!(:user) { create(:user, account: account) }
+  let!(:inbox) { create(:inbox, account: account) }
+  let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) }
+
+  describe '#message_created' do
+    let(:event_name) { :'conversation.created' }
+
+    context 'when user activity message' do
+      it 'does not increment messages count' do
+        activity_message = create(:message, message_type: 'activity', account: account, inbox: inbox, conversation: conversation)
+        event = Events::Base.new(event_name, Time.zone.now, message: activity_message)
+
+        allow(Reports::UpdateAccountIdentity).to receive(:new).and_return(report_identity)
+        allow(report_identity).to receive(:incr_outgoing_messages_count).exactly(0).times
+        allow(report_identity).to receive(:incr_incoming_messages_count).exactly(0).times
+
+        listener.message_created(event)
+      end
+    end
+
+    context 'when user conversation message' do
+      it 'increments messages count' do
+        conversation_message = create(:message, message_type: 'outgoing', account: account, inbox: inbox, conversation: conversation)
+        event = Events::Base.new(event_name, Time.zone.now, message: conversation_message)
+
+        allow(Reports::UpdateAccountIdentity).to receive(:new).and_return(report_identity)
+        allow(report_identity).to receive(:incr_outgoing_messages_count).once
+        allow(report_identity).to receive(:incr_incoming_messages_count).exactly(0).times
+
+        listener.message_created(event)
+      end
+    end
+  end
+end