Rails 5.1 migration.

Gemfile

@@ -2,7 +2,7 @@ source 'https://rubygems.org'
 
 ruby '2.4.1'
 
-gem 'rails', '5.0.5'
+gem 'rails', '5.1.4'
 gem 'rails-observers'
 gem 'activerecord-session_store'
 
@@ -53,7 +53,8 @@ gem 'mime-types'
 
 gem 'biz'
 
-gem 'composite_primary_keys'
+# temporary till pull request gets merged: https://github.com/composite-primary-keys/composite_primary_keys/pull/404
+gem 'composite_primary_keys', git: 'https://github.com/jkowens/composite_primary_keys.git', branch: 'rails-5_1'
 gem 'delayed_job_active_record'
 gem 'daemons'
 

Gemfile.lock

@@ -1,3 +1,11 @@
+GIT
+  remote: https://github.com/jkowens/composite_primary_keys.git
+  revision: 7f4670b54b3c6e94992161b4efe2c8717d7c0e71
+  branch: rails-5_1
+  specs:
+    composite_primary_keys (9.0.7)
+      activerecord (~> 5.1.0)
+
 GIT
   remote: https://github.com/thorsteneckel/autodiscover.git
   revision: 29d713ee0c8c25fcf74c4292ff13fe1fa4d0d827
@@ -17,38 +25,38 @@ GIT
 GEM
   remote: https://rubygems.org/
   specs:
-    actioncable (5.0.5)
+    actioncable (5.1.4)
-      actionpack (= 5.0.5)
+      actionpack (= 5.1.4)
-      nio4r (>= 1.2, < 3.0)
+      nio4r (~> 2.0)
       websocket-driver (~> 0.6.1)
-    actionmailer (5.0.5)
+    actionmailer (5.1.4)
-      actionpack (= 5.0.5)
+      actionpack (= 5.1.4)
-      actionview (= 5.0.5)
+      actionview (= 5.1.4)
-      activejob (= 5.0.5)
+      activejob (= 5.1.4)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (5.0.5)
+    actionpack (5.1.4)
-      actionview (= 5.0.5)
+      actionview (= 5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
       rack (~> 2.0)
-      rack-test (~> 0.6.3)
+      rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    actionview (5.0.5)
+    actionview (5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
       builder (~> 3.1)
-      erubis (~> 2.7.0)
+      erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.3)
-    activejob (5.0.5)
+    activejob (5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
       globalid (>= 0.3.6)
-    activemodel (5.0.5)
+    activemodel (5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
-    activerecord (5.0.5)
+    activerecord (5.1.4)
-      activemodel (= 5.0.5)
+      activemodel (= 5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
-      arel (~> 7.0)
+      arel (~> 8.0)
     activerecord-nulldb-adapter (0.3.7)
       activerecord (>= 2.0.0)
     activerecord-session_store (1.1.0)
@@ -57,14 +65,14 @@ GEM
       multi_json (~> 1.11, >= 1.11.2)
       rack (>= 1.5.2, < 3)
       railties (>= 4.0, < 5.2)
-    activesupport (5.0.5)
+    activesupport (5.1.4)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (~> 0.7)
       minitest (~> 5.1)
       tzinfo (~> 1.1)
     addressable (2.5.2)
       public_suffix (>= 2.0.2, < 4.0)
-    arel (7.1.4)
+    arel (8.0.0)
     argon2 (1.1.3)
       ffi (~> 1.9)
       ffi-compiler (~> 0.1)
@@ -94,8 +102,6 @@ GEM
       coffee-script
       execjs
       json
-    composite_primary_keys (9.0.7)
-      activerecord (~> 5.0.0)
     concurrent-ruby (1.0.5)
     coveralls (0.8.21)
       json (>= 1.8, < 3)
@@ -127,7 +133,7 @@ GEM
       eventmachine (>= 0.12.9)
       http_parser.rb (~> 0.6.0)
     equalizer (0.0.11)
-    erubis (2.7.0)
+    erubi (1.6.1)
     eventmachine (1.2.5)
     execjs (2.7.0)
     factory_girl (4.8.0)
@@ -219,7 +225,7 @@ GEM
       thread_safe (~> 0.3, >= 0.3.1)
     method_source (0.8.2)
     mime-types (2.99.3)
-    mini_portile2 (2.2.0)
+    mini_portile2 (2.3.0)
     minitest (5.10.3)
     multi_json (1.12.1)
     multi_xml (0.6.0)
@@ -231,8 +237,8 @@ GEM
     net-ldap (0.16.0)
     netrc (0.11.0)
     nio4r (2.1.0)
-    nokogiri (1.8.0)
+    nokogiri (1.8.1)
-      mini_portile2 (~> 2.2.0)
+      mini_portile2 (~> 2.3.0)
     nori (2.6.0)
     notiffany (0.1.1)
       nenv (~> 0.1)
@@ -294,19 +300,19 @@ GEM
     rack (2.0.3)
     rack-livereload (0.3.16)
       rack
-    rack-test (0.6.3)
+    rack-test (0.7.0)
-      rack (>= 1.0)
+      rack (>= 1.0, < 3)
-    rails (5.0.5)
+    rails (5.1.4)
-      actioncable (= 5.0.5)
+      actioncable (= 5.1.4)
-      actionmailer (= 5.0.5)
+      actionmailer (= 5.1.4)
-      actionpack (= 5.0.5)
+      actionpack (= 5.1.4)
-      actionview (= 5.0.5)
+      actionview (= 5.1.4)
-      activejob (= 5.0.5)
+      activejob (= 5.1.4)
-      activemodel (= 5.0.5)
+      activemodel (= 5.1.4)
-      activerecord (= 5.0.5)
+      activerecord (= 5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
       bundler (>= 1.3.0)
-      railties (= 5.0.5)
+      railties (= 5.1.4)
       sprockets-rails (>= 2.0.0)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
@@ -315,16 +321,16 @@ GEM
       loofah (~> 2.0)
     rails-observers (0.1.5)
       activemodel (>= 4.0)
-    railties (5.0.5)
+    railties (5.1.4)
-      actionpack (= 5.0.5)
+      actionpack (= 5.1.4)
-      activesupport (= 5.0.5)
+      activesupport (= 5.1.4)
       method_source
       rake (>= 0.8.7)
       thor (>= 0.18.1, < 2.0)
     rainbow (2.2.2)
       rake
     raindrops (0.19.0)
-    rake (12.0.0)
+    rake (12.1.0)
     rb-fsevent (0.10.2)
     rb-inotify (0.9.10)
       ffi (>= 0.5.0, < 2)
@@ -476,7 +482,7 @@ DEPENDENCIES
   coffee-rails
   coffee-script-source
   coffeelint
-  composite_primary_keys
+  composite_primary_keys!
   coveralls
   daemons
   delayed_job_active_record
@@ -516,7 +522,7 @@ DEPENDENCIES
   pre-commit
   puma
   rack-livereload
-  rails (= 5.0.5)
+  rails (= 5.1.4)
   rails-observers
   rb-fsevent
   rspec-rails

app/controllers/long_polling_controller.rb

@@ -1,7 +1,7 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class LongPollingController < ApplicationController
-  skip_action_callback :session_update # prevent race conditions
+  skip_before_action :session_update # prevent race conditions
 
   # GET /api/v1/message_send
   def message_send

app/models/application_model/can_lookup_search_index_attributes.rb

@@ -55,7 +55,7 @@ returns
     }
 
     ignored_attributes = self.class.instance_variable_get(:@search_index_attributes_ignored) || []
-    return attributes if ignored_attributes.empty?
+    return attributes if ignored_attributes.blank?
 
     ignored_attributes.each { |attribute|
       attributes.delete(attribute.to_s)

app/models/application_model/has_cache.rb

@@ -41,14 +41,14 @@ module ApplicationModel::HasCache
     }
 
     # delete old name / login caches
-    if changed?
+    if saved_changes?
-      if changes.key?('name')
+      if saved_changes.key?('name')
-        name = changes['name'][0]
+        name = saved_changes['name'][0]
         key = "#{self.class}::#{name}"
         Cache.delete(key)
       end
-      if changes.key?('login')
+      if saved_changes.key?('login')
-        name = changes['login'][0]
+        name = saved_changes['login'][0]
         key = "#{self.class}::#{name}"
         Cache.delete(key)
       end

app/models/channel/email_parser.rb

@@ -475,7 +475,7 @@ returns
     # check ignore header
     if mail['x-zammad-ignore'.to_sym] == 'true' || mail['x-zammad-ignore'.to_sym] == true
       Rails.logger.info "ignored email with msgid '#{mail[:message_id]}' from '#{mail[:from]}' because of x-zammad-ignore header"
-      return true
+      return
     end
 
     # set interface handle
@@ -514,7 +514,7 @@ returns
         set_attributes_by_x_headers(ticket, 'ticket', mail, 'followup')
 
         # save changes set by x-zammad-ticket-followup-* headers
-        ticket.save if ticket.changed?
+        ticket.save! if ticket.has_changes_to_save?
 
         state      = Ticket::State.find(ticket.state_id)
         state_type = Ticket::StateType.find(state.state_type_id)
@@ -650,7 +650,7 @@ returns
 
   def self.sender_properties(from)
     data = {}
-
+    return data if from.blank?
     begin
       list = Mail::AddressList.new(from)
       list.addresses.each { |address|

app/models/concerns/has_activity_stream_log.rb

@@ -32,13 +32,13 @@ log object update activity stream, if configured - will be executed automaticall
 =end
 
   def activity_stream_update
-    return true if !changed?
+    return true if !saved_changes?
 
     ignored_attributes  = self.class.instance_variable_get(:@activity_stream_attributes_ignored) || []
     ignored_attributes += %i(created_at updated_at created_by_id updated_by_id)
 
     log = false
-    changes.each { |key, _value|
+    saved_changes.each { |key, _value|
       next if ignored_attributes.include?(key.to_sym)
 
       log = true

app/models/concerns/has_history.rb

@@ -33,13 +33,13 @@ log object update history with all updated attributes, if configured - will be e
 =end
 
   def history_update
-    return if !changed?
+    return if !saved_changes?
 
     # return if it's no update
     return if new_record?
 
     # new record also triggers update, so ignore new records
-    changes = self.changes
+    changes = saved_changes
     if history_changes_last_done
       history_changes_last_done.each { |key, value|
         if changes.key?(key) && changes[key] == value

app/models/observer/sla/ticket_rebuild_escalation.rb

@@ -27,7 +27,7 @@ class Observer::Sla::TicketRebuildEscalation < ActiveRecord::Observer
   def _check(record)
 
     # return if we run import mode
-    return if Setting.get('import_mode') && !Setting.get('import_ignore_sla')
+    return true if Setting.get('import_mode') && !Setting.get('import_ignore_sla')
 
     # check if condition has changed
     changed = false
@@ -38,11 +38,11 @@ class Observer::Sla::TicketRebuildEscalation < ActiveRecord::Observer
                         %w(timezone business_hours default ical_url public_holidays)
                       end
     fields_to_check.each { |item|
-      next if !record.changes[item]
+      next if !record.saved_change_to_attribute(item)
-      next if record.changes[item][0] == record.changes[item][1]
+      next if record.saved_change_to_attribute(item)[0] == record.saved_change_to_attribute(item)[1]
       changed = true
     }
-    return if !changed
+    return true if !changed
 
     _rebuild(record)
   end

app/models/observer/ticket/last_owner_update.rb

@@ -20,7 +20,7 @@ class Observer::Ticket::LastOwnerUpdate < ActiveRecord::Observer
 
     # check if owner has changed
     if type == 'update'
-      return true if record.changes['owner_id'].blank?
+      return true if record.changes_to_save['owner_id'].blank?
     end
 
     # check if owner is nobody

app/models/observer/ticket/online_notification_seen.rb

@@ -19,8 +19,8 @@ class Observer::Ticket::OnlineNotificationSeen < ActiveRecord::Observer
     return false if Setting.get('import_mode')
 
     # set seen only if state has changes
-    return false if record.changes.blank?
+    return false if !record.saved_changes?
-    return false if record.changes['state_id'].blank?
+    return false if record.saved_changes['state_id'].blank?
 
     # check if existing online notifications for this ticket should be set to seen
     return true if !record.online_notification_seen_state

app/models/observer/ticket/ref_object_touch.rb

@@ -18,10 +18,10 @@ class Observer::Ticket::RefObjectTouch < ActiveRecord::Observer
   def ref_object_touch(record)
 
     # return if we run import mode
-    return if Setting.get('import_mode')
+    return true if Setting.get('import_mode')
 
     # touch old customer if changed
-    cutomer_id_changed = record.changes['customer_id']
+    cutomer_id_changed = record.saved_changes['customer_id']
     if cutomer_id_changed && cutomer_id_changed[0] != cutomer_id_changed[1]
       if cutomer_id_changed[0]
         User.find(cutomer_id_changed[0]).touch
@@ -34,7 +34,7 @@ class Observer::Ticket::RefObjectTouch < ActiveRecord::Observer
     end
 
     # touch old organization if changed
-    organization_id_changed = record.changes['organization_id']
+    organization_id_changed = record.saved_changes['organization_id']
     if organization_id_changed && organization_id_changed[0] != organization_id_changed[1]
       if organization_id_changed[0]
         Organization.find(organization_id_changed[0]).touch
@@ -42,7 +42,7 @@ class Observer::Ticket::RefObjectTouch < ActiveRecord::Observer
     end
 
     # touch new/current organization
-    return if !record.organization
+    return true if !record.organization
 
     record.organization.touch
   end

app/models/observer/ticket/stats_reopen.rb

@@ -19,6 +19,6 @@ class Observer::Ticket::StatsReopen < ActiveRecord::Observer
 
     # return if we run import mode
     return if Setting.get('import_mode')
-    Stats::TicketReopen.log('Ticket', record.id, record.changes, record.updated_by_id)
+    Stats::TicketReopen.log('Ticket', record.id, record.saved_changes, record.updated_by_id)
   end
 end

app/models/observer/transaction.rb

@@ -201,7 +201,7 @@ class Observer::Transaction < ActiveRecord::Observer
 
     # ignore certain attributes
     real_changes = {}
-    record.changes.each { |key, value|
+    record.changes_to_save.each { |key, value|
       next if key == 'updated_at'
       next if key == 'first_response_at'
       next if key == 'close_at'

app/models/observer/user/ref_object_touch.rb

@@ -22,7 +22,7 @@ class Observer::User::RefObjectTouch < ActiveRecord::Observer
 
     # touch old organization if changed
     member_ids = []
-    organization_id_changed = record.changes['organization_id']
+    organization_id_changed = record.saved_changes['organization_id']
     if organization_id_changed && organization_id_changed[0] != organization_id_changed[1]
       if organization_id_changed[0]
 

app/models/observer/user/ticket_organization.rb

@@ -15,7 +15,7 @@ class Observer::User::TicketOrganization < ActiveRecord::Observer
   def check_organization(record)
 
     # check if organization has changed
-    return if !record.changes['organization_id']
+    return true if !record.saved_change_to_attribute?('organization_id')
 
     # update last 100 tickets of user
     tickets = Ticket.where(customer_id: record.id).limit(100)

app/models/taskbar.rb

@@ -104,7 +104,7 @@ class Taskbar < ApplicationModel
   end
 
   def notify_clients
-    return true if !changes['preferences']
+    return true if !saved_change_to_attribute?('preferences')
     data = {
       event: 'taskbar:preferences',
       data: {

app/models/ticket.rb

@@ -1160,7 +1160,7 @@ result
   def reset_pending_time
 
     # ignore if no state has changed
-    return true if !changes['state_id']
+    return true if !changes_to_save['state_id']
 
     # ignore if new state is blank and
     # let handle ActiveRecord the error

app/models/ticket/escalation.rb

@@ -76,10 +76,10 @@ returns
 
     # if no escalation is enabled
     if !sla || !calendar
-      preferences[:escalation_calculation] = {}
 
       # nothing to change
       return false if !escalation_at && !first_response_escalation_at && !update_escalation_at && !close_escalation_at
+      preferences['escalation_calculation'] = {}
       self.escalation_at = nil
       self.first_response_escalation_at = nil
       self.escalation_at = nil
@@ -122,7 +122,7 @@ returns
       first_response_at_changed = false
     end
     last_update_at_changed = true
-    if escalation_calculation['last_update_at'] == last_update_at && !changes['state_id']
+    if escalation_calculation['last_update_at'] == last_update_at && !saved_change_to_attribute('state_id')
       last_update_at_changed = false
     end
     close_at_changed = true
@@ -352,7 +352,7 @@ returns
     ).map(&:name)
 
     # add state changes till now
-    if add_current && changes['state_id'] && changes['state_id'][0] && changes['state_id'][1]
+    if add_current && saved_change_to_attribute('state_id') && saved_change_to_attribute('state_id')[0] && saved_change_to_attribute('state_id')[1]
       last_history_state = nil
       history_list.each { |history_item|
         next if !history_item['attribute']
@@ -361,14 +361,14 @@ returns
         last_history_state = history_item
       }
       local_updated_at = updated_at
-      if changes['updated_at'] && changes['updated_at'][1]
+      if saved_change_to_attribute('updated_at') && saved_change_to_attribute('updated_at')[1]
-        local_updated_at = changes['updated_at'][1]
+        local_updated_at = saved_change_to_attribute('updated_at')[1]
       end
       history_item = {
         'attribute' => 'state',
         'created_at' => local_updated_at,
-        'value_from' => Ticket::State.find(changes['state_id'][0]).name,
+        'value_from' => Ticket::State.find(saved_change_to_attribute('state_id')[0]).name,
-        'value_to' => Ticket::State.find(changes['state_id'][1]).name,
+        'value_to' => Ticket::State.find(saved_change_to_attribute('state_id')[1]).name,
       }
       if last_history_state
         last_history_state = history_item

app/models/user.rb

@@ -1055,7 +1055,7 @@ raise 'Minimum one user need to have admin permissions'
   def avatar_for_email_check
     return true if email.blank?
     return true if email !~ /@/
-    return true if !changes['email'] && updated_at > Time.zone.now - 10.days
+    return true if !saved_change_to_attribute?('email') && updated_at > Time.zone.now - 10.days
 
     # save/update avatar
     avatar = Avatar.auto_detection(
@@ -1106,8 +1106,7 @@ raise 'Minimum one user need to have admin permissions'
 
   # reset login_failed if password is changed
   def reset_login_failed
-    return true if !changes
+    return true if !will_save_change_to_attribute?('password')
-    return true if !changes['password']
     self.login_failed = 0
     true
   end

lib/import/base_resource.rb

@@ -37,7 +37,7 @@ module Import
     def changed_attributes
       return if @resource.blank?
       # dry run
-      return @resource.changes if @resource.changed?
+      return @resource.changes_to_save if @resource.has_changes_to_save?
       # live run
       @resource.previous_changes
     end

lib/sequencer/unit/import/common/model/update.rb

@@ -39,9 +39,9 @@ class Sequencer
 
             def changes
               @changes ||= begin
-                if instance.changed?
+                if instance.has_changes_to_save?
                   # dry run
-                  instance.changes
+                  instance.changes_to_save
                 else
                   # live run
                   instance.previous_changes

spec/lib/external_sync_spec.rb

@@ -15,8 +15,8 @@ RSpec.describe ExternalSync do
         current_changes:  current_changes,
       )
 
-      expect(result).to          be false
+      expect(result).to                      be false
-      expect(object.changed?).to be false
+      expect(object.has_changes_to_save?).to be false
     end
 
     it 'keeps ActiveRecord instance unchanged on local and remote changes' do
@@ -30,8 +30,8 @@ RSpec.describe ExternalSync do
         current_changes:  current_changes,
       )
 
-      expect(result).to          be false
+      expect(result).to                      be false
-      expect(object.changed?).to be false
+      expect(object.has_changes_to_save?).to be false
     end
 
     it 'changes ActiveRecord instance attribute(s) for remote changes' do
@@ -45,8 +45,8 @@ RSpec.describe ExternalSync do
         current_changes:  current_changes,
       )
 
-      expect(result).to          be true
+      expect(result).to                      be true
-      expect(object.changed?).to be true
+      expect(object.has_changes_to_save?).to be true
     end
 
     it 'prevents ActiveRecord method calls' do
@@ -61,9 +61,9 @@ RSpec.describe ExternalSync do
         current_changes:  current_changes,
       )
 
-      expect(result).to          be false
+      expect(result).to                      be false
-      expect(object.changed?).to be false
+      expect(object.has_changes_to_save?).to be false
-      expect(object.destroyed?).to be false
+      expect(object.destroyed?).to           be false
     end
 
   end

test/integration/email_helper_test.rb

@@ -209,7 +209,7 @@ class EmailHelperTest < ActiveSupport::TestCase
       }
     )
     assert_equal('invalid', result[:result])
-    assert_equal('Authentication failed, username incorrect!', result[:message_human])
+    assert_match(/Authentication failed, username incorrect|Authentication failed, invalid credentials/, result[:message_human])
     assert_equal('imap.gmail.com', result[:settings][:options][:host])
 
     result = EmailHelper::Probe.inbound(
@@ -225,8 +225,8 @@ class EmailHelperTest < ActiveSupport::TestCase
     assert_equal('invalid', result[:result])
 
     # if we have to many failed logins, we need to handle another error message
-    if result[:message_human] && !result[:message_human].empty?
+    if result[:message_human].present?
-      assert_equal('Authentication failed, invalid credentials!', result[:message_human])
+      assert_match(/Authentication failed, username incorrect|Authentication failed, invalid credentials/, result[:message_human])
     else
       assert_match(/Web login required/, result[:message])
     end
@@ -457,7 +457,7 @@ class EmailHelperTest < ActiveSupport::TestCase
       email: mailbox_user,
       password: mailbox_password,
     )
-    assert_equal(nil, result[:reason])
+    assert_nil(result[:reason])
     assert_equal('ok', result[:result])
     assert_equal('pop.gmail.com', result[:setting][:inbound][:options][:host])
     assert_equal('smtp.gmail.com', result[:setting][:outbound][:options][:host])

test/integration/user_device_controller_test.rb

@@ -61,7 +61,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     assert_equal(0, email_notification_count('user_device_new_location', @admin.email))
 
     params = { without_fingerprint: 'none', username: 'user-device-admin', password: 'adminpw' }
-    post '/api/v1/signin', params.to_json, @headers
+    post '/api/v1/signin', params: params.to_json, headers: @headers
     assert_response(422)
     result = JSON.parse(@response.body)
 
@@ -85,7 +85,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     assert_equal(0, email_notification_count('user_device_new_location', @admin.email))
 
     params = { fingerprint: 'my_finger_print', username: 'user-device-admin', password: 'adminpw' }
-    post '/api/v1/signin', params.to_json, @headers
+    post '/api/v1/signin', params: params.to_json, headers: @headers
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Hash)
@@ -102,7 +102,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     sleep 2
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Array)
@@ -117,7 +117,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     assert_equal(user_device_last.updated_at.to_s, user_device_first.updated_at.to_s)
 
     params = { fingerprint: 'my_finger_print' }
-    get '/api/v1/signshow', params, @headers
+    get '/api/v1/signshow', params: params, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Hash)
@@ -135,7 +135,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
 
     ENV['USER_DEVICE_UPDATED_AT'] = (Time.zone.now - 4.hours).to_s
     params = {}
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Array)
@@ -153,7 +153,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     ENV['TEST_REMOTE_IP'] = '195.65.29.254' # ch
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -171,7 +171,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
   test '04 - login index with admin with fingerprint - II' do
 
     params = { fingerprint: 'my_finger_print_II', username: 'user-device-admin', password: 'adminpw' }
-    post '/api/v1/signin', params.to_json, @headers
+    post '/api/v1/signin', params: params.to_json, headers: @headers
     assert_response(201)
     result = JSON.parse(@response.body)
 
@@ -185,7 +185,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     assert(result['config'])
     assert('my_finger_print_II', controller.session[:user_device_fingerprint])
 
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Array)
@@ -197,7 +197,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     assert_equal(0, email_notification_count('user_device_new_location', @admin.email))
 
     params = { fingerprint: 'my_finger_print_II' }
-    get '/api/v1/signshow', params, @headers
+    get '/api/v1/signshow', params: params, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Hash)
@@ -214,7 +214,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     ENV['TEST_REMOTE_IP'] = '195.65.29.254' # ch
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -232,7 +232,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
   test '05 - login index with admin with fingerprint - II' do
 
     params = { fingerprint: 'my_finger_print_II', username: 'user-device-admin', password: 'adminpw' }
