diff --git a/app/models/chargeable_field.rb b/app/models/chargeable_field.rb index 522bd217f2b..db7cef5757b 100644 --- a/app/models/chargeable_field.rb +++ b/app/models/chargeable_field.rb @@ -70,6 +70,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 34d99c7b0b5..4142956193c 100644 --- a/app/models/chargeback.rb +++ b/app/models/chargeback.rb @@ -116,7 +116,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 @@ -152,7 +152,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].kind_of?(Numeric) ? (self[field] || 0) + value : value diff --git a/app/models/chargeback_rate.rb b/app/models/chargeback_rate.rb index 2c2d9ef6097..b09b38cd86e 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 d0d0ea387c7..3de5d303a33 100644 --- a/app/models/chargeback_rate_detail.rb +++ b/app/models/chargeback_rate_detail.rb @@ -12,7 +12,7 @@ class ChargebackRateDetail < ApplicationRecord delegate :rate_type, :to => :chargeback_rate, :allow_nil => true - delegate :metric_key, :cost_keys, :rate_key, :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 = { 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 4eae1145b58..4520c4f62c3 100644 --- a/spec/models/chargeback_vm_spec.rb +++ b/spec/models/chargeback_vm_spec.rb @@ -627,6 +627,16 @@ def result_row_for_vm(vm) subject { ChargebackVm.build_results_for_report_ChargebackVm(options).first.first } + 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)