Fixes #4604 - Log Trigger and Scheduler in Ticket History.

Co-authored-by: Dominik Klein <dk@zammad.com>
Co-authored-by: Dusan Vuckovic <dv@zammad.com>
Co-authored-by: Florian Liebe <fl@zammad.com>
Co-authored-by: Mantas Masalskis <mm@zammad.com>

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

@@ -10,6 +10,7 @@ class App.GenericHistory extends App.ControllerModal
 
   constructor: ->
     super
+    @reverse = false
     @fetch()
 
   content: =>
@@ -28,41 +29,62 @@ class App.GenericHistory extends App.ControllerModal
     @renderPopovers()
 
   sortorder: =>
-    @items = @items.reverse()
+    @reverse = !@reverse
     @update()
 
   T: (name) ->
     App.i18n.translateInline(name)
 
+  sourceableTypeDisplayName: (type) ->
+    {
+      'PostmasterFilter': __('Postmaster Filter'),
+      'Job': __('Scheduler'),
+    }[type] || type
+
   reworkItems: (items) ->
     newItems = []
     newItem = {}
+    lastSource = '_'
     lastUserId = undefined
     lastTime   = undefined
     items = clone(items)
     for item in items
-
       if item.object is 'Ticket::Article'
         item.object = 'Article'
       if item.object is 'Ticket::SharedDraftZoom'
         item.object = 'Draft'
 
-      data = item
-      data.created_by = App.User.find( item.created_by_id )
-
       currentItemTime = new Date( item.created_at )
       lastItemTime    = new Date( new Date( lastTime ).getTime() + (15 * 1000) )
 
       # start new section if user or time has changed
-      if lastUserId isnt item.created_by_id || currentItemTime > lastItemTime
-        lastTime   = item.created_at
-        lastUserId = item.created_by_id
+      if _.isEmpty(newItem) || currentItemTime > lastItemTime
+        lastTime = item.created_at
         if !_.isEmpty(newItem)
           newItems.push newItem
         newItem =
           created_at: item.created_at
           created_by: App.User.find( item.created_by_id )
-          records: []
+          sources: []
+
+      recordsSource = _.findWhere(newItem.sources, { sourceable_id: item.sourceable_id })
+      if !recordsSource
+        recordsSource = {
+          sourceable_id: item.sourceable_id,
+          sourceable_type: @sourceableTypeDisplayName(item.sourceable_type),
+          sourceable_name: item.sourceable_name,
+          users: []
+        }
+        newItem.sources.push(recordsSource)
+
+      recordsUser = _.findWhere(recordsSource.users, { id: item.created_by_id })
+      if !recordsUser
+        recordsUser = {
+          id: item.created_by_id,
+          object: App.User.find(item.created_by_id),
+          records: [],
+        }
+        recordsSource.users.push(recordsUser)
 
       # build content
       content = ''
@@ -79,7 +101,7 @@ class App.GenericHistory extends App.ControllerModal
           when 'escalation_warning'
             __("trigger '%s' was performed because ticket will escalate soon")
 
-        content = App.i18n.translateContent(message, item.value_to)
+        content = App.i18n.translateContent(message, item.sourceable_name)
       else if item.type is 'received_merge'
         ticket = App.Ticket.find( item.id_from )
         ticket_link = if ticket
@@ -118,11 +140,14 @@ class App.GenericHistory extends App.ControllerModal
         else if item.value_from
           content += " → '-'"
 
-      newItem.records.push content
+      recordsUser.records.push content
 
     if !_.isEmpty(newItem)
       newItems.push newItem
 
+    if @reverse
+      newItems = newItems.reverse()
+
     newItems
 
   translateItemValue: ({object, attribute}, value) ->

app/assets/javascripts/app/views/generic/history.jst.eco

@@ -4,14 +4,28 @@
 <hr>
 
 <% for item in @items: %>
