Maintenance: Webhook custom payload - Improve notification usage.

app/assets/javascripts/app/controllers/_application_controller/_generic_index.coffee

@@ -174,14 +174,15 @@ class App.ControllerGenericIndex extends App.Controller
       return
 
     new constructor(
-      id:            item.id
-      pageData:      @pageData
-      genericObject: @genericObject
-      container:     @container
-      small:         @small
-      large:         @large
-      veryLarge:     @veryLarge
-      handlers:      @handlers
+      id:               item.id
+      pageData:         @pageData
+      genericObject:    @genericObject
+      container:        @container
+      small:            @small
+      large:            @large
+      veryLarge:        @veryLarge
+      handlers:         @handlers
+      validateOnSubmit: @validateOnSubmit
     )
 
   newControllerClass: ->

app/jobs/trigger_webhook_job/custom_payload.rb

@@ -20,6 +20,10 @@ class TriggerWebhookJob::CustomPayload
     User
   ].freeze
 
+  ALLOWED_NOTIFICATION_CLASSES = %w[
+    Struct::Notification
+  ].freeze
+
   ALLOWED_SIMPLE_CLASSES = %w[
     Integer
     String
@@ -35,7 +39,8 @@ class TriggerWebhookJob::CustomPayload
     ALLOWED_TICKET_CLASSES +
     ALLOWED_USER_CLASSES +
     ALLOWED_SIMPLE_CLASSES +
-    ALLOWED_RAILS_CLASSES
+    ALLOWED_RAILS_CLASSES +
+    ALLOWED_NOTIFICATION_CLASSES
 
   ALLOWED_TICKET_METHODS = %w[
     created_by
@@ -61,10 +66,19 @@ class TriggerWebhookJob::CustomPayload
     fullname
   ].freeze
 
+  ALLOWED_NOTIFICATION_METHODS = %w[
+    subject
+    link
+    message
+    body
+    changes
+  ].freeze
+
   ALLOWED_METHODS = {
     'Ticket'          => ALLOWED_TICKET_METHODS,
     'Ticket::Article' => ALLOWED_TICKET_ARTICLE_METHODS,
     'User'            => ALLOWED_USER_METHODS,
+    'Notification'    => ALLOWED_NOTIFICATION_METHODS,
   }.freeze
 
   DENIED_USER_ATTRIBUTES = %w[
@@ -87,7 +101,8 @@ class TriggerWebhookJob::CustomPayload
     return JSON.parse(record) if variables.blank?
 
     tracks.transform_keys!(&:to_sym)
-    mappings = parse(variables, tracks, event)
+    tracks[:notification] = TriggerWebhookJob::CustomPayload::Notification.generate(tracks, event)
+    mappings = parse(variables, tracks)
 
     # NeverShouldHappen(TM)
     return JSON.parse(record) if mappings.blank?
@@ -121,11 +136,9 @@ class TriggerWebhookJob::CustomPayload
     variables
   end
 
-  def self.parse(variables, tracks, event)
+  def self.parse(variables, tracks)
     mappings = {}
 
-    notification(variables, mappings, tracks, event)
-
     variables.each do |variable|
       methods = variable.split('.')
       object = methods.shift
@@ -140,13 +153,6 @@ class TriggerWebhookJob::CustomPayload
     mappings
   end
 
-  def self.notification(variables, mappings, tracks, event)
-    return if variables.exclude?('notification')
-
-    mappings['notification'] = TriggerWebhookJob::CustomPayload::Notification.generate(tracks, event)
-    variables.delete('notification')
-  end
-
   def self.validate_methods!(methods, reference, display)
     return "\#{#{display} / missing method}" if methods.blank?
 
@@ -202,6 +208,8 @@ class TriggerWebhookJob::CustomPayload
   end
 
   def self.allowed_subroutines(klass)
+    return ALLOWED_NOTIFICATION_METHODS if klass.to_s.in?(ALLOWED_NOTIFICATION_CLASSES)
+
     klass_attributes = klass.attribute_names - (DENIED_ATTRIBUTES[klass.to_s] || [])
     klass_methods    = ALLOWED_METHODS[klass.to_s] || []
 
@@ -210,7 +218,14 @@ class TriggerWebhookJob::CustomPayload
 
   def self.replace(record, mappings)
     mappings.each do |variable, value|
-      record.gsub!("\#{#{variable}}", value.to_s.gsub(%r{"}, '\"'))
+      record.gsub!("\#{#{variable}}", value
+      .to_s
+      .gsub(%r{"}, '\"')
+      .gsub(%r{\n}, '\n')
+      .gsub(%r{\r}, '\r')
+      .gsub(%r{\t}, '\t')
+      .gsub(%r{\f}, '\f')
+      .gsub(%r{\v}, '\v'))
     end
 
     record
@@ -223,7 +238,6 @@ class TriggerWebhookJob::CustomPayload
   private_class_method %i[
     allowed_class_method?
     allowed_subroutines
-    notification
     parse
     replace
     scan

app/jobs/trigger_webhook_job/custom_payload/notification.rb

@@ -1,6 +1,7 @@
 # Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
 module TriggerWebhookJob::CustomPayload::Notification
+  Notification = Struct.new('Notification', :subject, :message, :link, :changes, :body)
 
   def self.generate(tracks = {}, event)
     return '\#{bad event}' if event.exclude?(:execution)
@@ -8,8 +9,8 @@ module TriggerWebhookJob::CustomPayload::Notification
     type = type!(event)
     return '\#{bad type}' if type.blank?
 
-    notification = fetch(tracks, event, type)
-    sanitize(notification[:body].to_s)
+    template = fetch(tracks, event, type)
+    assemble(template, has_article: tracks[:article].present?)
   end
 
   # private class methods
@@ -35,21 +36,46 @@ module TriggerWebhookJob::CustomPayload::Notification
     nil
   end
 
-  # NOTE: Somewhere down in the code some weird white space replacement is
-  # done. This leads to improper (Ruby) JSON formatting. This hackish method
-  # works around this issue.
-  def self.sanitize(string)
-    string
-      .gsub(%r{\n}, '\n')
-      .gsub(%r{\r}, '\r')
-      .gsub(%r{\t}, '\t')
-      .gsub(%r{\f}, '\f')
-      .gsub(%r{\v}, '\v')
+  def self.assemble(template, has_article: false)
+    match = regex(has_article).match(template[:body])
+    notification = {
+      subject: template[:subject][2..],
+      message: match[:message].presence || '',
+      link:    match[:link].presence || '',
+      changes: match[:changes].presence || '',
+      body:    has_article ? match[:body].presence || '' : '',
+    }
+
+    sanitize(notification)
+    Notification.new(*notification.values)
+  end
+
+  def self.regex(extended)
+    source = '_<(?<link>.+)>:(?<message>.+)_\n(?<changes>.+)'
+    source = "#{source}\n{3,4}(?<body>.+)?" if extended
+
+    Regexp.new(source, Regexp::MULTILINE)
+  end
+
+  def self.sanitize(hash)
+    hash.each do |key, value|
+      if key.eql?(:changes)
+        value = value
+          .split(%r{\n})
+          .map(&:strip)
+          .compact_blank
+          .join('\n')
+      end
+
+      hash[key] = value.strip
+    end
   end
 
   private_class_method %i[
+    assemble
     fetch
-    type!
+    regex
     sanitize
+    type!
   ]
 end

app/views/messaging/ticket_info/cs.md.erb

@@ -1,11 +0,0 @@
-# #{ticket.title}
-_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Aktualizoval #{current_user.longname} #{ticket.updated_at}_
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
-
-<% if @objects[:article] %>
-#{article.body_as_text}
-<% end %>

app/views/messaging/ticket_info/de.md.erb

@@ -1,11 +0,0 @@
-# #{ticket.title}
-_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Aktualisiert von #{current_user.longname} um #{ticket.updated_at}_
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
-
-<% if @objects[:article] %>
-#{article.body_as_text}
-<% end %>

app/views/messaging/ticket_info/en.md.erb

@@ -1,10 +1,8 @@
 # #{ticket.title}
-_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Updated by #{current_user.longname} at #{ticket.updated_at}_
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
+_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Last updated at #{ticket.updated_at}_
+* Gruppe: #{ticket.group.name}
+* Besitzer: #{ticket.owner.fullname}
+* Status: #{t(ticket.state.name)}
 
 <% if @objects[:article] %>
 #{article.body_as_text}

app/views/messaging/ticket_info/es.md.erb

@@ -1,11 +0,0 @@
-# #{ticket.title}
-_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Tiquete##{ticket.number}>: Actualizado por #{current_user.longname} el #{ticket.updated_at}_
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
-
-<% if @objects[:article] %>
-#{article.body_as_text}
-<% end %>

app/views/messaging/ticket_info/fr.md.erb

@@ -1,11 +0,0 @@
-# #{ticket.title}
-_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Mis à jour par #{current_user.longname} à #{ticket.updated_at}_
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
-
-<% if @objects[:article] %>
-#{article.body_as_text}
-<% end %>

app/views/messaging/ticket_info/hr.md.erb

@@ -1,11 +0,0 @@
-# #{ticket.title}
-_<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Izmijenjeno od strane #{current_user.longname} u #{ticket.updated_at}_
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
-
-<% if @objects[:article] %>
-#{article.body_as_text}
-<% end %>

app/views/messaging/ticket_info/hu.md.erb

@@ -1,11 +0,0 @@
-# #{ticket.title}
-_<<#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}|Ticket##{ticket.number}>: Frissítette: #{current_user.longname} ekkor: #{ticket.updated_at}
-<% if @objects[:changes].present? %>
-  <% @objects[:changes].each do |key, value| %>
-  * <%= t key %>: <%= h value[0] %> -> <%= h value[1] %>
-  <% end %>
-<% end %>
-
-<% if @objects[:article] %>
-#{article.body_as_text}
-<% end %>