use advanced instead of + semester duration...

use advanced instead of duration, as it is more timezone safe

fix and make tests with Time more explicit and timezone safe

fix format helper test

fix format helper test

remove tz_parse helper

app/helpers/format_helper.rb

@@ -4,7 +4,7 @@ module FormatHelper
   end
 
   def format_hours(hours)
-    t('activerecord.attributes.billing_expense.hours', count: hours.to_i)
+    t('activerecord.attributes.billing_expense.hours', count: hours)
   end
 
   def format_hours_semester(hours)

app/models/billing_expense.rb

@@ -3,7 +3,7 @@ class BillingExpense < ApplicationRecord
   include FullBankDetails
   include BillingExpenseSemesterUtils
 
-  SEMESTER_LENGTH = 6.months
+  SEMESTER_LENGTH = 6
 
   attr_accessor :import_mode
 
@@ -84,7 +84,7 @@ class BillingExpense < ApplicationRecord
         text: "#{semester_of_year(last_semester)}. Semester #{semester_display_year(last_semester)}"
       }
 
-      last_semester -= SEMESTER_LENGTH
+      last_semester.advance(months: - SEMESTER_LENGTH)
     end
 
     semesters

app/models/hour.rb

@@ -22,8 +22,7 @@ class Hour < ApplicationRecord
     if date.present?
       date = Time.zone.parse(date) unless date.is_a? Time
       return if date.blank?
-
-      date_between(:meeting_date, date, date + BillingExpense::SEMESTER_LENGTH)
+      date_between(:meeting_date, date, date.advance(months: BillingExpense::SEMESTER_LENGTH))
     end
   }
 

app/views/billing_expenses/new.html.slim

@@ -35,6 +35,6 @@ ul.list-inline
                 = volunteer.full_bank_details
               - else
                 span.label.label-danger Keine IBAN angegeben
-            td= link_to format_hours(volunteer.total_hours), volunteer_hours_path(volunteer)
+            td= link_to format_hours(volunteer.total_hours.to_i), volunteer_hours_path(volunteer)
             td= format_currency BillingExpense.amount_for(volunteer.total_hours)
             td= format_hours_semester volunteer.hours.semester(@selected_billing_semester).billable

test/helpers/format_helper_test.rb

@@ -7,35 +7,35 @@ class FormatHelperTest < ActionView::TestCase
   end
 
   test 'format_hours' do
-    assert_equal '1 Stunde', format_hours(1)
+    assert_equal 'eine Stunde', format_hours(1)
     assert_equal '1.1 Stunden', format_hours(1.1)
     assert_equal '0.9 Stunden', format_hours(0.9)
     assert_equal '2.35 Stunden', format_hours(2.35)
   end
 
   test 'format_hours_semester' do
