Fixed bug: Empty ActiveRecord::AttributeMethods::Dirty#saved_changes in cache invalidation callback (ApplicationModel::HasCache) leads to wrong cache entries if record is a ticket.

app/models/observer/ticket/escalation_update.rb

@@ -14,9 +14,15 @@ class Observer::Ticket::EscalationUpdate < ActiveRecord::Observer
     # return if we run import mode
     return true if Setting.get('import_mode')
 
-    return true if !Ticket.exists?(record.id)
+    # we need to fetch the a new instance of the record
+    # from the DB instead of using `record.reload`
+    # because Ticket#reload clears the ActiveMode::Dirty state
+    # state of the record instance which leads to empty
+    # Ticket#saved_changes (etc.) results in other callbacks
+    # later in the chain
+    updated_ticket = Ticket.find_by(id: record.id)
+    return true if !updated_ticket
 
-    record.reload
-    record.escalation_calculation
+    updated_ticket.escalation_calculation
   end
 end

spec/models/ticket_spec.rb

@@ -450,7 +450,7 @@ RSpec.describe Ticket, type: :model do
 
           it 'switches to "open"' do
             expect { article }
-              .to change { ticket.state.name }.from('new').to('open')
+              .to change { ticket.reload.state.name }.from('new').to('open')
           end
         end
       end
@@ -462,7 +462,7 @@ RSpec.describe Ticket, type: :model do
           let(:article) { create(:ticket_article, ticket: ticket, sender_name: 'Agent') }
 
           it 'stays "closed"' do
-            expect { article }.not_to change { ticket.state.name }
+            expect { article }.not_to change { ticket.reload.state.name }
           end
         end
       end
@@ -503,7 +503,7 @@ RSpec.describe Ticket, type: :model do
         before { sla }  # create sla
 
         it 'is set based on SLA’s #first_response_time' do
-          expect(ticket.escalation_at.to_i)
+          expect(ticket.reload.escalation_at.to_i)
             .to eq(1.hour.from_now.to_i)
         end
 
@@ -540,7 +540,7 @@ RSpec.describe Ticket, type: :model do
 
         it 'is updated based on the new SLA’s #first_response_time' do
           expect { ticket.save! }
-            .to change { ticket.escalation_at.to_i }.from(0).to(1.hour.from_now.to_i)
+            .to change { ticket.reload.escalation_at.to_i }.from(0).to(1.hour.from_now.to_i)
         end
       end
 
@@ -553,7 +553,7 @@ RSpec.describe Ticket, type: :model do
 
         it 'is set to nil' do
           expect { ticket.save! }
-            .to change(ticket, :escalation_at).to(nil)
+            .to change { ticket.reload.escalation_at }.to(nil)
         end
       end
     end
@@ -574,7 +574,7 @@ RSpec.describe Ticket, type: :model do
         before { sla }  # create sla
 
         it 'is set based on SLA’s #first_response_time' do
-          expect(ticket.first_response_escalation_at.to_i)
+          expect(ticket.reload.first_response_escalation_at.to_i)
             .to eq(1.hour.from_now.to_i)
         end
 
@@ -606,7 +606,7 @@ RSpec.describe Ticket, type: :model do
         before { sla }  # create sla
 
         it 'is set based on SLA’s #update_time' do
-          expect(ticket.update_escalation_at.to_i)
+          expect(ticket.reload.update_escalation_at.to_i)
             .to eq(3.hours.from_now.to_i)
         end
 
@@ -642,7 +642,7 @@ RSpec.describe Ticket, type: :model do
         before { sla }  # create sla
 
         it 'is set based on SLA’s #solution_time' do
-          expect(ticket.close_escalation_at.to_i)
+          expect(ticket.reload.close_escalation_at.to_i)
             .to eq(4.hours.from_now.to_i)
         end
 

test/unit/ticket_sla_test.rb

@@ -165,6 +165,7 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       first_response_at: '2013-03-21 10:00:00 UTC',
     )
+    ticket.reload
 
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 11:30:00 UTC', 'ticket.escalation_at verify 3')
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 3')
@@ -184,6 +185,7 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       first_response_at: '2013-03-21 14:00:00 UTC',
     )
+    ticket.reload
 
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 11:30:00 UTC', 'ticket.escalation_at verify 4')
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 4')
@@ -203,6 +205,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       last_contact_agent_at: '2013-03-21 11:00:00 UTC',
     )
+    ticket.reload
+
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 12:30:00 UTC', 'ticket.escalation_at verify 5')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 5')
@@ -222,6 +226,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       last_contact_agent_at: '2013-03-21 12:00:00 UTC',
     )
+    ticket.reload
+
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 12:30:00 UTC', 'ticket.escalation_at verify 6')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 6')
@@ -241,6 +247,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       last_contact_customer_at: '2013-03-21 12:05:00 UTC',
     )
+    ticket.reload
+
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 12:30:00 UTC', 'ticket.escalation_at verify 6')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 6')
@@ -260,6 +268,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       last_contact_agent_at: '2013-03-21 12:10:00 UTC',
     )
+    ticket.reload
+
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 12:30:00 UTC', 'ticket.escalation_at verify 6')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 6')
@@ -279,6 +289,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       close_at: '2013-03-21 11:30:00 UTC',
     )
+    ticket.reload
+
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 14:10:00 UTC', 'ticket.escalation_at verify 7')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 7')
@@ -298,6 +310,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       close_at: '2013-03-21 13:00:00 UTC',
     )
+    ticket.reload
+
     assert_equal(ticket.escalation_at.gmtime.to_s, '2013-03-21 14:10:00 UTC', 'ticket.escalation_at verify 8')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 8')
@@ -317,6 +331,8 @@ class TicketSlaTest < ActiveSupport::TestCase
     ticket.update!(
       state: Ticket::State.lookup(name: 'closed')
     )
+    ticket.reload
+
     assert_nil(ticket.escalation_at, 'ticket.escalation_at verify 9')
 
     assert_equal(ticket.first_response_escalation_at.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escalation_at verify 9')