From 415bb23c37a0c5b709c0be5806235cba959d9f1b Mon Sep 17 00:00:00 2001
From: Vishnu Narayanan <vishnu@chatwoot.com>
Date: Thu, 12 Oct 2023 17:01:23 +0530
Subject: [PATCH] fix: Handle invalid metric in ReportsController (#8086)

Co-authored-by: Pranav Raj S <pranav@chatwoot.com>

Fixes CW-2643
---
 app/builders/v2/report_builder.rb       | 15 ++++++++++++-
 spec/builders/v2/report_builder_spec.rb | 28 +++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/app/builders/v2/report_builder.rb b/app/builders/v2/report_builder.rb
index c96950ee9..1442c5799 100644
--- a/app/builders/v2/report_builder.rb
+++ b/app/builders/v2/report_builder.rb
@@ -15,7 +15,10 @@ class V2::ReportBuilder
   end
 
   def timeseries
-    send(params[:metric])
+    return send(params[:metric]) if metric_valid?
+
+    Rails.logger.error "ReportBuilder: Invalid metric - #{params[:metric]}"
+    {}
   end
 
   # For backward compatible with old report
@@ -53,6 +56,16 @@ class V2::ReportBuilder
 
   private
 
+  def metric_valid?
+    %w[conversations_count
+       incoming_messages_count
+       outgoing_messages_count
+       avg_first_response_time
+       avg_resolution_time reply_time
+       resolutions_count
+       reply_time].include?(params[:metric])
+  end
+
   def inbox
     @inbox ||= account.inboxes.find(params[:id])
   end
diff --git a/spec/builders/v2/report_builder_spec.rb b/spec/builders/v2/report_builder_spec.rb
index 304e7bd97..936c070c4 100644
--- a/spec/builders/v2/report_builder_spec.rb
+++ b/spec/builders/v2/report_builder_spec.rb
@@ -171,6 +171,34 @@ describe V2::ReportBuilder do
         builder = described_class.new(account, params)
         expect { builder.timeseries }.to raise_error(ArgumentError)
       end
+
+      it 'logs error when metric is nil' do
+        params = {
+          metric: nil, # Set metric to nil to test this case
+          type: :account,
+          since: (Time.zone.today - 3.days).to_time.to_i.to_s,
+          until: Time.zone.today.end_of_day.to_time.to_i.to_s
+        }
+
+        builder = described_class.new(account, params)
+
+        expect(Rails.logger).to receive(:error).with('ReportBuilder: Invalid metric - ')
+        builder.timeseries
+      end
+
+      it 'calls the appropriate metric method for a valid metric' do
+        params = {
+          metric: 'not_conversation_count', # Provide a invalid metric
+          type: :account,
+          since: (Time.zone.today - 3.days).to_time.to_i.to_s,
+          until: Time.zone.today.end_of_day.to_time.to_i.to_s
+        }
+
+        builder = described_class.new(account, params)
+        expect(Rails.logger).to receive(:error).with('ReportBuilder: Invalid metric - not_conversation_count')
+
+        builder.timeseries
+      end
     end
 
     context 'when report type is label' do