Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schedule find_occurrence when interval is greater than 1 won't use the start date causing invalid results #92

Merged
merged 1 commit into from
Oct 11, 2012

Conversation

CoreyWinkelmannPP
Copy link
Contributor

I found when the find_occurrences function is called in Schedule that it wants to start from the opening_time when looping to find the occurrences between the begin_time and end_time when the interval is greater than 1. When the interval is > 1 I believe it should begin looping at the start_time to make sure when it gets to the opening_time and the closing_time that it knows that the occurrence falls in the range.

I added @interval in daily_interval.rb, weekly_interval.rb, monthly_interval.rb, and yearly_interval.rb to allow Rule to have access to it. Then I altered the full_required? method to check the interval and make sure it exists and if > 1 to return true.

# Whether this rule requires a full run
def full_required?
  !@count.nil? || (! @interval.nil? && @interval > 1)
end

@seejohnrun
Copy link
Collaborator

This is an exceptional pull request. Thanks so much, and apologies for the delay.
Is there a reason you kept @interval off of the hourly, minutely, secondly classes? seems it applies just as well (if not as noticably) there

seejohnrun added a commit that referenced this pull request Oct 11, 2012
Schedule find_occurrence when interval is greater than 1 won't use the start date causing invalid results
@seejohnrun seejohnrun merged commit b592cb6 into ice-cube-ruby:master Oct 11, 2012
@CoreyWinkelmannPP
Copy link
Contributor Author

No reason whatsoever. I was using the daily, weekly, and monthly intervals in the code I was writing and I didn't even look at the other intervals. Thanks for getting this in!

@seejohnrun
Copy link
Collaborator

Cool - I'll add it to the other classes as well and it will go out with 0.9.0
Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants