Fixes #2620 - Deleting articles with accounted time to not affect accounted time.

app/models/ticket/article.rb

@@ -22,7 +22,7 @@ class Ticket::Article < ApplicationModel
   before_save   :touch_ticket_if_needed
   before_create :check_subject, :check_body, :check_message_id_md5
   before_update :check_subject, :check_body, :check_message_id_md5
-  after_destroy :store_delete
+  after_destroy :store_delete, :update_time_units
 
   store :preferences
 
@@ -339,6 +339,11 @@ returns
     )
   end
 
+  # recalculate time accounting
+  def update_time_units
+    Ticket::TimeAccounting.update_ticket(ticket)
+  end
+
   def touch_ticket_if_needed
     return if !internal_changed?
 

app/models/ticket/time_accounting.rb

@@ -4,25 +4,22 @@ class Ticket::TimeAccounting < ApplicationModel
   belongs_to :ticket, optional: true
   belongs_to :ticket_article, class_name: 'Ticket::Article', inverse_of: :ticket_time_accounting, optional: true
 
-  after_create :ticket_time_unit_update
-  after_update :ticket_time_unit_update
+  after_create :update_time_units
+  after_update :update_time_units
 
-  def ticket_time_unit_update
-    exists = false
-    time_units = 0
-    Ticket::TimeAccounting.where(ticket_id: ticket_id).each do |record|
-      time_units += record.time_unit
-      exists = true
+  def update_time_units
+    self.class.update_ticket(ticket)
   end
-    return false if exists == false
 
-    ticket = Ticket.lookup(id: ticket_id)
-    return false if !ticket
-    return false if ticket.time_unit == time_units
+  def self.update_ticket(ticket)
+    time_units = total(ticket)
+    return if ticket.time_unit.to_d == time_units
 
     ticket.time_unit = time_units
     ticket.save!
-    true
   end
 
+  def self.total(ticket)
+    ticket.ticket_time_accounting.sum(:time_unit)
+  end
 end

spec/models/ticket/time_accounting_spec.rb

@@ -22,6 +22,42 @@ RSpec.describe Ticket::TimeAccounting, type: :model do
             .to change(described_class, :count).by(-1)
         end
       end
+
+      context 'when recalculating articles' do
+        let(:ticket)   { create(:ticket) }
+        let(:article1) { create(:ticket_article, ticket: ticket) }
+        let(:article2) { create(:ticket_article, ticket: ticket) }
+
+        it 'one article' do
+          time_accounting = create(:'ticket/time_accounting', ticket: ticket, ticket_article: article1)
+
+          expect(ticket.reload.time_unit).to eq(time_accounting.time_unit)
+        end
+
+        it 'multiple article' do
+          time_accounting1 = create(:'ticket/time_accounting', ticket: ticket, ticket_article: article1, time_unit: 5.5)
+          time_accounting2 = create(:'ticket/time_accounting', ticket: ticket, ticket_article: article2, time_unit: 10.5)
+
+          expect(ticket.reload.time_unit).to eq(time_accounting1.time_unit + time_accounting2.time_unit)
+        end
+
+        it 'destroy article' do
+          time_accounting1 = create(:'ticket/time_accounting', ticket: ticket, ticket_article: article1, time_unit: 5.5)
+          create(:'ticket/time_accounting', ticket: ticket, ticket_article: article2, time_unit: 10.5)
+          article2.destroy
+
+          expect(ticket.reload.time_unit).to eq(time_accounting1.time_unit)
+        end
+
+        it 'destroy all articles' do
+          create(:'ticket/time_accounting', ticket: ticket, ticket_article: article1, time_unit: 5.5)
+          create(:'ticket/time_accounting', ticket: ticket, ticket_article: article2, time_unit: 10.5)
+          article1.destroy
+          article2.destroy
+
+          expect(ticket.reload.time_unit).to eq(0)
+        end
+      end
     end
   end
 end