-  <span class="user-popover" data-id="<%= item.created_by.id %>"><%= item.created_by.displayName() %></span> -
-  <%- @humanTime(item.created_at) %>
-  <ul>
-    <% for content in item.records: %>
-    <li><%- content %></li>
+  <div><%- @humanTime(item.created_at) %></div>
+  <div class="history-row-list">
+    <% for source in item.sources: %>
+      <% for source_user in source.users: %>
+        <span<% if !source.sourceable_name: %> class="user-popover" data-id="<%= source_user.id %>"<% end %>>
+        <% if source.sourceable_name: %>
+          <%= @T(source.sourceable_type) %>: <%= @T(source.sourceable_name) %>
+        <% else if source_user.id == 1: %>
+          <%- @T('System') %>
+        <% else: %>
+          <%= source_user.object.displayName() %>
+        <% end %>
+        </span>
+        <ul>
+          <% for content in source_user.records: %>
+          <li><%- content %></li>
+          <% end %>
+        </ul>
+     <% end %>
     <% end %>
-  </ul>
+  </div>
   <hr>
 <% end %>
 
-</div>
+</div>

app/assets/stylesheets/zammad.scss

@@ -14460,3 +14460,11 @@ span.is-disabled {
     }
   }
 }
+
+.history-row-list {
+  padding-left: 10px;
+
+  span {
+    text-decoration: underline;
+  }
+}

app/models/channel/email_parser.rb

@@ -270,7 +270,7 @@ returns
       # apply tags to ticket
       if mail[:'x-zammad-ticket-tags'].present?
         mail[:'x-zammad-ticket-tags'].each do |tag|
-          ticket.tag_add(tag)
+          ticket.tag_add(tag, sourceable: mail[:'x-zammad-ticket-tags-source'])
         end
       end
 
@@ -480,7 +480,7 @@ returns
             Rails.logger.info "set_attributes_by_x_headers assign #{item_object.class} #{key}=#{assoc_object.id}"
 
             item_object[key] = assoc_object.id
-
+            item_object.history_change_source_attribute(mail[:"#{header}-source"], key)
           end
         end
       end
@@ -490,10 +490,11 @@ returns
       if suffix
         header = "x-zammad-#{header_name}-#{suffix}-#{key}"
       end
-      if mail[header.to_sym]
-        Rails.logger.info "set_attributes_by_x_headers header #{header} found. Assign #{key}=#{mail[header.to_sym]}"
-        item_object[key] = mail[header.to_sym]
-      end
+      next if !mail[header.to_sym]
+
+      Rails.logger.info "set_attributes_by_x_headers header #{header} found. Assign #{key}=#{mail[header.to_sym]}"
+      item_object[key] = mail[header.to_sym]
+      item_object.history_change_source_attribute(mail[:"#{header}-source"], key)
     end
   end
 

app/models/channel/filter/database.rb

@@ -66,6 +66,7 @@ module Channel::Filter::Database
               next if mail[ 'x-zammad-ticket-tags'.downcase.to_sym ].include?(tag)
 
               mail[ 'x-zammad-ticket-tags'.downcase.to_sym ].push tag
+              mail[:'x-zammad-ticket-tags-source'] = filter
             end
           when 'remove'
             tags.each do |tag|
@@ -73,6 +74,7 @@ module Channel::Filter::Database
 
               tag.strip!
               mail[ 'x-zammad-ticket-tags'.downcase.to_sym ] -= [tag]
+              mail[:'x-zammad-ticket-tags-source'] = filter
             end
           end
           next
@@ -84,6 +86,7 @@ module Channel::Filter::Database
         end
 
         mail[ key.downcase.to_sym ] = meta['value']
+        mail[:"#{key.downcase}-source"] = filter
       end
     end
 

app/models/concerns/has_history.rb

@@ -6,11 +6,28 @@ module HasHistory
   included do
     attr_accessor :history_changes_last_done
 
-    after_create  :history_create
-    after_update  :history_update
+    after_create  :history_prefill, :history_create, :history_change_source_clear
+    after_update  :history_update, :history_change_source_clear
     after_destroy :history_destroy
   end
 
+  def history_prefill
+    return if @history_changes_source.blank?
+
+    @history_changes_source.each do |key, value|
+      next if !value.is_a?(PostmasterFilter)
+
+      attribute_name  = history_attribute_name(key)
+      attribute_value = history_attribute_changes(key, [nil, self[key]])
+
+      data = {
+        history_attribute: attribute_name,
+      }.merge(attribute_value)
+
+      history_log('set', created_by_id, data)
+    end
+  end
+
 =begin
 
 log object create history, if configured - will be executed automatically
