Skip to content

Commit

Permalink
Make our naive 1.month == 30.days more explicit
Browse files Browse the repository at this point in the history
Rails 5.1 switched from julian to gregorian calendar so 1.month and
1.year changed in value.

Note,  duration_of_report_step consumers should be 30.days and sometimes 1.month, ugh

rails/rails#27610 (comment)
rails/rails@2a5ae2b
Rails PR: #29971
  • Loading branch information
jrafanie committed Oct 23, 2018
1 parent 4d79f9c commit 8a5273e
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 5 deletions.
5 changes: 4 additions & 1 deletion app/models/chargeback.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,14 @@ def new_chargeback_calculate_costs(consumption, rates)
data[measure][dimension] = [value, r.showback_unit(ChargeableField::UNITS[r.chargeable_field.metric])]
end

# TODO: duration_of_report_step is 30.days for price plans but for consumption history,
# it's used for date ranges and needs to be 1.month with rails 5.1
duration = @options.interval == "monthly" ? 30.days : @options.duration_of_report_step
results = plan.calculate_list_of_costs_input(resource_type: showback_category,
data: data,
start_time: consumption.instance_variable_get("@start_time"),
end_time: consumption.instance_variable_get("@end_time"),
cycle_duration: @options.duration_of_report_step)
cycle_duration: duration)

results.each do |cost_value, sb_rate|
r = ChargebackRateDetail.find(sb_rate.concept)
Expand Down
4 changes: 2 additions & 2 deletions app/models/chargeback/consumption.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def consumed_hours_in_interval
def hours_in_month
# If the interval is monthly, we have use exact number of days in interval (i.e. 28, 29, 30, or 31)
# othewise (for weekly and daily intervals) we assume month equals to 30 days
monthly? ? hours_in_interval : (1.month / 1.hour)
monthly? ? hours_in_interval : (30.days / 1.hour)
end

def consumption_start
Expand All @@ -45,7 +45,7 @@ def hours_in_interval

def monthly?
# A heuristic. Is the interval lenght about 30 days?
(hours_in_interval * 1.hour - 1.month).abs < 3.days
(hours_in_interval * 1.hour - 30.days).abs < 3.days
end

def born_at
Expand Down
4 changes: 2 additions & 2 deletions spec/models/chargeback_rate_detail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
end
end

let(:consumption) { instance_double('Consumption', :hours_in_month => (1.month / 1.hour)) }
let(:consumption) { instance_double('Consumption', :hours_in_month => (30.days / 1.hour)) }

it '#hourly_cost' do
cvalue = 42.0
Expand Down Expand Up @@ -169,7 +169,7 @@
let(:monthly_rate) { FactoryGirl.build(:chargeback_rate_detail, :per_time => 'monthly') }
let(:weekly_consumption) { Chargeback::ConsumptionWithRollups.new([], Time.current - 1.week, Time.current) }
it 'monhtly rate returns correct hourly(_rate) when consumption slice is weekly' do
expect(monthly_rate.hourly(rate, weekly_consumption)).to eq(rate / (1.month / 1.hour))
expect(monthly_rate.hourly(rate, weekly_consumption)).to eq(rate / (30.days / 1.hour))
end
end

Expand Down
27 changes: 27 additions & 0 deletions spec/models/chargeback_vm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1398,3 +1398,30 @@ def find_result_by_vm_name_and_region(chargeback_result, vm_name, region)
include_examples "ChargebackVm", :skip
end
end

describe Chargeback::Consumption do
context "#hours_in_month" do
before do
@start_time = Time.parse("2017-10-1 15:53:55 -0400")
end

[
[33.days, 720],
[32.days, 768],
[31.days, 744],
[30.days, 720],
[29.days, 696],
[28.days, 672],
[20.days, 720],
[5.days, 720]
].each do |duration, expected|
it "next #{duration / 1.day}.days" do
expect(described_class.new(@start_time, @start_time + duration).hours_in_month).to eq(expected)
end

it "prior #{duration / 1.day}.days" do
expect(described_class.new(@start_time - duration, @start_time).hours_in_month).to eq(expected)
end
end
end
end

0 comments on commit 8a5273e

Please sign in to comment.