-    post '/api/v1/signin', params.to_json, @headers
+    post '/api/v1/signin', params: params.to_json, headers: @headers
     assert_response(201)
     result = JSON.parse(@response.body)
 
@@ -253,7 +253,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-admin', 'adminpw')
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
+    get '/api/v1/users', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -267,7 +267,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     sleep 2
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
+    get '/api/v1/users', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -285,7 +285,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     user_device_last.save!
 
     params = {}
-    get '/api/v1/users', params, @headers.merge('Authorization' => credentials)
+    get '/api/v1/users', params: params, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -307,7 +307,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-admin', 'adminpw')
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
+    get '/api/v1/users', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -330,7 +330,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-agent', 'agentpw')
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
+    get '/api/v1/users', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -353,7 +353,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-agent', 'agentpw')
 
     params = {}
-    get '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
+    get '/api/v1/users', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
 
@@ -375,7 +375,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
     ENV['SWITCHED_FROM_USER_ID'] = @admin.id.to_s
 
     params = { fingerprint: 'my_finger_print_II', username: 'user-device-agent', password: 'agentpw' }
-    post '/api/v1/signin', params.to_json, @headers
+    post '/api/v1/signin', params: params.to_json, headers: @headers
     assert_response(201)
     result = JSON.parse(@response.body)
 