@@ -59,51 +76,66 @@ log object update history with all updated attributes, if configured - will be e
       next if ignored_attributes.include?(key.to_sym)
 
       # get attribute name
-      attribute_name = key.to_s
-      if attribute_name[-3, 3] == '_id'
-        attribute_name = attribute_name[ 0, attribute_name.length - 3 ]
-      end
+      attribute_name  = history_attribute_name(key)
+      attribute_value = history_attribute_changes(key, value)
+
+      data = {
+        history_attribute: attribute_name,
+      }.merge(attribute_value)
+
+      # logger.info "HIST NEW #{self.class.to_s}.find(#{self.id}) #{data.inspect}"
+      history_log('updated', updated_by_id, data)
+    end
+  end
 
-      value_id = []
-      value_str = [ value[0], value[1] ]
-      if key.to_s[-3, 3] == '_id'
-        value_id[0] = value[0]
-        value_id[1] = value[1]
-
-        if respond_to?(attribute_name) && send(attribute_name)
-          relation_class = send(attribute_name).class
-          if relation_class && value_id[0]
-            relation_model = relation_class.lookup(id: value_id[0])
-            if relation_model
-              if relation_model['name']
-                value_str[0] = relation_model['name']
-              elsif relation_model.respond_to?(:fullname)
-                value_str[0] = relation_model.send(:fullname)
-              end
+  def history_attribute_name(key)
+    attribute_name = key.to_s
+    if attribute_name[-3, 3] == '_id'
+      attribute_name = attribute_name[ 0, attribute_name.length - 3 ]
+    end
+    attribute_name
+  end
+
+  def history_attribute_changes(key, value_changes)
+    attribute_name  = history_attribute_name(key)
+    value_id        = []
+    value_str       = [ value_changes[0], value_changes[1] ]
+
+    if key.to_s[-3, 3] == '_id'
+      value_id[0] = value_changes[0]
+      value_id[1] = value_changes[1]
+
+      if respond_to?(attribute_name) && send(attribute_name)
+        relation_class = send(attribute_name).class
+        if relation_class && value_id[0]
+          relation_model = relation_class.lookup(id: value_id[0])
+          if relation_model
+            if relation_model['name']
+              value_str[0] = relation_model['name']
+            elsif relation_model.respond_to?(:fullname)
+              value_str[0] = relation_model.send(:fullname)
             end
           end
-          if relation_class && value_id[1]
-            relation_model = relation_class.lookup(id: value_id[1])
-            if relation_model
-              if relation_model['name']
-                value_str[1] = relation_model['name']
-              elsif relation_model.respond_to?(:fullname)
-                value_str[1] = relation_model.send(:fullname)
-              end
+        end
+        if relation_class && value_id[1]
+          relation_model = relation_class.lookup(id: value_id[1])
+          if relation_model
+            if relation_model['name']
+              value_str[1] = relation_model['name']
+            elsif relation_model.respond_to?(:fullname)
+              value_str[1] = relation_model.send(:fullname)
             end
           end
         end
       end
-      data = {
-        history_attribute: attribute_name,
-        value_from:        value_str[0].to_s,
-        value_to:          value_str[1].to_s,
-        id_from:           value_id[0],
-        id_to:             value_id[1],
-      }
-      # logger.info "HIST NEW #{self.class.to_s}.find(#{self.id}) #{data.inspect}"
-      history_log('updated', updated_by_id, data)
     end
+
+    {
+      value_from: value_str[0].to_s,
+      value_to:   value_str[1].to_s,
+      id_from:    value_id[0],
+      id_to:      value_id[1],
+    }
   end
 
 =begin
@@ -145,6 +177,8 @@ returns
       created_at:             updated_at,
     ).merge!(history_log_attributes)
 
+    attributes[:sourceable] = @history_changes_source.try(:delete, attributes[:history_attribute]) || @history_changes_source.try(:delete, "#{attributes[:history_attribute]}_id") || @history_changes_source&.dig(type)
+
     History.add(attributes)
   end
 
