Fixed #1690 - renaming tags will break overviews condition

app/models/tag.rb

@@ -259,6 +259,8 @@ rename tag items
         return true
       end
 
+      update_referenced_objects(old_tag_item.name, new_tag_name)
+
       # update new tag name
       old_tag_item.name = new_tag_name
       old_tag_item.save
@@ -303,6 +305,52 @@ remove tag item (destroy with reverences)
       true
     end
 
+=begin
+
+  Update referenced objects such as triggers, overviews, schedulers, and postmaster filters
+
+  Specifically, the following fields are updated:
+
+  Overview.condition
+  Trigger.condition   Trigger.perform
+  Job.condition       Job.perform
+                      PostmasterFilter.perform
+
+=end
+
+    def self.update_referenced_objects(old_name, new_name)
+      objects = Overview.all + Trigger.all + Job.all + PostmasterFilter.all
+
+      objects.each do |object|
+        changed = false
+        if object.has_attribute?(:condition)
+          changed |= update_condition_hash object.condition, old_name, new_name
+        end
+        if object.has_attribute?(:perform)
+          changed |= update_condition_hash object.perform, old_name, new_name
+        end
+        object.save if changed
+      end
+    end
+
+    def self.update_condition_hash(hash, old_name, new_name)
+      changed = false
+      hash.each do |key, condition|
+        next if %w[ticket.tags x-zammad-ticket-tags].exclude? key
+        next if condition[:value].split(', ').exclude? old_name
+        condition[:value] = update_name(condition[:value], old_name, new_name)
+        changed = true
+      end
+      changed
+    end
+
+    def self.update_name(condition, old_name, new_name)
+      tags = condition.split(', ')
+      return new_name if tags.size == 1
+      tags = tags.map { |t| t == old_name ? new_name : t }
+      tags.join(', ')
+    end
+
   end
 
 end

spec/factories/job.rb

@@ -0,0 +1,39 @@
+FactoryBot.define do
+  factory :job do
+    sequence(:name) { |n| "Test job #{n}"  }
+    condition     { { 'ticket.state_id' => { 'operator' => 'is not', 'value' => 4 } } }
+    perform       { { 'ticket.state_id' => { 'value' => 4 } } }
+    active        true
+    created_by_id 1
+    updated_by_id 1
+    timeplan      do
+      { 'days' => { 'Mon' => true, 'Tue' => false, 'Wed' => false, 'Thu' => false, 'Fri' => false, 'Sat' => false, 'Sun' => false },
+        'hours' =>
+       { '0' => true,
+         '1' => false,
+         '2' => false,
+         '3' => false,
+         '4' => false,
+         '5' => false,
+         '6' => false,
+         '7' => false,
+         '8' => false,
+         '9' => false,
+         '10' => false,
+         '11' => false,
+         '12' => false,
+         '13' => false,
+         '14' => false,
+         '15' => false,
+         '16' => false,
+         '17' => false,
+         '18' => false,
+         '19' => false,
+         '20' => false,
+         '21' => false,
+         '22' => false,
+         '23' => false },
+        'minutes' => { '0' => true, '10' => false, '20' => false, '30' => false, '40' => false, '50' => false } }
+    end
+  end
+end

spec/factories/postmaster_filter.rb

@@ -0,0 +1,11 @@
+FactoryBot.define do
+  factory :postmaster_filter do
+    sequence(:name) { |n| "Test PostmasterFilter #{n}"  }
+    channel       'email'
+    match         { { 'from' => { 'operator' => 'contains', 'value' => 'a' } } }
+    perform       { { 'x-zammad-ticket-tags' => { 'operator' => 'remove', 'value' => 'test2, test7' } } }
+    active        true
+    created_by_id 1
+    updated_by_id 1
+  end
+end

spec/models/tag_spec.rb