@@ -397,7 +397,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
 
     ENV['USER_DEVICE_UPDATED_AT'] = (Time.zone.now - 4.hours).to_s
     params = {}
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Array)
@@ -412,7 +412,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest
 
     ENV['TEST_REMOTE_IP'] = '195.65.29.254' # ch
     params = {}
-    get '/api/v1/users', params.to_json, @headers
+    get '/api/v1/users', params: params.to_json, headers: @headers
     assert_response(200)
     result = JSON.parse(@response.body)
 

test/test_helper.rb

@@ -62,6 +62,8 @@ class ActiveSupport::TestCase
     PostmasterFilter.destroy_all
     Ticket.destroy_all
     Taskbar.destroy_all
+    Sla.destroy_all
+    Calendar.destroy_all
 
     # reset settings
     Setting.all.pluck(:name).each { |name|

test/unit/assets_test.rb

@@ -446,7 +446,21 @@ class AssetsTest < ActiveSupport::TestCase
       roles: roles,
     )
 
-    calendar1 = Calendar.first
+    calendar1 = Calendar.create_or_update(
+      name: 'US 1',
+      timezone: 'America/Los_Angeles',
+      business_hours: {
+        mon: { '09:00' => '17:00' },
+        tue: { '09:00' => '17:00' },
+        wed: { '09:00' => '17:00' },
+        thu: { '09:00' => '17:00' },
+        fri: { '09:00' => '17:00' }
+      },
+      default: true,
+      ical_url: nil,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
     ticket_state1 = Ticket::State.find_by(name: 'new')
     ticket_state2 = Ticket::State.find_by(name: 'open')
     sla = Sla.create_or_update(

test/unit/calendar_test.rb

@@ -61,7 +61,7 @@ class CalendarTest < ActiveSupport::TestCase
     assert_equal(true, calendar3.default)
 
     calendar2.default = true
-    calendar2.save
+    calendar2.save!
 
     calendar1 = Calendar.find_by(name: 'US 1')
     calendar2 = Calendar.find_by(name: 'US 2')
@@ -72,7 +72,7 @@ class CalendarTest < ActiveSupport::TestCase
     assert_equal(false, calendar3.default)
 
     calendar2.default = false
-    calendar2.save
+    calendar2.save!
 
     calendar1 = Calendar.find_by(name: 'US 1')
     calendar2 = Calendar.find_by(name: 'US 2')
@@ -82,12 +82,16 @@ class CalendarTest < ActiveSupport::TestCase
     assert_equal(false, calendar2.default)
     assert_equal(false, calendar3.default)
 
-    calendar1.destroy
+    calendar1.destroy!
     calendar2 = Calendar.find_by(name: 'US 2')
     calendar3 = Calendar.find_by(name: 'US 3')
 
     assert_equal(true, calendar2.default)
     assert_equal(false, calendar3.default)
+
+    calendar2.destroy!
+    calendar3.destroy!
+
     travel_back
   end
 
@@ -237,6 +241,8 @@ class CalendarTest < ActiveSupport::TestCase
     assert_equal('Christmas1', calendar1.public_holidays['2019-12-24']['summary'])
     assert_nil(calendar1.public_holidays['2020-12-24'])
 
+    calendar1.destroy!
+
     travel_back
 
   end

test/unit/karma_test.rb

@@ -91,7 +91,7 @@ class KarmaTest < ActiveSupport::TestCase
     ticket1.updated_by_id = agent1.id
     ticket1.updated_at = Time.zone.now - 9.hours
     ticket1.created_at = Time.zone.now - 9.hours
-    ticket1.save
+    ticket1.save!
 
     # execute object transaction
     Observer::Transaction.commit
@@ -113,7 +113,7 @@ class KarmaTest < ActiveSupport::TestCase
     ticket1.updated_by_id = agent1.id
     ticket1.updated_at = Time.zone.now - 9.hours
     ticket1.created_at = Time.zone.now - 9.hours
-    ticket1.save
+    ticket1.save!
 
     # execute object transaction
     Observer::Transaction.commit
@@ -127,7 +127,7 @@ class KarmaTest < ActiveSupport::TestCase
     ticket1.updated_by_id = agent2.id
     ticket1.updated_at = Time.zone.now - 9.hours
     ticket1.created_at = Time.zone.now - 9.hours
-    ticket1.save
+    ticket1.save!
 
     # execute object transaction
     Observer::Transaction.commit
@@ -138,7 +138,7 @@ class KarmaTest < ActiveSupport::TestCase
     assert_equal(0, Karma.score_by_user(customer1))
 
     ticket1.state = Ticket::State.lookup(name: 'open')
-    ticket1.save
+    ticket1.save!
 
     Ticket::Article.create(
       ticket_id: ticket1.id,
@@ -169,7 +169,7 @@ class KarmaTest < ActiveSupport::TestCase
     Scheduler.worker(true)
 
     ticket1.state = Ticket::State.lookup(name: 'closed')
-    ticket1.save
+    ticket1.save!
 
     # execute object transaction
     Observer::Transaction.commit
@@ -385,7 +385,7 @@ class KarmaTest < ActiveSupport::TestCase
 
     ticket2.state = Ticket::State.lookup(name: 'pending reminder')
     ticket2.pending_time = Time.zone.now - 1.day
-    ticket2.save
+    ticket2.save!
 
     Ticket.process_pending
 
@@ -395,7 +395,7 @@ class KarmaTest < ActiveSupport::TestCase
 
     ticket2.state = Ticket::State.lookup(name: 'pending reminder')
     ticket2.pending_time = Time.zone.now - 3.days
-    ticket2.save
+    ticket2.save!
 
     Ticket.process_pending
 
@@ -459,7 +459,7 @@ class KarmaTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     ticket2.state = Ticket::State.lookup(name: 'open')
-    ticket2.save
+    ticket2.save!
 
     Observer::Transaction.commit
     Scheduler.worker(true)
@@ -511,10 +511,10 @@ class KarmaTest < ActiveSupport::TestCase
     assert_equal('Hero', Karma::User.level_by_score(50_000))
 
     # cleanup
-    ticket1.destroy
+    ticket1.destroy!
-    ticket2.destroy
+    ticket2.destroy!
-    calendar1.destroy
+    calendar1.destroy!
-    sla1.destroy
+    sla1.destroy!
 
   end
 

test/unit/package_test.rb

@@ -33,7 +33,7 @@ class PackageTest < ActiveSupport::TestCase
     {
       "permission": "644",
       "location": "db/addon/unit_test_sample/20121212000001_create_base.rb",
-      "content": "Y2xhc3MgQ3JlYXRlQmFzZSA8IEFjdGl2ZVJlY29yZDo6TWlncmF0aW9uDQogIGRlZiBzZWxmLnVw\nDQogICBjcmVhdGVfdGFibGUgOnNhbXBsZV90YWJsZXMgZG8gfHR8DQogICAgICB0LmNvbHVtbiA6\nbmFtZSwgICAgICAgICAgIDpzdHJpbmcsIDpsaW1pdCA9PiAxNTAsICA6bnVsbCA9PiB0cnVlDQog\nICAgICB0LmNvbHVtbiA6ZGF0YSwgICAgICAgICAgIDpzdHJpbmcsIDpsaW1pdCA9PiA1MDAwLCA6\nbnVsbCA9PiB0cnVlDQogICAgZW5kDQogIGVuZA0KDQogIGRlZiBzZWxmLmRvd24NCiAgICBkcm9w\nX3RhYmxlIDpzYW1wbGVfdGFibGVzDQogIGVuZA0KZW5k"
+      "content": "Y2xhc3MgQ3JlYXRlQmFzZSA8IEFjdGl2ZVJlY29yZDo6TWlncmF0aW9uWzQuMl0NCiAgZGVmIHNlbGYudXANCiAgIGNyZWF0ZV90YWJsZSA6c2FtcGxlX3RhYmxlcyBkbyB8dHwNCiAgICAgIHQuY29sdW1uIDpuYW1lLCAgICAgICAgICAgOnN0cmluZywgOmxpbWl0ID0+IDE1MCwgIDpudWxsID0+IHRydWUNCiAgICAgIHQuY29sdW1uIDpkYXRhLCAgICAgICAgICAgOnN0cmluZywgOmxpbWl0ID0+IDUwMDAsIDpudWxsID0+IHRydWUNCiAgICBlbmQNCiAgZW5kDQoNCiAgZGVmIHNlbGYuZG93bg0KICAgIGRyb3BfdGFibGUgOnNhbXBsZV90YWJsZXMNCiAgZW5kDQplbmQ="
     }
   ]
 }',
@@ -168,7 +168,7 @@ class PackageTest < ActiveSupport::TestCase
     {
       "permission": "644",
       "location": "db/addon/unit_test_sample/20121212000001_create_base.rb",
-      "content": "Y2xhc3MgQ3JlYXRlQmFzZSA8IEFjdGl2ZVJlY29yZDo6TWlncmF0aW9uDQogIGRlZiBzZWxmLnVw\nDQogICBjcmVhdGVfdGFibGUgOnNhbXBsZV90YWJsZXMgZG8gfHR8DQogICAgICB0LmNvbHVtbiA6\nbmFtZSwgICAgICAgICAgIDpzdHJpbmcsIDpsaW1pdCA9PiAxNTAsICA6bnVsbCA9PiB0cnVlDQog\nICAgICB0LmNvbHVtbiA6ZGF0YSwgICAgICAgICAgIDpzdHJpbmcsIDpsaW1pdCA9PiA1MDAwLCA6\nbnVsbCA9PiB0cnVlDQogICAgZW5kDQogIGVuZA0KDQogIGRlZiBzZWxmLmRvd24NCiAgICBkcm9w\nX3RhYmxlIDpzYW1wbGVfdGFibGVzDQogIGVuZA0KZW5k"
+      "content": "Y2xhc3MgQ3JlYXRlQmFzZSA8IEFjdGl2ZVJlY29yZDo6TWlncmF0aW9uWzQuMl0NCiAgZGVmIHNlbGYudXANCiAgIGNyZWF0ZV90YWJsZSA6c2FtcGxlX3RhYmxlcyBkbyB8dHwNCiAgICAgIHQuY29sdW1uIDpuYW1lLCAgICAgICAgICAgOnN0cmluZywgOmxpbWl0ID0+IDE1MCwgIDpudWxsID0+IHRydWUNCiAgICAgIHQuY29sdW1uIDpkYXRhLCAgICAgICAgICAgOnN0cmluZywgOmxpbWl0ID0+IDUwMDAsIDpudWxsID0+IHRydWUNCiAgICBlbmQNCiAgZW5kDQoNCiAgZGVmIHNlbGYuZG93bg0KICAgIGRyb3BfdGFibGUgOnNhbXBsZV90YWJsZXMNCiAgZW5kDQplbmQ="
     }
   ]
 }',
