Fixes issue #2497 - Missing business hours validation for calendars creates infinitive loop (also for background jobs).

This commit is contained in:
Martin Edenhofer 2019-03-07 10:00:56 +01:00 committed by Thorsten Eckel
parent 2367e1e31c
commit 160b521116
6 changed files with 142 additions and 33 deletions

View file

@ -105,7 +105,7 @@ GEM
execjs execjs
binding_of_caller (0.8.0) binding_of_caller (0.8.0)
debug_inspector (>= 0.0.1) debug_inspector (>= 0.0.1)
biz (1.7.0) biz (1.8.2)
clavius (~> 1.0) clavius (~> 1.0)
tzinfo tzinfo
browser (2.5.2) browser (2.5.2)
@ -121,7 +121,7 @@ GEM
xpath (>= 2.0, < 4.0) xpath (>= 2.0, < 4.0)
childprocess (0.9.0) childprocess (0.9.0)
ffi (~> 1.0, >= 1.0.11) ffi (~> 1.0, >= 1.0.11)
clavius (1.0.3) clavius (1.0.4)
clearbit (0.2.8) clearbit (0.2.8)
nestful (~> 1.1.0) nestful (~> 1.1.0)
coderay (1.1.2) coderay (1.1.2)

View file

@ -53,10 +53,10 @@ class App.ControllerForm extends App.Controller
@form @form
showAlert: (message) => showAlert: (message) =>
@form.find('.alert').first().removeClass('hide').html(App.i18n.translateInline(message)) @form.find('.alert--danger').first().removeClass('hide').html(App.i18n.translateInline(message))
hideAlert: => hideAlert: =>
@form.find('.alert').addClass('hide').html() @form.find('.alert--danger').addClass('hide').html()
html: => html: =>
@form.html() @form.html()

View file

@ -7,8 +7,8 @@ class Calendar < ApplicationModel
store :business_hours store :business_hours
store :public_holidays store :public_holidays
before_create :validate_public_holidays, :fetch_ical before_create :validate_public_holidays, :validate_hours, :fetch_ical
before_update :validate_public_holidays, :fetch_ical before_update :validate_public_holidays, :validate_hours, :fetch_ical
after_create :sync_default, :min_one_check after_create :sync_default, :min_one_check
after_update :sync_default, :min_one_check after_update :sync_default, :min_one_check
after_destroy :min_one_check after_destroy :min_one_check
@ -265,6 +265,67 @@ returns
[day, comment] [day, comment]
end end
=begin
calendar = Calendar.find(123)
calendar.business_hours_to_hash
returns
{
mon: {'09:00' => '18:00'},
tue: {'09:00' => '18:00'},
wed: {'09:00' => '18:00'},
thu: {'09:00' => '18:00'},
sat: {'09:00' => '18:00'}
}
=end
def business_hours_to_hash
hours = {}
business_hours.each do |day, meta|
next if !meta[:active]
next if !meta[:timeframes]
hours[day.to_sym] = {}
meta[:timeframes].each do |frame|
next if !frame[0]
next if !frame[1]
hours[day.to_sym][frame[0]] = frame[1]
end
end
hours
end
=begin
calendar = Calendar.find(123)
calendar.public_holidays_to_array
returns
[
Thu, 08 Mar 2020,
Sun, 25 Mar 2020,
Thu, 29 Mar 2020,
]
=end
def public_holidays_to_array
holidays = []
public_holidays&.each do |day, meta|
next if !meta
next if !meta['active']
next if meta['removed']
holidays.push Date.parse(day)
end
holidays
end
private private
# if changed calendar is default, set all others default to false # if changed calendar is default, set all others default to false
@ -330,4 +391,25 @@ returns
end end
true true
end end
# validate business hours
def validate_hours
# get business hours
hours = business_hours_to_hash
raise Exceptions::UnprocessableEntity, 'No configured business hours found!' if hours.blank?
# validate if business hours are usable by execute a try calculation
begin
Biz.configure do |config|
config.hours = hours
end
Biz.time(10, :minutes).after(Time.zone.parse('Tue, 05 Feb 2019 21:40:28 UTC +00:00'))
rescue => e
raise Exceptions::UnprocessableEntity, e.message
end
true
end
end end

View file

