Fix #2001 (Use non-bang gsub in NotificationFactory:Template)

lib/notification_factory/template.rb

@@ -18,42 +18,33 @@ examples how to use
 
   def to_s
     strip_html
-
-    @template
   end
 
   def strip_html
     # some browsers start adding HTML tags
     # fixes https://github.com/zammad/zammad/issues/385
-    @template.gsub!(/\#\{\s*t\((.+?)\)\s*\}/m) do
-      content = $1
-      if content =~ /^'(.+?)'$/
-        "<%= t \"#{strip_content($1)}\", #{@escape} %>"
+    @template.gsub(/\#\{\s*t\((.+?)\)\s*\}/m) do
+      if $1 =~ /^'(.+?)'$/
+        %(<%= t "#{strip_content($1)}", #{@escape} %>)
       else
-        "<%= t d\"#{strip_variable(content)}\", #{@escape} %>"
+        %(<%= t d"#{strip_variable(strip_content($1))}", #{@escape} %>)
       end
-    end
-    @template.gsub!(/\#\{\s*config\.(.+?)\s*\}/m) do
-      "<%= c \"#{strip_variable($1)}\", #{@escape} %>"
-    end
-    @template.gsub!(/\#\{(.*?)\}/m) do
-      "<%= d \"#{strip_variable($1)}\", #{@escape} %>"
+    end.gsub(/\#\{\s*config\.(.+?)\s*\}/m) do
+      %(<%= c "#{strip_variable($1)}", #{@escape} %>)
+    end.gsub(/\#\{(.*?)\}/m) do
+      %(<%= d "#{strip_variable($1)}", #{@escape} %>)
     end
   end
 
   def strip_content(string)
-    return string if !string
-    string.gsub!(/\t|\r|\n/, '')
-    string.gsub!(/"/, '\"')
-    string
+    string&.gsub(/\t|\r|\n/, '')
+          &.gsub(/"/, '\"')
   end
 
   def strip_variable(string)
-    return string if !string
-    string.gsub!(/\t|\r|\n|"|'|§|;/, '')
-    string.gsub!(/\s*/, '')
-    string.gsub!(/<.+?>/, '')
-    string
+    string&.gsub(/\t|\r|\n|"|'|§|;/, '')
+          &.gsub(/\s*/, '')
+          &.gsub(/<.+?>/, '')
   end
 
 end

spec/models/ticket_spec.rb

@@ -175,7 +175,7 @@ RSpec.describe Ticket do
 
   describe '#perform_changes' do
 
-    it 'performes a ticket state change on a ticket' do
+    it 'performs a ticket state change on a ticket' do
       source_ticket = create(:ticket)
 
       changes = {
@@ -188,7 +188,7 @@ RSpec.describe Ticket do
       expect(source_ticket.state.name).to eq('closed')
     end
 
-    it 'performes a ticket deletion on a ticket' do
+    it 'performs a ticket deletion on a ticket' do
       source_ticket = create(:ticket)
 
       changes = {
@@ -201,6 +201,25 @@ RSpec.describe Ticket do
       expect(ticket_with_source_ids).to match_array([])
     end
 
+    # Regression test for https://github.com/zammad/zammad/issues/2001
+    it 'does not modify its arguments' do
+      trigger = Trigger.new(
+        perform: {
+          # rubocop:disable Lint/InterpolationCheck
+          'notification.email' => {
+            body: 'Hello #{ticket.customer.firstname} #{ticket.customer.lastname},',
+            recipient: %w[article_last_sender ticket_owner ticket_customer ticket_agents],
+            subject: 'Autoclose (#{ticket.title})'
+          }
+          # rubocop:enable Lint/InterpolationCheck
+        }
+      )
+
+      expect { Ticket.first.perform_changes(trigger.perform, 'trigger', {}, 1) }
+        .to not_change { trigger.perform['notification.email'][:body] }
+        .and not_change { trigger.perform['notification.email'][:subject] }
+    end
+
   end
 
   describe '#selectors' do