@@ -248,7 +248,7 @@ class PackageTest < ActiveSupport::TestCase
     {
       "permission": "644",
       "location": "db/addon/unit_test_sample/20121212000001_create_base.rb",
-      "content": "Y2xhc3MgQ3JlYXRlQmFzZSA8IEFjdGl2ZVJlY29yZDo6TWlncmF0aW9uDQogIGRlZiBzZWxmLnVw\nDQogICBjcmVhdGVfdGFibGUgOnNhbXBsZV90YWJsZXMgZG8gfHR8DQogICAgICB0LmNvbHVtbiA6\nbmFtZSwgICAgICAgICAgIDpzdHJpbmcsIDpsaW1pdCA9PiAxNTAsICA6bnVsbCA9PiB0cnVlDQog\nICAgICB0LmNvbHVtbiA6ZGF0YSwgICAgICAgICAgIDpzdHJpbmcsIDpsaW1pdCA9PiA1MDAwLCA6\nbnVsbCA9PiB0cnVlDQogICAgZW5kDQogIGVuZA0KDQogIGRlZiBzZWxmLmRvd24NCiAgICBkcm9w\nX3RhYmxlIDpzYW1wbGVfdGFibGVzDQogIGVuZA0KZW5k"
+      "content": "Y2xhc3MgQ3JlYXRlQmFzZSA8IEFjdGl2ZVJlY29yZDo6TWlncmF0aW9uWzQuMl0NCiAgZGVmIHNlbGYudXANCiAgIGNyZWF0ZV90YWJsZSA6c2FtcGxlX3RhYmxlcyBkbyB8dHwNCiAgICAgIHQuY29sdW1uIDpuYW1lLCAgICAgICAgICAgOnN0cmluZywgOmxpbWl0ID0+IDE1MCwgIDpudWxsID0+IHRydWUNCiAgICAgIHQuY29sdW1uIDpkYXRhLCAgICAgICAgICAgOnN0cmluZywgOmxpbWl0ID0+IDUwMDAsIDpudWxsID0+IHRydWUNCiAgICBlbmQNCiAgZW5kDQoNCiAgZGVmIHNlbGYuZG93bg0KICAgIGRyb3BfdGFibGUgOnNhbXBsZV90YWJsZXMNCiAgZW5kDQplbmQ="
     }
   ]
 }',
