WIP: Group results

[TheRealNeil]

Continuing with Issue #41 I created a PR to demonstrate how we could group metrics by the dimensions to allow for easy charting by some popular gems e.g. chartkick.

The report object has a new option called group_results which can be passed true or false(default). The default behaviour is unchanged and will return an Array of Hashes with metric name / value and dimension labels / values e.g.

[ { metric_name => metric_value, dimension_1_label => dimension_1_value, dimension_2_label => dimension_2_value } ]

Setting group_results to true will return a hash with the keys containing the dimensions values and the values containing the metrics values e.g.

{ [ dimension_1_value, dimension_2_value ] => metric_value }

If no dimensions have been defined in the report, it groups by the metric label e.g.

{ [ metric_name ] => metric_value }

This could be implemented as application logic in which case I think it would be helpful to expose the dimension names and metric names on the report object. This could help turn this

metric = @report.instance_variable_get(:@metric).instance_variable_get(:@name)
dimensions = @report.instance_variable_get(:@dimensions).map { |d| d.instance_variable_get(:@label_name).to_s }
Hash[@report.run.map { |r| [ r.fetch_values(*dimensions), r.fetch(metric.to_s)] }]

into this

Hash[@report.run.map { |r| [ r.fetch_values(*@report.dimension_label_names), r.fetch(@report.metric_name)] }]

I have not written any tests yet. I will look into this if the basic direction is okay.

[t27duck]

I'm not against this, but I still have issues with naming... So this would introduce a single bool to deviate from the normal output for one usecase. I feel like if this gem were to provide alternative outputs, this could be generalized a bit.

For example, maybe instead of a bool, it would be called something like output or output_mode or... something better and take maybe a string or symbol to specify what the output would be (defaults to like, nil or "normal" or w/e). #build_data would then look at that value and call an appropriate method to take and return the formatted results.

Something like...

ActiveReporting::Report.new(..., format: :values_only)

Or... something.

🤷‍♂️

[TheRealNeil]

Hi @t27duck, I have refactored the params in line with your suggestions so the report now takes a symbol as a param called data_format e.g.

ActiveReporting::Report.new(..., data_format: :grouped)

In addition I thought about allowing a block to be passed to the ActiveReporting::Report.run method that yields the metric, dimensions and data which can then be used to format as required.

@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

I am not 100% sure about this, it works okay but I have been calling @report.run in views and this would not be great in that scenario. Anyway, I am interested to hear what you think.

I implemented a couple of tests to test and show how each of the implementations could be used.

[t27duck]

Passing the block does seem awkward... though I can see some utility in it. My concern is we're passing three things into the block and I can see a "need" in the future to pass in more "stuff"...

The metric and raw data is probably the most important things people would need. Maybe we make the argument order in the yield metric, data, and "other stuff" after those.

If you can update the readme explaining these options with maybe an example, that would be great as well. Beyond that, this could be good to go.