From c4fb5eaaae9759e008bda0552fa58bf7a40d0979 Mon Sep 17 00:00:00 2001 From: lpichler Date: Mon, 14 May 2018 09:57:08 +0200 Subject: [PATCH] Filter relevant fields also according to chargeback class in Chargeback --- app/models/chargeable_field.rb | 7 +++++++ app/models/chargeback.rb | 4 ++-- app/models/chargeback_rate.rb | 10 +++++++--- app/models/chargeback_rate_detail.rb | 2 ++ spec/models/chargeback_rate_spec.rb | 12 ++++++++++++ spec/models/chargeback_vm_spec.rb | 10 ++++++++++ 6 files changed, 40 insertions(+), 5 deletions(-) diff --git a/app/models/chargeable_field.rb b/app/models/chargeable_field.rb index 258e8b796e2..3fc952278e1 100644 --- a/app/models/chargeable_field.rb +++ b/app/models/chargeable_field.rb @@ -66,6 +66,13 @@ def metric_key(sub_metric = nil) "#{rate_name}_#{sub_metric ? sub_metric + '_' : ''}metric" # metric value (e.g. Storage [Used|Allocated|Fixed]) end + # Fixed metric has _1 or _2 in name but column + # fixed_compute_metric is used in report and calculations + # TODO: remove and unify with metric_key + def metric_column_key + fixed? ? metric_key.gsub(/\_1|\_2/, '') : metric_key + end + def cost_keys(sub_metric = nil) keys = ["#{rate_name}_#{sub_metric ? sub_metric + '_' : ''}cost", # cost associated with metric (e.g. Storage [Used|Allocated|Fixed] Cost) 'total_cost'] diff --git a/app/models/chargeback.rb b/app/models/chargeback.rb index 958e5558bdc..19889e85eec 100644 --- a/app/models/chargeback.rb +++ b/app/models/chargeback.rb @@ -101,7 +101,7 @@ def new_chargeback_calculate_costs(consumption, rates) :resource => MiqEnterprise.first) data = {} - rate.rate_details_relevant_to(relevant_fields).each do |r| + rate.rate_details_relevant_to(relevant_fields, self.class.attribute_names).each do |r| r.populate_showback_rate(plan, r, showback_category) measure = r.chargeable_field.showback_measure dimension, _, _ = r.chargeable_field.showback_dimension @@ -137,7 +137,7 @@ def calculate_costs(consumption, rates) self.class.try(:refresh_dynamic_metric_columns) rates.each do |rate| - rate.rate_details_relevant_to(relevant_fields).each do |r| + rate.rate_details_relevant_to(relevant_fields, self.class.attribute_names).each do |r| r.charge(relevant_fields, consumption, @options).each do |field, value| next unless self.class.attribute_names.include?(field) self[field] = (self[field] || 0) + value diff --git a/app/models/chargeback_rate.rb b/app/models/chargeback_rate.rb index f327fe760f5..8dcaab71402 100644 --- a/app/models/chargeback_rate.rb +++ b/app/models/chargeback_rate.rb @@ -35,9 +35,13 @@ def self.tag_class(klass) super(klass) end - def rate_details_relevant_to(report_cols) - # we can memoize, as we get the same report_cols thrrough the life of the object - @relevant ||= chargeback_rate_details.select { |r| r.affects_report_fields(report_cols) } + def rate_details_relevant_to(report_cols, allowed_cols) + # we can memoize, as we get the same report_cols through the life of the object + @relevant ||= begin + chargeback_rate_details.select do |r| + r.affects_report_fields(report_cols) && allowed_cols.include?(r.metric_column_key) + end + end end def self.validate_rate_type(type) diff --git a/app/models/chargeback_rate_detail.rb b/app/models/chargeback_rate_detail.rb index ba0ed4e3d93..2fb9687dfb2 100644 --- a/app/models/chargeback_rate_detail.rb +++ b/app/models/chargeback_rate_detail.rb @@ -14,6 +14,8 @@ class ChargebackRateDetail < ApplicationRecord delegate :metric_key, :cost_keys, :to => :chargeable_field + delegate :metric_column_key, :metric_key, :cost_keys, :rate_key, :to => :chargeable_field + FORM_ATTRIBUTES = %i(description per_time per_unit metric group source metric chargeable_field_id sub_metric).freeze PER_TIME_TYPES = { "hourly" => _("Hourly"), diff --git a/spec/models/chargeback_rate_spec.rb b/spec/models/chargeback_rate_spec.rb index 7215f741c57..ae52dc08c3e 100644 --- a/spec/models/chargeback_rate_spec.rb +++ b/spec/models/chargeback_rate_spec.rb @@ -1,4 +1,16 @@ describe ChargebackRate do + describe "#rate_details_relevant_to" do + let(:count_hourly_variable_tier_rate) { {:variable_rate => '10'} } + + let!(:chargeback_rate) do + FactoryGirl.create(:chargeback_rate, :detail_params => {:chargeback_rate_detail_cpu_cores_allocated => {:tiers => [count_hourly_variable_tier_rate]}}) + end + + it "skips cpu_cores_allocated column" do + expect(chargeback_rate.rate_details_relevant_to(['total_cost'].to_set, ChargebackVm.attribute_names)).to be_empty + end + end + context ".validate_rate_type" do it "handles valid types" do [:compute, :storage, 'compute', 'storage', 'Compute', 'Storage'].each do |type| diff --git a/spec/models/chargeback_vm_spec.rb b/spec/models/chargeback_vm_spec.rb index eec9effd99f..b58d8837c31 100644 --- a/spec/models/chargeback_vm_spec.rb +++ b/spec/models/chargeback_vm_spec.rb @@ -640,6 +640,16 @@ def result_row_for_vm(vm) end end + context "chargeback rate contains rate unrelated to chargeback vm" do + let!(:chargeback_rate) do + FactoryGirl.create(:chargeback_rate, :detail_params => detail_params.merge(:chargeback_rate_detail_cpu_cores_allocated => {:tiers => [count_hourly_variable_tier_rate]})) + end + + it "skips unrelated columns and calculate related columns" do + expect(subject.cpu_allocated_metric).to eq(cpu_count) + end + end + it "cpu" do expect(subject.cpu_allocated_metric).to eq(cpu_count) used_metric = used_average_for(:cpu_usagemhz_rate_average, hours_in_month, @vm1)