From e58379e81f8fbc441a7d8f46acb4d4890d9bfb38 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Tue, 4 Aug 2015 23:08:13 +0200 Subject: [PATCH] This fixes failing tests when the timezone is different than utc There were several problems: * Time.now returns the systems time, not the users time * fixtures do not translate dates from timezone to utc, but stores the date verbatim * calling a controller will set the timezone to the preference of the current_user. So it could be changed while you do not realize this. I fixed the failing test, but problems could be elsewhere --- app/controllers/todos_controller.rb | 6 +- app/helpers/stats_helper.rb | 4 +- .../abstract_recurrence_pattern.rb | 24 +++---- .../monthly_recurrence_pattern.rb | 23 +----- app/models/stats/totals.rb | 2 +- app/views/layouts/application.html.erb | 2 +- app/views/layouts/mobile.m.erb | 2 +- app/views/preferences/_date_and_time.html.erb | 9 ++- config/application.rb | 4 +- config/environments/test.rb | 2 - lib/todo_from_recurring_todo.rb | 2 +- .../recurring_todos_controller_test.rb | 14 ++-- test/controllers/stats_controller_test.rb | 71 ++++++++++--------- test/controllers/todos_controller_test.rb | 31 ++++---- test/fixtures/contexts.yml | 9 ++- test/fixtures/notes.yml | 23 +++--- test/fixtures/projects.yml | 7 +- test/fixtures/recurring_todos.yml | 15 ++-- test/fixtures/tags.yml | 24 +++++-- test/fixtures/todos.yml | 26 +++---- test/models/project_test.rb | 3 +- test/models/recurring_todo_test.rb | 14 ++-- .../abstract_recurrence_pattern_test.rb | 22 +++--- test/models/todo_create_params_helper_test.rb | 5 +- test/models/todo_test.rb | 36 +++++----- test/models/user_test.rb | 22 +++--- test/test_helper.rb | 33 +++++---- 27 files changed, 221 insertions(+), 214 deletions(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 515450f6..497056fe 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -1071,10 +1071,10 @@ end if recurring_todo.todos.active.count == 0 # check for next todo either from the due date or the show_from date - date_to_check = todo.due.nil? ? todo.show_from : todo.due + date_to_check = todo.due || todo.show_from # if both due and show_from are nil, check for a next todo from now - date_to_check = Time.zone.now if date_to_check.nil? + date_to_check ||= Time.zone.now if recurring_todo.active? && recurring_todo.continues_recurring?(date_to_check) @@ -1087,7 +1087,7 @@ end # for tomorrow. date = date_to_check.at_midnight >= Time.zone.now.at_midnight ? date_to_check : Time.zone.now-1.day - new_recurring_todo = TodoFromRecurringTodo.new(current_user, recurring_todo).create(date.at_midnight) + new_recurring_todo = TodoFromRecurringTodo.new(current_user, recurring_todo).create(date) end end end diff --git a/app/helpers/stats_helper.rb b/app/helpers/stats_helper.rb index 22bbef02..94e93ae2 100644 --- a/app/helpers/stats_helper.rb +++ b/app/helpers/stats_helper.rb @@ -5,7 +5,7 @@ module StatsHelper end def month_and_year_label(i) - t('date.month_names')[ (Time.now.mon - i -1 ) % 12 + 1 ]+ " " + (Time.now - i.months).year.to_s + t('date.month_names')[ (Time.zone.now.mon - i -1 ) % 12 + 1 ]+ " " + (Time.zone.now - i.months).year.to_s end def array_of_month_and_year_labels(count) @@ -13,7 +13,7 @@ module StatsHelper end def month_label(i) - t('date.month_names')[ (Time.now.mon - i -1 ) % 12 + 1 ] + t('date.month_names')[ (Time.zone.now.mon - i -1 ) % 12 + 1 ] end def array_of_month_labels(count) diff --git a/app/models/recurring_todos/abstract_recurrence_pattern.rb b/app/models/recurring_todos/abstract_recurrence_pattern.rb index 90045347..fe07a0f9 100644 --- a/app/models/recurring_todos/abstract_recurrence_pattern.rb +++ b/app/models/recurring_todos/abstract_recurrence_pattern.rb @@ -151,7 +151,7 @@ module RecurringTodos end def get_next_date(previous) - raise "Should not call AbstractRecurrencePattern.get_next_date directly. Overwrite in subclass" + raise "Should not call AbstractRecurrencePattern.get_next_date directly. Override in subclass" end def continues_recurring?(previous) @@ -174,13 +174,13 @@ module RecurringTodos # same day as the previous def determine_start(previous, offset=0.day) start = self.start_from || NullTime.new - now = Time.zone.now if previous # check if the start_from date is later than previous. If so, use # start_from as start to search for next date start > previous ? start : previous + offset else # skip to present + now = Time.zone.now start > now ? start : now end end @@ -199,32 +199,24 @@ module RecurringTodos end def find_last_day_x_of_month(weekday, month, year) - # count backwards. use UTC to avoid strange timezone oddities - # where last_day -= 1.day seems to shift tz+0100 to tz+0000 - last_day = Time.utc(year, month, Time.days_in_month(month)) + last_day = Time.zone.local(year, month, Time.days_in_month(month)) while last_day.wday != weekday last_day -= 1.day end - # convert back to local timezone - Time.zone.local(last_day.year, last_day.month, last_day.day) + last_day end def find_xth_day_of_month(x, weekday, month, year) - # 1-4th -> count upwards last -> count backwards. use UTC to avoid strange - # timezone oddities where last_day -= 1.day seems to shift tz+0100 to - # tz+0000 - start = Time.utc(year,month,1) + start = Time.zone.local(year,month,1) n = x while n > 0 while start.wday() != weekday - start+= 1.day + start += 1.day end n -= 1 - start+= 1.day unless n==0 + start += 1.day unless n==0 end - # convert back to local timezone - Time.zone.local(start.year, start.month, start.day) + start end - end end diff --git a/app/models/recurring_todos/monthly_recurrence_pattern.rb b/app/models/recurring_todos/monthly_recurrence_pattern.rb index 7bb73612..38d6f7ae 100644 --- a/app/models/recurring_todos/monthly_recurrence_pattern.rb +++ b/app/models/recurring_todos/monthly_recurrence_pattern.rb @@ -82,18 +82,9 @@ module RecurringTodos def find_specific_day_of_month(previous, start, n) if (previous && start.mday >= every_x_day) || (previous.nil? && start.mday > every_x_day) # there is no next day n in this month, search in next month - # - # start += n.months - # - # The above seems to not work. Fiddle with timezone. Looks like we hit a - # bug in rails here where 2008-12-01 +0100 plus 1.month becomes - # 2008-12-31 +0100. For now, just calculate in UTC and convert back to - # local timezone. - # - # TODO: recheck if future rails versions have this problem too - start = Time.utc(start.year, start.month, start.day)+n.months + start += n.months end - Time.zone.local(start.year, start.month, every_x_day) + start.in_time_zone.change(day: every_x_day) end def find_relative_day_of_month(start, n) @@ -101,13 +92,7 @@ module RecurringTodos if the_next.nil? || the_next <= start # the nth day is already passed in this month, go to next month and try # again - - # fiddle with timezone. Looks like we hit a bug in rails here where - # 2008-12-01 +0100 plus 1.month becomes 2008-12-31 +0100. For now, just - # calculate in UTC and convert back to local timezone. - # TODO: recheck if future rails versions have this problem too - the_next = Time.utc(the_next.year, the_next.month, the_next.day)+n.months - the_next = Time.zone.local(the_next.year, the_next.month, the_next.day) + the_next += n.months # TODO: if there is still no match, start will be set to nil. if we ever # support 5th day of the month, we need to handle this case @@ -136,7 +121,5 @@ module RecurringTodos day: day_of_week_as_text(day_of_week), n_months: n_months) end - end - end diff --git a/app/models/stats/totals.rb b/app/models/stats/totals.rb index a80f8e71..49104678 100644 --- a/app/models/stats/totals.rb +++ b/app/models/stats/totals.rb @@ -19,7 +19,7 @@ module Stats end def first_action_at - first_action.created_at if first_action + first_action.try(:created_at) end def projects diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 1d2101d5..1a859330 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -40,7 +40,7 @@ <% if @count -%> <%= @count %> <% end -%> - <%= l(Date.today, :format => current_user.prefs.title_date_format) %> + <%= l(Time.zone.today, :format => current_user.prefs.title_date_format) %>