-    assert_equal '1. Semester 2013 - 1. Semester 2015', format_hours_semester([
+    assert_equal '1. Semester 2013 – 1. Semester 2015', format_hours_semester([
       create(:hour, meeting_date: '2015-04-03'),
       create(:hour, meeting_date: '2014-05-06'),
       create(:hour, meeting_date: '2012-12-01')
     ])
-    assert_equal '1. Semester 2013 - 2. Semester 2015', format_hours_semester([
+    assert_equal '1. Semester 2013 – 2. Semester 2015', format_hours_semester([
       create(:hour, meeting_date: '2015-07-03'),
       create(:hour, meeting_date: '2014-05-06'),
       create(:hour, meeting_date: '2012-12-01')
     ])
-    assert_equal '2. Semester 2013 - 2. Semester 2015', format_hours_semester([
+    assert_equal '2. Semester 2013 – 2. Semester 2015', format_hours_semester([
       create(:hour, meeting_date: '2015-07-03'),
       create(:hour, meeting_date: '2014-05-06'),
       create(:hour, meeting_date: '2013-06-01')
     ])
-    assert_equal '2. Semester 2013 - 1. Semester 2015', format_hours_semester([
+    assert_equal '2. Semester 2013 – 1. Semester 2015', format_hours_semester([
       create(:hour, meeting_date: '2015-01-03'),
       create(:hour, meeting_date: '2014-05-06'),
       create(:hour, meeting_date: '2013-06-01')
     ])
 
-    assert_equal '1. - 2. Semester 2015', format_hours_semester([
+    assert_equal '1. – 2. Semester 2015', format_hours_semester([
       create(:hour, meeting_date: '2015-04-03'),
       create(:hour, meeting_date: '2015-09-06')
     ])

test/models/billing_expense_test.rb

@@ -16,23 +16,26 @@ class BillingExpenseTest < ActiveSupport::TestCase
     assert_equal 0, BillingExpense.amount_for(-999)
   end
 
-  test 'create_for!' do # rubocop:disable Metrics/BlockLength
-    volunteer1 = create :volunteer, bank: 'Bank 1'
-    hour1a = create :hour, volunteer: volunteer1
-    hour1b = create :hour, volunteer: volunteer1
-    hour1c = create :hour, volunteer: volunteer1
-    create :billing_expense, volunteer: volunteer1, hours: [hour1c]
+  test 'create_for' do # rubocop:disable Metrics/BlockLength
+    travel_to Time.zone.parse('2017-07-12')
+    volunteer1 = create :volunteer_with_user, bank: 'Bank 1'
+    other_creator = volunteer1.registrar
+    assignment1 = create :assignment, volunteer: volunteer1, creator: other_creator
+    hour1a = create :hour, volunteer: volunteer1, meeting_date: Time.zone.parse('2017-04-02'), hourable: assignment1
+    hour1b = create :hour, volunteer: volunteer1, meeting_date: Time.zone.parse('2017-05-12'), hourable: assignment1
+    hour1c = create :hour, volunteer: volunteer1, meeting_date: Time.zone.parse('2017-01-18'), hourable: assignment1
+    create :billing_expense, volunteer: volunteer1, hours: [hour1c], user: other_creator
 
-    volunteer2 = create :volunteer, bank: 'Bank 2'
-    hour2 = create :hour, volunteer: volunteer2, hours: 75
 
-    creator = create :user
+    volunteer2 = create :volunteer, bank: 'Bank 2'
+    creator = volunteer2.registrar
+    group_assignment1 = create :group_assignment, volunteer: volunteer2, creator: creator
+    hour2 = create :hour, volunteer: volunteer2, hours: 75, meeting_date: Time.zone.parse('2017-03-22'), hourable: group_assignment1
 
     assert_equal 1, BillingExpense.count
     assert_equal 1, volunteer1.billing_expenses.count
     assert_equal 0, volunteer2.billing_expenses.count
-
-    BillingExpense.create_for!(Volunteer.with_billable_hours, creator)
+    BillingExpense.create_for!(Volunteer.with_billable_hours('2016-12-01'), creator)
 
     volunteer1.reload
     volunteer2.reload
@@ -54,8 +57,9 @@ class BillingExpenseTest < ActiveSupport::TestCase
     assert_equal 50, billing_expense1.amount
     assert_equal 150, billing_expense2.amount
 
-    assert_equal [hour1a, hour1b], billing_expense1.hours.reorder(:id)
-    assert_equal [hour2], billing_expense2.hours
+    assert_includes billing_expense1.hours, hour1a
+    assert_includes billing_expense1.hours, hour1b
+    assert_includes billing_expense2.hours, hour2
 
     assert_equal 'Bank 1', billing_expense1.bank
     assert_equal volunteer1.iban, billing_expense1.iban
@@ -63,9 +67,9 @@ class BillingExpenseTest < ActiveSupport::TestCase
     assert_equal 'Bank 2', billing_expense2.bank
     assert_equal volunteer2.iban, billing_expense2.iban
 
-    assert_equal creator, hour1a.reload.reviewer
-    assert_equal creator, hour1b.reload.reviewer
-    refute_equal creator, hour1c.reload.reviewer
+    assert_equal creator, hour1a.reviewer
+    assert_equal creator, hour1b.reviewer
+    refute_equal creator, hour1c.reviewer
   end
 
   test 'generate_semester_filters without hours' do
@@ -101,21 +105,20 @@ class BillingExpenseTest < ActiveSupport::TestCase
   end
 
   test 'semester scope' do
-    date = Time.zone.now.beginning_of_year
-
     billing_expense1 = create :billing_expense,
-      hours: [create(:hour, meeting_date: date + 1.week)]
+      hours: [create(:hour, meeting_date: Time.zone.parse('2017-01-12'))]
 
     billing_expense2 = create :billing_expense,
       hours: [
-        create(:hour, meeting_date: date + 1.week),
-        create(:hour, meeting_date: date - 1.week)
+        create(:hour, meeting_date: Time.zone.parse('2017-02-01')),
+        create(:hour, meeting_date: Time.zone.parse('2017-05-12'))
       ]
 
     _billing_expense3 = create :billing_expense,
-      hours: [create(:hour, meeting_date: date - 1.week)]
+      hours: [create(:hour, meeting_date: Time.zone.parse('2016-11-30'))]
 
-    assert_equal [billing_expense1, billing_expense2], BillingExpense.semester(date).reorder(:id)
+    assert_includes BillingExpense.semester('2016-12-01'), billing_expense1
+    assert_includes BillingExpense.semester('2016-12-01'), billing_expense2
   end
 
   test 'amount can be overwriten' do

test/models/hour_test.rb

@@ -11,28 +11,32 @@ class HourTest < ActiveSupport::TestCase
   end
 
   test 'semester returns hours for a billing_expense semester' do
-    travel_to Time.zone.parse('2014-05-12')
-    a_semester_ago = BillingExpense::SEMESTER_LENGTH.ago
-    last_semester_ago = a_semester_ago - BillingExpense::SEMESTER_LENGTH
-    format = '%Y-%m-%d'
-
+    travel_to Time.zone.parse('2018-05-25')
     volunteer = create :volunteer
-    hour1 = create :hour, volunteer: volunteer, hours: 1, meeting_date: a_semester_ago - 1.month
-    hour2 = create :hour, volunteer: volunteer, hours: 1, meeting_date: a_semester_ago - 2.months
-    hour3 = create :hour, volunteer: volunteer, hours: 1, meeting_date: a_semester_ago + 1.month
-    hour4 = create :hour, volunteer: volunteer, hours: 1, meeting_date: a_semester_ago + 2.months
+    prev_hour1 = create :hour, volunteer: volunteer, hours: 1, meeting_date: Time.zone.parse('2016-11-15')
+    prev_hour2 = create :hour, volunteer: volunteer, hours: 1, meeting_date: Time.zone.parse('2016-10-01')
+    this_hour1 = create :hour, volunteer: volunteer, hours: 1, meeting_date: Time.zone.parse('2016-12-01')
+    this_hour2 = create :hour, volunteer: volunteer, hours: 1, meeting_date: Time.zone.parse('2017-05-11')
+    other_hour = create :hour, volunteer: volunteer, hours: 1, meeting_date: Time.zone.parse('2013-11-21')
+
+    current_semester_hours = Hour.semester '2016-12-01'
+    last_semester_hours = Hour.semester '2016-06-01'
+    all_semester_hours = Hour.semester
 
-    current_semester_hours = Hour.semester a_semester_ago.strftime(format)
-    last_semester_hours = Hour.semester last_semester_ago.strftime(format)
+    assert_includes current_semester_hours, this_hour1
+    assert_includes current_semester_hours, this_hour2
+    assert_not_includes current_semester_hours, prev_hour1
+    assert_not_includes current_semester_hours, prev_hour2
+    assert_not_includes current_semester_hours, other_hour
 
-    assert_includes current_semester_hours, hour3
-    assert_includes current_semester_hours, hour4
-    assert_not_includes current_semester_hours, hour1
-    assert_not_includes current_semester_hours, hour2
+    assert_includes last_semester_hours, prev_hour1
+    assert_includes last_semester_hours, prev_hour2
+    assert_not_includes last_semester_hours, this_hour1
+    assert_not_includes last_semester_hours, this_hour2
+    assert_not_includes last_semester_hours, other_hour
 
-    assert_includes last_semester_hours, hour1
-    assert_includes last_semester_hours, hour2
-    assert_not_includes last_semester_hours, hour3
-    assert_not_includes last_semester_hours, hour4
+    [prev_hour1, prev_hour2, this_hour1, this_hour2, other_hour].map do |hour|
+      assert_includes all_semester_hours, hour
+    end
   end
 end

test/models/volunteer_scopes_test.rb

@@ -331,25 +331,23 @@ class VolunteerScopesTest < ActiveSupport::TestCase
   end
 
   test 'with_billable_hours returns volunteers with billable hours for an optional semester' do
-    current_semester_ago = BillingExpense::SEMESTER_LENGTH.ago
-    last_semester_ago = current_semester_ago - BillingExpense::SEMESTER_LENGTH
-    format = '%Y-%m-%d'
+    travel_to Time.zone.parse('2017-07-01')
     volunteers_in_current_semester_assertion = [@group_offer_member, @has_assignments]
     volunteers_in_last_semester_assertion    = [@has_multiple, @has_active_and_inactive]
 
     volunteers_in_current_semester_assertion.each do |volunteer|
-      create :hour, hours: 1, volunteer: volunteer, meeting_date: current_semester_ago + 1.month
-      create :hour, hours: 2, volunteer: volunteer, meeting_date: current_semester_ago + 2.months
+      create :hour, hours: 1, volunteer: volunteer, meeting_date: Time.zone.parse('2017-01-01')
+      create :hour, hours: 2, volunteer: volunteer, meeting_date: Time.zone.parse('2017-02-02')
     end
 
     volunteers_in_last_semester_assertion.each do |volunteer|
-      create :hour, hours: 3, volunteer: volunteer, meeting_date: current_semester_ago - 1.month
-      create :hour, hours: 4, volunteer: volunteer, meeting_date: current_semester_ago - 2.months
+      create :hour, hours: 3, volunteer: volunteer, meeting_date: Time.zone.parse('2016-09-01')
+      create :hour, hours: 4, volunteer: volunteer, meeting_date: Time.zone.parse('2016-11-11')
     end
 
     volunteers_with_billable_hours = Volunteer.with_billable_hours
-    volunteers_in_current_semester = Volunteer.with_billable_hours current_semester_ago.strftime(format)
-    volunteers_in_last_semester = Volunteer.with_billable_hours last_semester_ago.strftime(format)
+    volunteers_in_current_semester = Volunteer.with_billable_hours '2016-12-01'
+    volunteers_in_last_semester = Volunteer.with_billable_hours '2016-06-01'
 
     (volunteers_in_current_semester + volunteers_in_last_semester).each do |volunteer|
       assert_includes volunteers_with_billable_hours, volunteer

test/test_helper.rb

@@ -65,4 +65,8 @@ class ActiveSupport::TestCase
       controllers_action_list.values
     end
   end
+
+  def tz_parse(time_string)
+    Time.zone.parse(time_string)
+  end
 end