@@ -237,6 +271,21 @@ returns
     @history_relation_object ||= self.class.instance_variable_get(:@history_relation_object) || []
   end
 
+  def history_change_source_clear
+    @history_changes_source = nil
+    @history_changes_source_last = nil
+  end
+
+  def history_change_source_attribute(source, attribute)
+    return if source.blank?
+    return if [Job, Trigger, PostmasterFilter].exclude?(source.class)
+    return if !source.persisted?
+
+    @history_changes_source ||= {}
+    @history_changes_source[attribute] = source
+    @history_changes_source_last       = source
+  end
+
   # methods defined here are going to extend the class, not the instance of it
   class_methods do
 =begin
@@ -269,6 +318,5 @@ end
       @history_relation_object ||= []
       @history_relation_object |= attributes
     end
-
   end
 end

app/models/concerns/has_tags.rb

@@ -16,12 +16,13 @@ add an tag to model
 
 =end
 
-  def tag_add(name, current_user_id = nil)
+  def tag_add(name, current_user_id = nil, sourceable: nil)
     Tag.tag_add(
       object:        self.class.to_s,
       o_id:          id,
       item:          name,
       created_by_id: current_user_id,
+      sourceable:    sourceable,
     )
   end
 
@@ -34,12 +35,13 @@ remove an tag of model
 
 =end
 
-  def tag_remove(name, current_user_id = nil)
+  def tag_remove(name, current_user_id = nil, sourceable: nil)
     Tag.tag_remove(
       object:        self.class.to_s,
       o_id:          id,
       item:          name,
       created_by_id: current_user_id,
+      sourceable:    sourceable,
     )
   end
 

app/models/history.rb

@@ -9,6 +9,7 @@ class History < ApplicationModel
   belongs_to :history_type,      class_name: 'History::Type', optional: true
   belongs_to :history_object,    class_name: 'History::Object', optional: true
   belongs_to :history_attribute, class_name: 'History::Attribute', optional: true
+  belongs_to :sourceable, polymorphic: true, optional: true
 
 =begin
 
@@ -60,6 +61,8 @@ add a new history entry for an object
       history_type_id:           history_type.id,
       history_object_id:         history_object.id,
       history_attribute_id:      history_attribute_id,
+      sourceable:                data[:sourceable],
+      sourceable_name:           data[:sourceable].try(:name),
       related_history_object_id: related_history_object_id,
       related_o_id:              data[:related_o_id],
       value_from:                data[:value_from],

app/models/tag.rb

@@ -41,6 +41,7 @@ add tags for certain object
       tag_item_id:   tag_item_id,
       o_id:          data[:o_id],
       created_by_id: data[:created_by_id],
+      sourceable:    data[:sourceable],
     )
 
     # touch reference
@@ -89,7 +90,10 @@ or by ids
       tag_item_id:   data[:tag_item_id],
       o_id:          data[:o_id],
     )
-    result.each(&:destroy)
+    result.each do |item|
+      item.sourceable = data[:sourceable]
+      item.destroy
+    end
 
     # touch reference
     touch_reference_by_params(data)

app/models/tag/writes_to_ticket_history.rb

@@ -7,6 +7,11 @@ module Tag::WritesToTicketHistory
   included do
     after_create  :write_tag_added_to_ticket_history
     after_destroy :write_tag_removed_to_ticket_history
+
+    # used to forward the sourceable to the tag model
+    # to keep track of added and removed tags by
+    # postmaster filters, triggers and schedulers
+    attr_accessor :sourceable
   end
 
   private
@@ -22,6 +27,7 @@ module Tag::WritesToTicketHistory
       history_attribute: 'tag',
       value_to:          tag_item.name,
       created_by_id:     created_by_id,
+      sourceable:        sourceable,
     )
   end
 
@@ -36,6 +42,7 @@ module Tag::WritesToTicketHistory
       history_attribute: 'tag',
       value_to:          tag_item.name,
       created_by_id:     created_by_id,
+      sourceable:        sourceable,
     )
   end
 end