@@ -304,57 +304,57 @@ class PackageTest < ActiveSupport::TestCase
     tests.each { |test|
       if test[:action] == 'install'
         begin
-          package = Package.install( string: test[:zpm] )
+          package = Package.install(string: test[:zpm])
         rescue => e
           puts 'ERROR: ' + e.inspect
         end
         if test[:result]
-          assert( package, 'install package not successful' )
+          assert(package, 'install package not successful')
           issues = package.verify
-          assert( !issues, 'package verify not successful' )
+          assert(!issues, 'package verify not successful')
         else
-          assert( !package, 'install package successful but should not' )
+          assert(!package, 'install package successful but should not')
         end
       elsif test[:action] == 'reinstall'
         begin
-          package = Package.reinstall( test[:name] )
+          package = Package.reinstall(test[:name])
         rescue
           package = false
         end
         if test[:result]
-          assert( package, 'reinstall package not successful' )
+          assert(package, 'reinstall package not successful')
           issues = package.verify
-          assert( !issues, 'package verify not successful' )
+          assert(!issues, 'package verify not successful')
         else
-          assert( !package, 'reinstall package successful but should not' )
+          assert(!package, 'reinstall package successful but should not')
         end
       elsif test[:action] == 'uninstall'
         if test[:zpm]
           begin
-            package = Package.uninstall( string: test[:zpm] )
+            package = Package.uninstall(string: test[:zpm])
           rescue
             package = false
           end
         else
           begin
-            package = Package.uninstall( name: test[:name], version: test[:version] )
+            package = Package.uninstall(name: test[:name], version: test[:version])
           rescue
             package = false
           end
         end
         if test[:result]
-          assert( package, 'uninstall package not successful' )
+          assert(package, 'uninstall package not successful')
         else
-          assert( !package, 'uninstall package successful but should not' )
+          assert(!package, 'uninstall package successful but should not')
         end
       elsif test[:action] == 'auto_install'
         if test[:zpm]
-          if !File.exist?( Rails.root.to_s + '/auto_install/' )
+          if !File.exist?(Rails.root.to_s + '/auto_install/')
-            Dir.mkdir( Rails.root.to_s + '/auto_install/', 0o755)
+            Dir.mkdir(Rails.root.to_s + '/auto_install/', 0o755)
           end
           location = Rails.root.to_s + '/auto_install/unittest.zpm'
-          file = File.new( location, 'wb' )
+          file = File.new(location, 'wb')
-          file.write( test[:zpm] )
+          file.write(test[:zpm])
           file.close
         end
         begin
@@ -363,22 +363,22 @@ class PackageTest < ActiveSupport::TestCase
           success = false
         end
         if test[:zpm]
-          File.delete( location )
+          File.delete(location )
         end
       end
       if test[:verify] && test[:verify][:package]
-        exists = Package.where( name: test[:verify][:package][:name], version: test[:verify][:package][:version] ).first
+        exists = Package.where(name: test[:verify][:package][:name], version: test[:verify][:package][:version]).first
-        assert( exists, "package '#{test[:verify][:package][:name]}' is not installed" )
+        assert(exists, "package '#{test[:verify][:package][:name]}' is not installed" )
       end
       next if !test[:verify]
       next if !test[:verify][:check_files]
 
       test[:verify][:check_files].each { |item|
-        exists = File.exist?( item[:location] )
+        exists = File.exist?(item[:location])
         if item[:result]
-          assert( exists, "'#{item[:location]}' exists" )
+          assert(exists, "'#{item[:location]}' exists" )
         else
-          assert( !exists, "'#{item[:location]}' doesn't exists" )
+          assert(!exists, "'#{item[:location]}' doesn't exists" )
         end
       }
     }

