From e56d5fdeaef3c139c97a6f960e0cc766586c17f3 Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 16 Sep 2022 01:27:07 +0100 Subject: [PATCH 1/7] Update report.rb Add a parameter to return raw SQL results --- lib/active_reporting/report.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/active_reporting/report.rb b/lib/active_reporting/report.rb index ad4f159..39f500e 100644 --- a/lib/active_reporting/report.rb +++ b/lib/active_reporting/report.rb @@ -14,7 +14,7 @@ class Report extend Forwardable def_delegators :@metric, :fact_model, :model - def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimensions: [], metric_filter: {}) + def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimensions: [], metric_filter: {}, raw_results: false) @metric = metric.is_a?(Metric) ? metric : ActiveReporting.fetch_metric(metric) raise UnknownMetric, "Unknown metric #{metric}" if @metric.nil? @@ -24,6 +24,7 @@ def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimens @metric_filter = @metric.metric_filter.merge(metric_filter) @ordering = @metric.order_by_dimension partition_dimension_filters dimension_filter + @raw_results = raw_results end # Builds and executes a query, returning the raw result @@ -36,8 +37,11 @@ def run private ###################################################################### def build_data - @data = model.connection.exec_query(statement.to_sql).to_a - apply_dimension_callbacks + @data = model.connection.exec_query(statement.to_sql) + unless @raw_results + @data = @data.to_a + apply_dimension_callbacks + end @data end From fd1ae98fd0b359f38ac2a704cc9a7474bf6b028d Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 16 Sep 2022 11:38:34 +0100 Subject: [PATCH 2/7] Group Results If requested, group the metric by dimensions --- lib/active_reporting/report.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/active_reporting/report.rb b/lib/active_reporting/report.rb index 39f500e..cb2fc58 100644 --- a/lib/active_reporting/report.rb +++ b/lib/active_reporting/report.rb @@ -14,7 +14,7 @@ class Report extend Forwardable def_delegators :@metric, :fact_model, :model - def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimensions: [], metric_filter: {}, raw_results: false) + def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimensions: [], metric_filter: {}, group_results: false) @metric = metric.is_a?(Metric) ? metric : ActiveReporting.fetch_metric(metric) raise UnknownMetric, "Unknown metric #{metric}" if @metric.nil? @@ -24,7 +24,7 @@ def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimens @metric_filter = @metric.metric_filter.merge(metric_filter) @ordering = @metric.order_by_dimension partition_dimension_filters dimension_filter - @raw_results = raw_results + @group_results = group_results end # Builds and executes a query, returning the raw result @@ -37,10 +37,11 @@ def run private ###################################################################### def build_data - @data = model.connection.exec_query(statement.to_sql) - unless @raw_results - @data = @data.to_a - apply_dimension_callbacks + @data = model.connection.exec_query(statement.to_sql).to_a + apply_dimension_callbacks + if @group_results + dimension_label_names = @dimensions.map { |d| d.instance_variable_get(:@label_name).to_s } + @data = Hash[@data.map { |r| [ r.fetch_values(*dimension_label_names), r.fetch(@metric.name.to_s)] }] end @data end From 77fcd2e460d4d14ae037578f2de29516638864d7 Mon Sep 17 00:00:00 2001 From: Neil Jobbins Date: Fri, 16 Sep 2022 12:56:31 +0100 Subject: [PATCH 3/7] Allow the `label_name` to be read from outside the `reporting_dimension` class and use it to build the grouped dimensions. --- lib/active_reporting/report.rb | 2 +- lib/active_reporting/reporting_dimension.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/active_reporting/report.rb b/lib/active_reporting/report.rb index cb2fc58..467047c 100644 --- a/lib/active_reporting/report.rb +++ b/lib/active_reporting/report.rb @@ -40,7 +40,7 @@ def build_data @data = model.connection.exec_query(statement.to_sql).to_a apply_dimension_callbacks if @group_results - dimension_label_names = @dimensions.map { |d| d.instance_variable_get(:@label_name).to_s } + dimension_label_names = @dimensions.map { |d| d.label_name.to_s } @data = Hash[@data.map { |r| [ r.fetch_values(*dimension_label_names), r.fetch(@metric.name.to_s)] }] end @data diff --git a/lib/active_reporting/reporting_dimension.rb b/lib/active_reporting/reporting_dimension.rb index 581ae4f..ab4010f 100644 --- a/lib/active_reporting/reporting_dimension.rb +++ b/lib/active_reporting/reporting_dimension.rb @@ -10,7 +10,7 @@ class ReportingDimension DATETIME_HIERARCHIES = %i[microseconds milliseconds second minute hour day week month quarter year decade century millennium date].freeze JOIN_METHODS = { joins: :joins, left_outer_joins: :left_outer_joins }.freeze - attr_reader :join_method, :label + attr_reader :join_method, :label, :label_name def_delegators :@dimension, :name, :type, :klass, :association, :model, :hierarchical? From ceec712721a6e7c73b313429177083a6ee045370 Mon Sep 17 00:00:00 2001 From: Neil Jobbins Date: Fri, 16 Sep 2022 14:35:52 +0100 Subject: [PATCH 4/7] If we dont have any dimensions then group by the metric name. --- lib/active_reporting/report.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/active_reporting/report.rb b/lib/active_reporting/report.rb index 467047c..1eab594 100644 --- a/lib/active_reporting/report.rb +++ b/lib/active_reporting/report.rb @@ -40,8 +40,12 @@ def build_data @data = model.connection.exec_query(statement.to_sql).to_a apply_dimension_callbacks if @group_results - dimension_label_names = @dimensions.map { |d| d.label_name.to_s } - @data = Hash[@data.map { |r| [ r.fetch_values(*dimension_label_names), r.fetch(@metric.name.to_s)] }] + if @dimensions.any? + dimension_label_names = @dimensions.map { |d| d.label_name.to_s } + @data = Hash[@data.map { |r| [ r.fetch_values(*dimension_label_names), r.fetch(@metric.name.to_s)] }] + else + @data = Hash[@data.map { |r| [ r.keys, r.fetch(@metric.name.to_s)] }] + end end @data end From 3dd2fe1e89edd0a74070eb8707d5a72c18aad781 Mon Sep 17 00:00:00 2001 From: Neil Jobbins Date: Fri, 23 Sep 2022 14:38:03 +0100 Subject: [PATCH 5/7] refactor: group_results to data_format refactor: extract data formatting logic from `build_data` into its own private method --- lib/active_reporting/report.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/active_reporting/report.rb b/lib/active_reporting/report.rb index 1eab594..fff2703 100644 --- a/lib/active_reporting/report.rb +++ b/lib/active_reporting/report.rb @@ -14,7 +14,7 @@ class Report extend Forwardable def_delegators :@metric, :fact_model, :model - def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimensions: [], metric_filter: {}, group_results: false) + def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimensions: [], metric_filter: {}, data_format: :standard) @metric = metric.is_a?(Metric) ? metric : ActiveReporting.fetch_metric(metric) raise UnknownMetric, "Unknown metric #{metric}" if @metric.nil? @@ -24,7 +24,7 @@ def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimens @metric_filter = @metric.metric_filter.merge(metric_filter) @ordering = @metric.order_by_dimension partition_dimension_filters dimension_filter - @group_results = group_results + @data_format = data_format end # Builds and executes a query, returning the raw result @@ -39,15 +39,22 @@ def run def build_data @data = model.connection.exec_query(statement.to_sql).to_a apply_dimension_callbacks - if @group_results + format_data unless @data_format == :standard + @data + end + + def format_data + case @data_format + when :grouped if @dimensions.any? dimension_label_names = @dimensions.map { |d| d.label_name.to_s } @data = Hash[@data.map { |r| [ r.fetch_values(*dimension_label_names), r.fetch(@metric.name.to_s)] }] else @data = Hash[@data.map { |r| [ r.keys, r.fetch(@metric.name.to_s)] }] end + else + raise UnknownDataFormat end - @data end def partition_dimension_filters(user_dimension_filter) From 97bdbdc3c683dc80f74bf6158ab5c1350e607f82 Mon Sep 17 00:00:00 2001 From: Neil Jobbins Date: Fri, 23 Sep 2022 14:39:06 +0100 Subject: [PATCH 6/7] feat: Add the ability to pass a block to `@report.run` --- lib/active_reporting/report.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/active_reporting/report.rb b/lib/active_reporting/report.rb index fff2703..eeb9708 100644 --- a/lib/active_reporting/report.rb +++ b/lib/active_reporting/report.rb @@ -29,9 +29,12 @@ def initialize(metric, dimension_identifiers: true, dimension_filter: {}, dimens # Builds and executes a query, returning the raw result # - # @return [Array] + # @return [Array, Hash] def run @run ||= build_data + + # Pass format as a block + block_given? ? yield(@metric, @dimensions, @run) : @run end private ###################################################################### From a33262085cee19a061ad5b79b259a08544df215b Mon Sep 17 00:00:00 2001 From: Neil Jobbins Date: Fri, 23 Sep 2022 14:39:50 +0100 Subject: [PATCH 7/7] test: Add some tests to demo the implementations --- test/active_reporting/report_test.rb | 48 ++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/active_reporting/report_test.rb b/test/active_reporting/report_test.rb index bc0f0fe..fc62e14 100644 --- a/test/active_reporting/report_test.rb +++ b/test/active_reporting/report_test.rb @@ -52,4 +52,52 @@ def test_accept_dimension_join_method_option report = ActiveReporting::Report.new(metric) assert report.send(:statement).to_sql.include?("LEFT OUTER JOIN") end + + ### test_data_format_grouped + # The standard data format returns an array of hashes (records). + # Specifying 'data_format: grouped' should return metric values grouped by metric dimensions e.g. + # ----- Data Format: Standard ----- + # [{"a_metric"=>26, "platform"=>"3DS"}, {"a_metric"=>1, "platform"=>"Switch"}, {"a_metric"=>20, "platform"=>"Wii U"}] + # ----- Data Format: Grouped ----- + # {["3DS"]=>26, ["Switch"]=>1, ["Wii U"]=>20} + def test_data_format_grouped_single_dimension + metric = ActiveReporting::Metric.new(:a_metric, fact_model: GameFactModel, dimensions: [:platform], aggregate: :count) + standard_report = ActiveReporting::Report.new(metric, dimension_identifiers: false) + standard_data = standard_report.run + assert standard_data.is_a?(Array) + + grouped_report = ActiveReporting::Report.new(metric, dimension_identifiers: false, data_format: :grouped) + grouped_data = grouped_report.run + assert grouped_data.is_a?(Hash) + + # Group the standard result so we can check we have the right result + metric_name = metric.instance_variable_get(:@name) + dimension_names = standard_report.instance_variable_get(:@dimensions).map { |d| d.instance_variable_get(:@label_name).to_s } + standard_grouped = Hash[standard_data.map { |r| [ r.fetch_values(*dimension_names), r.fetch(metric_name.to_s)] }] + + assert_equal standard_grouped, grouped_data + end + + def test_run_with_a_block + metric = ActiveReporting::Metric.new(:a_metric, fact_model: GameFactModel, dimensions: [:platform], aggregate: :count) + report = ActiveReporting::Report.new(metric, dimension_identifiers: false) + standard_data = report.run + assert standard_data.is_a?(Array) + + grouped_data = report.run do |metric, dimensions, data| + if dimensions.any? + dimension_label_names = dimensions.map { |d| d.label_name.to_s } + Hash[data.map { |r| [ r.fetch_values(*dimension_label_names), r.fetch(metric.name.to_s)] }] + else + Hash[data.map { |r| [ r.keys, r.fetch(metric.name.to_s)] }] + end + end + assert grouped_data.is_a?(Hash) + + # Group the standard result so we can check we have the right result + metric_name = metric.instance_variable_get(:@name) + dimension_names = report.instance_variable_get(:@dimensions).map { |d| d.instance_variable_get(:@label_name).to_s } + standard_grouped = Hash[standard_data.map { |r| [ r.fetch_values(*dimension_names), r.fetch(metric_name.to_s)] }] + assert_equal standard_grouped, grouped_data + end end