From b81d31a42fb97871d4f393a16f3a7cea9cdd2e24 Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@zammad.com>
Date: Fri, 31 Jul 2020 14:25:02 +0200
Subject: [PATCH] Fixed issue #3136 - Attribute changes remove Owner of closed
 Ticket.

---
 app/models/ticket.rb       |  3 +++
 spec/models/ticket_spec.rb | 41 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/app/models/ticket.rb b/app/models/ticket.rb
index 2eb30dcc7..f09ca42e3 100644
--- a/app/models/ticket.rb
+++ b/app/models/ticket.rb
@@ -1346,6 +1346,9 @@ result
   def check_owner_active
     return true if Setting.get('import_mode')
 
+    # only change the owner for non closed Tickets for historical/reporting reasons
+    return true if state.present? && Ticket::StateType.lookup(id: state.state_type_id)&.name == 'closed'
+
     # return when ticket is unassigned
     return true if owner_id.blank?
     return true if owner_id == 1
diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb
index e4205721f..4c4523dc5 100644
--- a/spec/models/ticket_spec.rb
+++ b/spec/models/ticket_spec.rb
@@ -537,6 +537,47 @@ RSpec.describe Ticket, type: :model do
               .to change { ticket.reload.owner }.to(User.first)
           end
         end
+
+        context 'when the Ticket is closed' do
+
+          before do
+            ticket.update!(state: Ticket::State.lookup(name: 'closed'))
+          end
+
+          context 'if original owner is still an active agent belonging to ticket.group' do
+            it 'does not change' do
+              expect { create(:ticket_article, ticket: ticket) }
+                .not_to change { ticket.reload.owner }
+            end
+          end
+
+          context 'if original owner has left ticket.group' do
+            before { original_owner.groups = [] }
+
+            it 'does not change' do
+              expect { create(:ticket_article, ticket: ticket) }
+                .not_to change { ticket.reload.owner }
+            end
+          end
+
+          context 'if original owner has become inactive' do
+            before { original_owner.update(active: false) }
+
+            it 'does not change' do
+              expect { create(:ticket_article, ticket: ticket) }
+                .not_to change { ticket.reload.owner }
+            end
+          end
+
+          context 'if original owner has lost agent status' do
+            before { original_owner.roles = [create(:role)] }
+
+            it 'does not change' do
+              expect { create(:ticket_article, ticket: ticket) }
+                .not_to change { ticket.reload.owner }
+            end
+          end
+        end
       end
     end