-
Notifications
You must be signed in to change notification settings - Fork 360
Prevent illogical rule combinations #420
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
Changes from all commits
7f879fe
c6ef94c
70d2e59
4ba3297
d698e84
7f7d5a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| module IceCube | ||
| class InputAlignment | ||
|
|
||
| def initialize(rule, value, rule_part) | ||
| @rule = rule | ||
| @value = value | ||
| @rule_part = rule_part | ||
| end | ||
|
|
||
| attr_reader :rule, :value, :rule_part | ||
|
|
||
| def verify(freq, options={}, &block) | ||
| @rule.validations[:interval] or return | ||
|
|
||
| case @rule | ||
| when DailyRule | ||
| verify_wday_alignment(freq, &block) | ||
| when MonthlyRule | ||
| verify_month_alignment(freq, &block) | ||
| else | ||
| verify_freq_alignment(freq, &block) | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def interval_validation | ||
| @interval_validation ||= @rule.validations[:interval].first | ||
| end | ||
|
|
||
| def interval_value | ||
| @interval_value ||= (rule_part == :interval) ? value : interval_validation.interval | ||
| end | ||
|
|
||
| def fixed_validations | ||
| @fixed_validations ||= @rule.validations.values.flatten.select { |v| | ||
| interval_type = (v.type == :wday ? :day : v.type) | ||
| v.class < Validations::FixedValue && | ||
| interval_type == rule.base_interval_validation.type | ||
| } | ||
| end | ||
|
|
||
| def verify_freq_alignment(freq) | ||
| interval_validation.type == freq or return | ||
| (last_validation = fixed_validations.min_by(&:value)) or return | ||
|
|
||
| alignment = (value - last_validation.value) % interval_validation.interval | ||
| return if alignment.zero? | ||
|
|
||
| validation_values = fixed_validations.map(&:value).join(', ') | ||
| if rule_part == :interval | ||
| message = "interval(#{value}) " \ | ||
| "must be a multiple of " \ | ||
| "intervals in #{last_validation.key}(#{validation_values})" | ||
| else | ||
| message = "intervals in #{last_validation.key}(#{validation_values}, #{value}) " \ | ||
| "must be multiples of " \ | ||
| "interval(#{interval_validation.interval})" | ||
| end | ||
|
|
||
| yield ArgumentError.new(message) | ||
| end | ||
|
|
||
| def verify_month_alignment(_freq) | ||
| return if interval_value == 1 || (interval_value % 12).zero? | ||
| return if fixed_validations.empty? | ||
|
|
||
| message = "month_of_year can only be used with interval(1) or multiples of interval(12)" | ||
|
|
||
| yield ArgumentError.new(message) | ||
| end | ||
|
|
||
| def verify_wday_alignment(freq) | ||
| return if interval_value == 1 | ||
|
|
||
| if freq == :wday | ||
| return if (interval_value % 7).zero? | ||
| return if Array(@rule.validations[:day]).empty? | ||
| message = "day can only be used with multiples of interval(7)" | ||
| else | ||
| (fixed_validation = fixed_validations.first) or return | ||
| message = "#{fixed_validation.key} can only be used with interval(1)" | ||
| end | ||
|
|
||
| yield ArgumentError.new(message) | ||
| end | ||
|
|
||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,11 @@ module Validations::DailyInterval | |
|
|
||
| # Add a new interval validation | ||
| def interval(interval) | ||
| @interval = normalized_interval(interval) | ||
| interval = normalized_interval(interval) | ||
| verify_alignment(interval, :wday, :interval) { |error| raise error } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we yield this error up instead of just
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is so that it gives a more relevant trace, since it appears from the called |
||
| verify_alignment(interval, :day, :interval) { |error| raise error } | ||
|
|
||
| @interval = interval | ||
| replace_validations_for(:interval, [Validation.new(@interval)]) | ||
| clobber_base_validations(:wday, :day) | ||
| self | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to splitting these out so we can only have them where they make sense