From ed039d4c4a6d28bde8e34e1609697f5a89b65ee9 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Mon, 31 Mar 2014 11:09:00 +0200 Subject: [PATCH] small refactorings and add some tests --- .../abstract_repeat_pattern.rb | 13 +++- .../recurring_todos/monthly_repeat_pattern.rb | 78 ++++++++++--------- .../recurring_todos/weekly_repeat_pattern.rb | 36 +++++---- .../recurring_todos/yearly_repeat_pattern.rb | 60 ++++++++------ .../abstract_repeat_pattern_test.rb | 28 ++++++- 5 files changed, 133 insertions(+), 82 deletions(-) diff --git a/app/models/recurring_todos/abstract_repeat_pattern.rb b/app/models/recurring_todos/abstract_repeat_pattern.rb index 9e6dcd3a..e678c9e4 100644 --- a/app/models/recurring_todos/abstract_repeat_pattern.rb +++ b/app/models/recurring_todos/abstract_repeat_pattern.rb @@ -164,8 +164,10 @@ module RecurringTodos private - # Determine start date to calculate next date for recurring todo - # offset needs to be 1.day for daily patterns + # Determine start date to calculate next date for recurring todo which + # takes start_from and previous into account. + # offset needs to be 1.day for daily patterns or the start will be the + # same day as the previous def determine_start(previous, offset=0.day) start = self.start_from || NullTime.new now = Time.zone.now @@ -179,9 +181,14 @@ module RecurringTodos end end + # Example: get 3rd (x) wednesday (weekday) of december (month) 2014 (year) + # 5th means last, so it will return the 4th if there is no 5th def get_xth_day_of_month(x, weekday, month, year) + raise "Weekday should be between 0 and 6 with 0=sunday. You supplied #{weekday}" unless (0..6).include?(weekday) + raise "x should be 1-4 for first-fourth or 5 for last. You supplied #{x}" unless (0..5).include?(x) + if x == 5 - # last -> count backwards. use UTC to avoid strange timezone oddities + # 5 means last -> 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)) while last_day.wday != weekday diff --git a/app/models/recurring_todos/monthly_repeat_pattern.rb b/app/models/recurring_todos/monthly_repeat_pattern.rb index 3fd6204f..04d1c8f6 100644 --- a/app/models/recurring_todos/monthly_repeat_pattern.rb +++ b/app/models/recurring_todos/monthly_repeat_pattern.rb @@ -66,54 +66,56 @@ module RecurringTodos def get_next_date(previous) start = determine_start(previous) - day = every_x_day n = get(:every_other2) case recurrence_selector when 0 # specific day of the month - if (previous && start.mday >= day) || (previous.nil? && start.mday > 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 = Time.zone.local(start.year, start.month, start.day) - - # go back to day - end - Time.zone.local(start.year, start.month, day) - + return find_specific_day_of_month(previous, start, n) when 1 # relative weekday of a month - the_next = get_xth_day_of_month(every_xth_day, day_of_week, start.month, start.year) - 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) - - # 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 - the_next = get_xth_day_of_month(every_xth_day, day_of_week, the_next.month, the_next.year) - end - the_next - else - raise Exception.new, "unknown monthly recurrence selection (#{self.recurrence_selector})" + return find_relative_day_of_month(start, n) end + nil end private + 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 + end + Time.zone.local(start.year, start.month, every_x_day) + end + + def find_relative_day_of_month(start, n) + the_next = get_xth_day_of_month(every_xth_day, day_of_week, start.month, start.year) + 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) + + # 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 + the_next = get_xth_day_of_month(every_xth_day, day_of_week, the_next.month, the_next.year) + end + the_next + end + def recurrence_pattern_for_specific_day on_day = " #{I18n.t('todos.recurrence.pattern.on_day_n', :n => every_x_day)}" if every_xth_day(0) > 1 diff --git a/app/models/recurring_todos/weekly_repeat_pattern.rb b/app/models/recurring_todos/weekly_repeat_pattern.rb index 8dead7fa..4e36ad43 100644 --- a/app/models/recurring_todos/weekly_repeat_pattern.rb +++ b/app/models/recurring_todos/weekly_repeat_pattern.rb @@ -36,22 +36,7 @@ module RecurringTodos end def get_next_date(previous) - # determine start - if previous.nil? - start = start_from.nil? ? Time.zone.now : self.start_from - else - start = previous + 1.day - if start.wday() == 0 - # we went to a new week , go to the nth next week and find first match - # that week. Note that we already went into the next week, so -1 - start += (every_x_week-1).week - end - unless self.start_from.nil? - # check if the start_from date is later than previous. If so, use - # start_from as start to search for next date - start = self.start_from if self.start_from > previous - end - end + start = determine_start_date(previous) day = find_first_day_in_this_week(start) return day unless day == -1 @@ -68,6 +53,25 @@ module RecurringTodos private + def determine_start_date(previous) + if previous.nil? + return self.start_from || Time.zone.now + else + start = previous + 1.day + if start.wday() == 0 + # we went to a new week, go to the nth next week and find first match + # that week. Note that we already went into the next week, so -1 + start += (every_x_week-1).week + end + unless self.start_from.nil? + # check if the start_from date is later than previous. If so, use + # start_from as start to search for next date + start = self.start_from if self.start_from > previous + end + return start + end + end + def find_first_day_in_this_week(start) # check if there are any days left this week for the next todo start.wday().upto 6 do |i| diff --git a/app/models/recurring_todos/yearly_repeat_pattern.rb b/app/models/recurring_todos/yearly_repeat_pattern.rb index bc1c1772..041909c3 100644 --- a/app/models/recurring_todos/yearly_repeat_pattern.rb +++ b/app/models/recurring_todos/yearly_repeat_pattern.rb @@ -34,8 +34,7 @@ module RecurringTodos def recurrence_pattern if self.recurrence_selector == 0 - I18n.t("todos.recurrence.pattern.every_year_on", - :date => I18n.l(DateTime.new(Time.zone.now.year, month_of_year, every_x_day), :format => :month_day)) + I18n.t("todos.recurrence.pattern.every_year_on", :date => date_as_month_day) else I18n.t("todos.recurrence.pattern.every_year_on", :date => I18n.t("todos.recurrence.pattern.the_xth_day_of_month", @@ -63,34 +62,47 @@ module RecurringTodos def get_next_date(previous) start = determine_start(previous) - day = every_x_day month = get(:every_other2) case recurrence_selector when 0 # specific day of a specific month - if start.month > month || (start.month == month && start.day >= day) - # if there is no next month n and day m in this year, search in next - # year - start = Time.zone.local(start.year+1, month, 1) - else - # if there is a next month n, stay in this year - start = Time.zone.local(start.year, month, 1) - end - Time.zone.local(start.year, month, day) - + return get_specific_day_of_month(start, month) when 1 # relative weekday of a specific month - # if there is no next month n in this year, search in next year - the_next = start.month > month ? Time.zone.local(start.year+1, month, 1) : start - - # get the xth day of the month - the_next = get_xth_day_of_month(self.every_xth_day, day_of_week, month, the_next.year) - - # if the_next is before previous, we went back into the past, so try next - # year - the_next = get_xth_day_of_month(self.every_xth_day, day_of_week, month, start.year+1) if the_next <= start - - the_next + return get_relative_weekday_of_month(start, month) end + nil + end + + private + + def date_as_month_day + I18n.l(DateTime.new(Time.zone.now.year, month_of_year, every_x_day), :format => :month_day) + end + + def get_specific_day_of_month(start, month) + if start.month > month || (start.month == month && start.day >= every_x_day) + # if there is no next month n and day m in this year, search in next + # year + start = Time.zone.local(start.year+1, month, 1) + else + # if there is a next month n, stay in this year + start = Time.zone.local(start.year, month, 1) + end + Time.zone.local(start.year, month, every_x_day) + end + + def get_relative_weekday_of_month(start, month) + # if there is no next month n in this year, search in next year + the_next = start.month > month ? Time.zone.local(start.year+1, month, 1) : start + + # get the xth day of the month + the_next = get_xth_day_of_month(self.every_xth_day, day_of_week, month, the_next.year) + + # if the_next is before previous, we went back into the past, so try next + # year + the_next = get_xth_day_of_month(self.every_xth_day, day_of_week, month, start.year+1) if the_next <= start + + the_next end end diff --git a/test/models/recurring_todos/abstract_repeat_pattern_test.rb b/test/models/recurring_todos/abstract_repeat_pattern_test.rb index fbf8a78a..998eb83c 100644 --- a/test/models/recurring_todos/abstract_repeat_pattern_test.rb +++ b/test/models/recurring_todos/abstract_repeat_pattern_test.rb @@ -100,6 +100,32 @@ module RecurringTodos assert !rt.continues_recurring?(Time.zone.now), "should end since all occurences are there" end + def test_determine_start + Timecop.travel(2013,1,1) do + rt = create_recurring_todo + assert_equal "2013-01-01 00:00:00 UTC", rt.send(:determine_start, nil).to_s, "no previous date, use today" + assert_equal "2013-01-01 00:00:00 UTC", rt.send(:determine_start, nil, 1.day).to_s, "no previous date, use today without offset" + assert_equal "2013-01-02 00:00:00 UTC", rt.send(:determine_start, Time.zone.now, 1.day).to_s, "use previous date and offset" + end + end + + def test_xth_day_of_month + rt = create_recurring_todo + + # march 2014 has 5 saturdays, the last will return the 5th + assert_equal "2014-03-01 00:00:00 UTC", rt.send(:get_xth_day_of_month, 1, 6, 3, 2014).to_s + assert_equal "2014-03-22 00:00:00 UTC", rt.send(:get_xth_day_of_month, 4, 6, 3, 2014).to_s + assert_equal "2014-03-29 00:00:00 UTC", rt.send(:get_xth_day_of_month, 5, 6, 3, 2014).to_s + + # march 2014 has 4 fridays, the last will return the 4th + assert_equal "2014-03-07 00:00:00 UTC", rt.send(:get_xth_day_of_month, 1, 5, 3, 2014).to_s + assert_equal "2014-03-28 00:00:00 UTC", rt.send(:get_xth_day_of_month, 4, 5, 3, 2014).to_s + assert_equal "2014-03-28 00:00:00 UTC", rt.send(:get_xth_day_of_month, 5, 5, 3, 2014).to_s + + assert_raise(RuntimeError, "should check on valid weekdays"){ rt.send(:get_xth_day_of_month, 5, 9, 3, 2014) } + assert_raise(RuntimeError, "should check on valid count x"){ rt.send(:get_xth_day_of_month, 6, 5, 3, 2014) } + end + private def create_pattern(attributes) @@ -108,7 +134,7 @@ module RecurringTodos builder.pattern end - def create_recurring_todo(attributes) + def create_recurring_todo(attributes={}) create_pattern(attributes.reverse_merge({ 'recurring_period' => 'weekly', 'recurring_target' => 'due_date',