Merge pull request #1572 from yeti-switch/1569-interval-report-except…

…ion-on-empty-group-by

#1569, fix when Render data of "Report Interval Cdr" with empty group_by

app/forms/report/interval_cdr_form.rb

@@ -9,7 +9,7 @@ class IntervalCdrForm < BaseForm
     attribute :filter, :string
     attribute :aggregate_by, :string
     attribute :interval_length, :integer
-    attribute :group_by, :string, array: { reject_blank: true }
+    attribute :group_by, :string, array: { reject_blank: true }, default: []
     attribute :send_to, :integer, array: { reject_blank: true }
 
     validate :validate_group_by
@@ -37,7 +37,7 @@ def _save
         aggregate_by: aggregate_by,
         interval_length: interval_length,
         filter: filter.presence,
-        group_by: group_by.presence,
+        group_by: group_by,
         send_to: send_to.presence
       )
     rescue CreateReport::CustomCdr::Error => e

app/models/report/interval_cdr.rb

@@ -10,7 +10,7 @@
 #  date_end        :timestamptz      not null
 #  date_start      :timestamptz      not null
 #  filter          :string
-#  group_by        :string           is an Array
+#  group_by        :string           default([]), not null, is an Array
 #  interval_length :integer(4)       not null
 #  send_to         :integer(4)       is an Array
 #  created_at      :timestamptz      not null

app/services/create_report/interval_cdr.rb

@@ -3,7 +3,7 @@
 module CreateReport
   class IntervalCdr < Base
     parameter :filter
-    parameter :group_by
+    parameter :group_by, default: []
     parameter :aggregation_function
     parameter :aggregate_by
     parameter :interval_length
@@ -18,7 +18,7 @@ def create_report!
         date_start: date_start,
         date_end: date_end,
         filter: filter.presence,
-        group_by: group_by.presence,
+        group_by: group_by,
         send_to: send_to.presence
       )
     end

db/cdr_migrate/20241006113650_change_default_for_cdr_interval_report.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class ChangeDefaultForCdrIntervalReport < ActiveRecord::Migration[7.0]
+  def change
+    change_column_default 'reports.cdr_interval_report', :group_by, from: nil, to: []
+  end
+end

db/cdr_migrate/20241006123022_update_reports_cdr_interval_report_to_change_null_group_by.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class UpdateReportsCdrIntervalReportToChangeNullGroupBy < ActiveRecord::Migration[7.0]
+  def up
+    execute "UPDATE reports.cdr_interval_report SET group_by = '{}' WHERE group_by IS NULL;"
+    change_column_null 'reports.cdr_interval_report', 'group_by', false
+  end
+
+  def down
+    change_column_null 'reports.cdr_interval_report', 'group_by', true
+    execute "UPDATE reports.cdr_interval_report SET group_by = NULL WHERE group_by = '{}';"
+  end
+end

db/cdr_structure.sql

@@ -2277,7 +2277,7 @@ CREATE TABLE reports.cdr_interval_report (
     date_start timestamp with time zone NOT NULL,
     date_end timestamp with time zone NOT NULL,
     filter character varying,
-    group_by character varying[],
+    group_by character varying[] DEFAULT '{}'::character varying[] NOT NULL,
     created_at timestamp with time zone NOT NULL,
     interval_length integer NOT NULL,
     aggregator_id integer NOT NULL,
@@ -4401,6 +4401,8 @@ INSERT INTO "public"."schema_migrations" (version) VALUES
 ('20240405165010'),
 ('20240411092931'),
 ('20240609092136'),
-('20240617084103');
+('20240617084103'),
+('20241006113650'),
+('20241006123022');
 
 

spec/factories/report/interval_cdrs.rb

@@ -10,7 +10,7 @@
 #  date_end        :timestamptz      not null
 #  date_start      :timestamptz      not null
 #  filter          :string
-#  group_by        :string           is an Array
+#  group_by        :string           default([]), not null, is an Array
 #  interval_length :integer(4)       not null
 #  send_to         :integer(4)       is an Array
 #  created_at      :timestamptz      not null

spec/features/reports/interval_cdrs/new_interval_cdr_spec.rb

@@ -31,7 +31,7 @@
       aggregate_by: 'destination_fee',
       interval_length: 10,
       send_to: nil,
-      group_by: nil,
+      group_by: [],
       filter: nil
     )
   end
@@ -106,7 +106,7 @@
                           aggregate_by: 'destination_fee',
                           interval_length: 10,
                           send_to: nil,
-                          group_by: nil,
+                          group_by: [],
                           filter: 'customer_id=123'
                         )
     end
@@ -132,7 +132,7 @@
                           aggregate_by: 'destination_fee',
                           interval_length: 10,
                           send_to: [contact.id],
-                          group_by: nil,
+                          group_by: [],
                           filter: nil
                         )
     end
@@ -159,7 +159,7 @@
                           aggregate_by: 'destination_fee',
                           interval_length: 10,
                           send_to: match_array([contacts.first.id, contacts.second.id]),
-                          group_by: nil,
+                          group_by: [],
                           filter: nil
                         )
     end

spec/forms/report/interval_cdr_form_spec.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+RSpec.describe Report::IntervalCdrForm do
+  subject { form.save }
+
+  let(:form) { described_class.new(form_attributes) }
+  let(:form_attributes) do
+    {
+      date_start: Time.current.to_fs(:db),
+      date_end: Time.current.to_fs(:db),
+      interval_length: 5,
+      aggregate_by: Report::IntervalCdr::CDR_AGG_COLUMNS.sample,
+      aggregator_id: Report::IntervalAggregator.take!.id
+    }
+  end
+
+  context 'when group_by is not filled' do
+    let(:form_attributes) { super().merge group_by: [''] }
+
+    it 'should create record with empty group_by' do
+      expect { subject }.to change(Report::IntervalCdr, :count).by(1)
+      expect(Report::IntervalCdr.take!).to have_attributes(group_by: [])
+    end
+  end
+
+  context 'when group_by is filled' do
+    let(:form_attributes) { super().merge group_by: Report::IntervalCdr::CDR_COLUMNS.first(2) }
+
+    it 'should create record with NOT empty group_by' do
+      expect { subject }.to change(Report::IntervalCdr, :count).by(1)
+      expect(Report::IntervalCdr.take!).to have_attributes group_by: Report::IntervalCdr::CDR_COLUMNS.first(2).map(&:to_s)
+    end
+  end
+end

spec/services/create_report/interval_cdr_spec.rb

@@ -80,23 +80,12 @@
     include_examples :creates_report
   end
 
-  context 'with group_by=null' do
-    let(:service_params) do
-      super().merge group_by: nil
-    end
-    let(:expected_report_attrs) do
-      super().merge group_by: nil
-    end
-
-    include_examples :creates_report
-  end
-
   context 'with group_by=[]' do
     let(:service_params) do
       super().merge group_by: []
     end
     let(:expected_report_attrs) do
-      super().merge group_by: nil
+      super().merge group_by: []
     end
 
     include_examples :creates_report

spec/services/generate_report_data/interval_cdr_spec.rb

@@ -24,7 +24,7 @@
       aggregation_function: aggregation_function,
       interval_length: 1440, # 24 hours
       aggregate_by: 'customer_price',
-      group_by: nil,
+      group_by: [],
       filter: nil
     }
   end