@ -157,34 +157,13 @@ returns
biz = Biz::Schedule.new do |config| biz = Biz::Schedule.new do |config|
# get business hours # get business hours
hours = {} hours = calendar.business_hours_to_hash
calendar.business_hours.each do |day, meta| raise "No configured hours found in calendar #{calendar.inspect}" if hours.blank?
next if !meta[:active]
next if !meta[:timeframes]
hours[day.to_sym] = {}
meta[:timeframes].each do |frame|
next if !frame[0]
next if !frame[1]
hours[day.to_sym][frame[0]] = frame[1]
end
end
config.hours = hours config.hours = hours
if hours.blank?
raise "No configured hours found in calendar #{calendar.inspect}"
end
# get holidays # get holidays
holidays = [] config.holidays = calendar.public_holidays_to_array
calendar.public_holidays&.each do |day, meta|
next if !meta
next if !meta['active']
next if meta['removed']
holidays.push Date.parse(day)
end
config.holidays = holidays
config.time_zone = calendar.timezone config.time_zone = calendar.timezone
end end

View file

@ -143,4 +143,52 @@ RSpec.describe Calendar, type: :model do
end end
end end
end end
describe '#validate_hours' do
context 'when business_hours are invalid' do
it 'fails for hours ending at 00:00' do
expect do
create(:calendar,
business_hours: {
mon: {
active: true,
timeframes: [['09:00', '00:00']]
},
tue: {
active: true,
timeframes: [['09:00', '00:00']]
},
wed: {
active: true,
timeframes: [['09:00', '00:00']]
},
thu: {
active: true,
timeframes: [['09:00', '00:00']]
},
fri: {
active: true,
timeframes: [['09:00', '00:00']]
},
sat: {
active: false,
timeframes: [['09:00', '00:00']]
},
sun: {
active: false,
timeframes: [['09:00', '00:00']]
}
})
end.to raise_error(Exceptions::UnprocessableEntity, 'nonsensical hours provided')
end
it 'fails for blank structure' do
expect do
create(:calendar,
business_hours: {})
end.to raise_error(Exceptions::UnprocessableEntity, 'No configured business hours found!')
end
end
end
end end

View file

@ -1017,8 +1017,8 @@ class TicketSlaTest < ActiveSupport::TestCase
) )
ticket2.escalation_calculation(true) ticket2.escalation_calculation(true)
ticket2 = Ticket.find(ticket2.id) ticket2 = Ticket.find(ticket2.id)
assert_equal(ticket2.escalation_at.gmtime.to_s, '2013-06-04 16:00:00 UTC', 'ticket2.escalation_at verify 1') assert_equal(ticket2.escalation_at.gmtime.to_s, '2013-06-05 07:00:00 UTC', 'ticket2.escalation_at verify 1')
assert_equal(ticket2.first_response_escalation_at.gmtime.to_s, '2013-06-04 16:00:00 UTC', 'ticket2.first_response_escalation_at verify 1') assert_equal(ticket2.first_response_escalation_at.gmtime.to_s, '2013-06-05 07:00:00 UTC', 'ticket2.first_response_escalation_at verify 1')
assert_nil(ticket2.first_response_in_min, 'ticket2.first_response_in_min verify 3') assert_nil(ticket2.first_response_in_min, 'ticket2.first_response_in_min verify 3')
assert_nil(ticket2.first_response_diff_in_min, 'ticket2.first_response_diff_in_min verify 3') assert_nil(ticket2.first_response_diff_in_min, 'ticket2.first_response_diff_in_min verify 3')
@ -1259,7 +1259,7 @@ class TicketSlaTest < ActiveSupport::TestCase
assert_nil(ticket.first_response_in_min, 'ticket.first_response_in_min verify 3') assert_nil(ticket.first_response_in_min, 'ticket.first_response_in_min verify 3')
assert_nil(ticket.first_response_diff_in_min, 'ticket.first_response_diff_in_min verify 3') assert_nil(ticket.first_response_diff_in_min, 'ticket.first_response_diff_in_min verify 3')
assert_equal(ticket.update_escalation_at.gmtime.to_s, '2013-06-04 15:00:00 UTC', 'ticket.update_escalation_at verify 1') assert_equal(ticket.update_escalation_at.gmtime.to_s, '2013-06-04 15:00:00 UTC', 'ticket.update_escalation_at verify 1')
assert_equal(ticket.close_escalation_at.gmtime.to_s, '2013-06-04 16:00:00 UTC', 'ticket.close_escalation_at verify 1') assert_equal(ticket.close_escalation_at.gmtime.to_s, '2013-06-05 07:00:00 UTC', 'ticket.close_escalation_at verify 1')
assert_equal(ticket.close_in_min, 0, 'ticket.close_in_min verify 3') assert_equal(ticket.close_in_min, 0, 'ticket.close_in_min verify 3')
assert_equal(ticket.close_diff_in_min, 240, 'ticket.close_diff_in_min# verify 3') assert_equal(ticket.close_diff_in_min, 240, 'ticket.close_diff_in_min# verify 3')