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.

Sign up for GitHub

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

Jump to bottom

in_hours? doesn't consider breaks #69

Closed
aserafin opened this issue Jun 8, 2016 · 1 comment
Closed

in_hours? doesn't consider breaks #69

aserafin opened this issue Jun 8, 2016 · 1 comment

Comments

@aserafin
Copy link

aserafin commented Jun 8, 2016 •
edited
Loading

Hi,

It looks like in_hours? implementation doesn't consider breaks. I think this change in the in the lib/biz/calculation/active.rb will fix it:

def result
  schedule.intervals.any? { |interval| interval.contains?(time) } &&
    schedule.breaks.none? { |b| b.contains?(time) } &&
    schedule.holidays.none? { |holiday| holiday.contains?(time) }
end

but before creating pull-request I want to make sure that this isn't somehow intentional?
@aserafin aserafin changed the title in_hours? doesn't consider breaks in_hours? doesn't consider breaks Jun 8, 2016
craiglittle added a commit that referenced this issue Jun 9, 2016
@craiglittle
Consider breaks in in_hours? calculation
382e51b 
Since the calculation behind `in_hours?` is optimized to avoid periods
generation, we need to take extra steps to ensure it takes breaks into
consideration.

Closes #69.
craiglittle added a commit that referenced this issue Jun 9, 2016
@craiglittle
Consider breaks in in_hours? calculation
40cd22d 
Since the calculation behind `in_hours?` is optimized to avoid period
generation, we need to take extra steps to ensure it takes breaks into
consideration.

Closes #69.
craiglittle added a commit that referenced this issue Jun 9, 2016
@craiglittle
Consider breaks in in_hours? calculation
483c5ac 
Since the calculation behind `in_hours?` is optimized to avoid period
generation, we need to take extra steps to ensure it takes breaks into
consideration.

Closes #69.
craiglittle added a commit that referenced this issue Jun 9, 2016
@craiglittle
Consider breaks in in_hours? calculation
751c3f5 
Since the calculation behind `in_hours?` is optimized to avoid period
generation, we need to take extra steps to ensure it takes breaks into
consideration.

Closes #69.
craiglittle added a commit that referenced this issue Jun 9, 2016
@craiglittle
Consider breaks in in_hours? calculation
1bbbf60 
Since the calculation behind `in_hours?` is optimized to avoid period
generation, we need to take extra steps to ensure it takes breaks into
consideration.

Closes #69.
@craiglittle craiglittle mentioned this issue Jun 9, 2016
Merged
@craiglittle
Copy link
Collaborator

Good catch! No worries, I've opened a PR: #70.

craiglittle added a commit that referenced this issue Jun 9, 2016
@craiglittle
Consider breaks in in_hours? calculation
a31f6dd 
Since the calculation behind `in_hours?` is optimized to avoid period
generation, we need to take extra steps to ensure it takes breaks into
consideration.

Closes #69.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@craiglittle @aserafin