test/unit/ticket_escalation_test.rb

@@ -0,0 +1,129 @@
+# encoding: utf-8
+require 'test_helper'
+
+class TicketEscalationTest < ActiveSupport::TestCase
+  test 'ticket create' do
+    ticket = Ticket.new(
+      title: 'some value 123',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    ticket.save!
+    assert(ticket, 'ticket created')
+    assert_not(ticket.escalation_at)
+    assert_not(ticket.has_changes_to_save?)
+
+    article = Ticket::Article.create!(
+      ticket_id: ticket.id,
+      type_id: Ticket::Article::Type.find_by(name: 'note').id,
+      sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id,
+      body: 'some body',
+      internal: false,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_not(article.has_changes_to_save?)
+    assert_not(ticket.has_changes_to_save?)
+
+    calendar = Calendar.create_or_update(
+      name: 'Escalation Test',
+      timezone: 'Europe/Berlin',
+      business_hours: {
+        mon: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+        tue: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+        wed: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+        thu: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+        fri: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+        sat: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+        sun: {
+          active: true,
+          timeframes: [ ['00:00', '23:59'] ]
+        },
+      },
+      default: true,
+      ical_url: nil,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    sla = Sla.create_or_update(
+      name: 'test sla 1',
+      condition: {
+        'ticket.title' => {
+          operator: 'contains',
+          value: 'some value 123',
+        },
+      },
+      first_response_time: 60,
+      update_time: 180,
+      solution_time: 240,
+      calendar_id: calendar.id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    ticket = Ticket.new(
+      title: 'some value 123',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    ticket.save!
+    assert(ticket, 'ticket created')
+    ticket_escalation_at = ticket.escalation_at
+    assert(ticket.escalation_at)
+    assert_not(ticket.has_changes_to_save?)
+
+    article = Ticket::Article.create!(
+      ticket_id: ticket.id,
+      type_id: Ticket::Article::Type.find_by(name: 'note').id,
+      sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id,
+      body: 'some body',
+      internal: false,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_not(article.has_changes_to_save?)
+    assert_not(ticket.has_changes_to_save?)
+
+    travel 1.second
+
+    sla.first_response_time = 30
+    sla.save!
+
+    ticket.save!
+    assert_not(ticket.has_changes_to_save?)
+    assert(ticket.escalation_at)
+    assert_not_equal(ticket_escalation_at.to_s, ticket.escalation_at.to_s)
+
+    sla.destroy!
+    calendar.destroy!
+
+    ticket.save!
+    assert_not(ticket.has_changes_to_save?)
+    assert_not(ticket.escalation_at)
+
+  end
+
+end

test/unit/ticket_sla_test.rb

@@ -497,6 +497,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_equal(ticket.last_contact_agent_at.to_s, article_outbound.created_at.to_s, 'ticket.last_contact_agent_at verify - inbound')
     assert_equal(ticket.first_response_at.to_s, article_outbound.created_at.to_s, 'ticket.first_response_at verify - inbound')
     assert_nil(ticket.close_at, 'ticket.close_at verify - inbound')
+    calendar1.destroy!
+    calendar2.destroy!
   end
 
   test 'ticket sla + selector' do
@@ -605,6 +607,7 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_nil(ticket.close_in_min, 'ticket.close_in_min')
     assert_nil(ticket.close_diff_in_min, 'ticket.close_diff_in_min')
 
+    calendar1.destroy!
   end
 
   test 'ticket sla + timezone + holiday' do
@@ -849,12 +852,13 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_equal(ticket.update_escalation_at.gmtime.to_s, '2015-09-23 08:30:00 UTC', 'ticket.update_escalation_at verify 1')
     assert_equal(ticket.close_escalation_at.gmtime.to_s, '2015-09-23 09:30:00 UTC', 'ticket.close_escalation_at verify 1')
 
-    delete = sla.destroy
+    delete = sla.destroy!
     assert(delete, 'sla destroy')
 
-    delete = ticket.destroy
+    delete = ticket.destroy!
     assert(delete, 'ticket destroy')
 
+    calendar.destroy!
   end
 
   test 'ticket escalation suspend close reopen bug' do
@@ -1043,13 +1047,15 @@ class TicketSlaTest < ActiveSupport::TestCase
     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')
 
-    delete = sla.destroy
+    delete = sla.destroy!
     assert(delete, 'sla destroy')
 
-    delete = ticket1.destroy
+    delete = ticket1.destroy!
     assert(delete, 'ticket1 destroy')
-    delete = ticket2.destroy
+    delete = ticket2.destroy!
     assert(delete, 'ticket2 destroy')
+
+    calendar.destroy!
   end
 
   test 'ticket escalation suspend' do
@@ -1552,12 +1558,13 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_equal(ticket.close_in_min, 60, 'ticket.close_in_min verify 3')
     assert_equal(ticket.close_diff_in_min, 180, 'ticket.close_diff_in_min# verify 3')
 
-    delete = sla.destroy
+    delete = sla.destroy!
     assert(delete, 'sla destroy')
 
-    delete = ticket.destroy
+    delete = ticket.destroy!
     assert(delete, 'ticket destroy')
 
+    calendar.destroy!
   end
 
   test 'ticket ticket.title and article.subject' do
@@ -1718,12 +1725,13 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_nil(ticket.close_in_min, 'ticket.close_in_min verify 3')
     assert_nil(ticket.close_diff_in_min, 'ticket.close_diff_in_min# verify 3')
 
-    delete = sla.destroy
+    delete = sla.destroy!
     assert(delete, 'sla destroy')
 
-    delete = ticket.destroy
+    delete = ticket.destroy!
     assert(delete, 'ticket destroy')
 
+    calendar.destroy!
   end
 
   test 'ticket sla + holiday 222' do
@@ -1943,11 +1951,13 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_equal(ticket.update_escalation_at.gmtime.to_s, '2016-11-09 10:26:36 UTC', 'ticket.update_escalation_at verify 1')
     assert_nil(ticket.close_escalation_at, 'ticket.close_escalation_at verify 1')
 
-    delete = sla.destroy
+    delete = sla.destroy!
     assert(delete, 'sla destroy')
 
-    delete = ticket.destroy
+    delete = ticket.destroy!
     assert(delete, 'ticket destroy')
+
+    calendar.destroy!
   end
 
   test 'ticket sla + observer check' do
@@ -2124,11 +2134,13 @@ class TicketSlaTest < ActiveSupport::TestCase
     assert_equal(ticket.update_escalation_at.gmtime.to_s, '2016-11-07 15:26:36 UTC', 'ticket.update_escalation_at verify 1')
     assert_nil(ticket.close_escalation_at, 'ticket.close_escalation_at verify 1')
 
-    delete = sla.destroy
+    delete = sla.destroy!
     assert(delete, 'sla destroy')
 
-    delete = ticket.destroy
+    delete = ticket.destroy!
     assert(delete, 'ticket destroy')
+
+    calendar.destroy!
   end
 
 end