@@ -0,0 +1,90 @@
+require 'rails_helper'
+
+RSpec.describe Tag do
+
+  context 'rename' do
+    before(:each) do
+      Tag::Item.lookup_by_name_and_create('test1')
+    end
+
+    def tag_rename
+      Tag::Item.rename(
+        id: Tag::Item.lookup(name: 'test1').id,
+        name: 'test1_renamed',
+        updated_by_id: 1,
+      )
+    end
+
+    it 'overview conditions with a single tag' do
+      object = create :overview, condition: { 'ticket.tags' => { operator: 'contains one', value: 'test1' } }
+      tag_rename
+      expect(Overview.find(object.id).condition['ticket.tags'][:value]).to eq('test1_renamed')
+    end
+
+    it 'overview conditions with a tag list ' do
+      object = create :overview, condition: { 'ticket.tags' => { operator: 'contains all', value: 'test1, test2, test3' } }
+      tag_rename
+      expect(Overview.find(object.id).condition['ticket.tags'][:value]).to eq('test1_renamed, test2, test3')
+    end
+
+    it 'trigger conditions with a single tag' do
+      object = create :trigger, condition: { 'ticket.tags' => { operator: 'contains one', value: 'test1' } }
+      tag_rename
+      expect(Trigger.find(object.id).condition['ticket.tags'][:value]).to eq('test1_renamed')
+    end
+
+    it 'trigger conditions with a tag list ' do
+      object = create :trigger, condition: { 'ticket.tags' => { operator: 'contains all', value: 'test1, test2, test3' } }
+      tag_rename
+      expect(Trigger.find(object.id).condition['ticket.tags'][:value]).to eq('test1_renamed, test2, test3')
+    end
+
+    it 'trigger performs with a single tag' do
+      object = create :trigger, perform: { 'ticket.tags' => { operator: 'contains one', value: 'test1' } }
+      tag_rename
+      expect(Trigger.find(object.id).perform['ticket.tags'][:value]).to eq('test1_renamed')
+    end
+
+    it 'trigger performs with a tag list ' do
+      object = create :trigger, perform: { 'ticket.tags' => { operator: 'contains all', value: 'test1, test2, test3' } }
+      tag_rename
+      expect(Trigger.find(object.id).perform['ticket.tags'][:value]).to eq('test1_renamed, test2, test3')
+    end
+
+    it 'scheduler conditions with a single tag' do
+      object = create :job, condition: { 'ticket.tags' => { operator: 'contains one', value: 'test1' } }
+      tag_rename
+      expect(Job.find(object.id).condition['ticket.tags'][:value]).to eq('test1_renamed')
+    end
+
+    it 'scheduler conditions with a tag list ' do
+      object = create :job, condition: { 'ticket.tags' => { operator: 'contains all', value: 'test1, test2, test3' } }
+      tag_rename
+      expect(Job.find(object.id).condition['ticket.tags'][:value]).to eq('test1_renamed, test2, test3')
+    end
+
+    it 'scheduler performs with a single tag' do
+      object = create :job, perform: { 'ticket.tags' => { operator: 'contains one', value: 'test1' } }
+      tag_rename
+      expect(Job.find(object.id).perform['ticket.tags'][:value]).to eq('test1_renamed')
+    end
+
+    it 'scheduler performs with a tag list ' do
+      object = create :job, perform: { 'ticket.tags' => { operator: 'contains all', value: 'test1, test2, test3' } }
+      tag_rename
+      expect(Job.find(object.id).perform['ticket.tags'][:value]).to eq('test1_renamed, test2, test3')
+    end
+
+    it 'PostmasterFilter performs with a single tag' do
+      object = create :postmaster_filter, perform: { 'x-zammad-ticket-tags' => { operator: 'contains one', value: 'test1' } }
+      tag_rename
+      expect(PostmasterFilter.find(object.id).perform['x-zammad-ticket-tags'][:value]).to eq('test1_renamed')
+    end
+
+    it 'PostmasterFilter performs with a tag list ' do
+      object = create :postmaster_filter, perform: { 'x-zammad-ticket-tags' => { operator: 'contains all', value: 'test1, test2, test3' } }
+      tag_rename
+      expect(PostmasterFilter.find(object.id).perform['x-zammad-ticket-tags'][:value]).to eq('test1_renamed, test2, test3')
+    end
+  end
+end