Moved to new model based history methodes.

app/assets/stylesheets/zzz.css

@@ -724,7 +724,7 @@ footer {
 
 .ticket-article {
   padding: 8px 0 6px 2px;
-  min-height: 110px;
+  min-height: 116px;
   margin: 2px 0;
 }
 .ticket-article-item {

app/controllers/tickets_controller.rb

@@ -130,7 +130,7 @@ class TicketsController < ApplicationController
     return if !ticket_permission( ticket )
 
     # get history of ticket
-    history = History.list( 'Ticket', params[:id], 'Ticket::Article' )
+    history = ticket.history_get
 
     # get related assets
     assets = ticket.assets({})

app/models/application_model.rb

@@ -1,12 +1,15 @@
 # Copyright (C) 2013-2013 Zammad Foundation, http://zammad-foundation.org/
 
 class ApplicationModel < ActiveRecord::Base
+  include ApplicationModel::HistoryLogBase
+
   self.abstract_class = true
 
   before_create  :check_attributes_protected, :cache_delete, :fill_up_user_create
   before_create  :cache_delete, :fill_up_user_create
   before_update  :cache_delete_before, :fill_up_user_update
-  before_destroy :cache_delete_before
+  before_destroy :cache_delete_before, :destroy_dependencies
+
   after_create  :cache_delete
   after_update  :cache_delete
   after_destroy :cache_delete
@@ -402,4 +405,19 @@ class OwnModel < ApplicationModel
       :data => { :id => self.id, :updated_at => self.updated_at }
     )
   end
+
+  private
+
+=begin
+
+destory object dependencies, will be executed automatically
+
+=end
+
+  def destroy_dependencies
+
+    # delete history
+    History.remove( self.class.to_s, self.id )
+  end
+
 end

app/models/application_model/history_log_base.rb

@@ -0,0 +1,64 @@
+# Copyright (C) 2012-2013 Zammad Foundation, http://zammad-foundation.org/
+
+module ApplicationModel::HistoryLogBase
+
+=begin
+
+create history entry for this object
+
+  organization = Organization.find(123)
+  result = organization.history_create( 'created', user_id )
+
+returns
+
+  result = true # false
+
+=end
+
+  def history_create (type, user_id, data = {})
+    data[:o_id]                   = self['id']
+    data[:history_type]           = type
+    data[:history_object]         = self.class.name
+    data[:related_o_id]           = nil
+    data[:related_history_object] = nil
+    data[:created_by_id]          = user_id
+    History.add(data)
+  end
+
+=begin
+
+get history log for this object
+
+  organization = Organization.find(123)
+  result = organization.history_get()
+
+returns
+
+  result = [
+    {
+      :history_type      => 'created',
+      :history_object    => 'Organization',
+      :created_by_id     => 3,
+      :created_at        => "2013-08-19 20:41:33",
+    },
+    {
+      :history_type      => 'updated',
+      :history_object    => 'Organization',
+      :history_attribute => 'note',
+      :o_id              => 1,
+      :id_to             => nil,
+      :id_from           => nil,
+      :value_from        => "some note",
+      :value_to          => "some other note",
+      :created_by_id     => 3,
+      :created_at        => "2013-08-19 20:41:33",
+    },
+  ]
+
+=end
+
+  def history_get
+    History.list( self.class.name, self['id'] )
+  end
+
+end

app/models/history.rb

@@ -267,6 +267,17 @@ return all histoy entries of an object
     return history_attribute
   end
 
+  private
+
+=begin
+
+nothing to do on destroying history entries 
+
+=end
+
+  def destroy_dependencies
+  end
+
   class Object < ApplicationModel
   end
 

app/models/observer/history.rb

@@ -3,7 +3,7 @@
 require 'history'
 
 class Observer::History < ActiveRecord::Observer
-  observe :ticket, :user, 'ticket::_article'
+  observe :ticket, 'ticket::_article', :user, :organization, :group
 
   def after_create(record)
 
@@ -13,22 +13,17 @@ class Observer::History < ActiveRecord::Observer
     puts "HISTORY OBSERVER, object created #{ record.class.name }.find(#{ record.id })"
     #    puts record.inspect
 
-    # if Ticket::Article has changed, remember ticket to be able
+    user_id = record.created_by_id || UserInfo.current_user_id || 1
-    # to show article changes in ticket history
+
-    related_o_id              = nil
+    # log activity stream
-    related_history_object_id = nil
+    if record.respond_to?('history_create')
-    if record.class.name == 'Ticket::Article'
+      record.history_create( 'created', user_id )
-      related_o_id           = record.ticket_id
+    end
-      related_history_object = 'Ticket'
+
+    # log activity stream
+    if record.respond_to?('activity_stream')
+      record.activity_stream( 'created', user_id )
     end
-    History.add(
-      :o_id                   => record.id,
-      :history_type           => 'created',
-      :history_object         => record.class.name,
-      :related_o_id           => related_o_id,
-      :related_history_object => related_history_object,
-      :created_by_id          => record.created_by_id || UserInfo.current_user_id || 1
-    )
   end
 
   def before_update(record)
@@ -83,6 +78,8 @@ class Observer::History < ActiveRecord::Observer
       :create_article_sender_id => true,
     }
 
+    user_id = record.created_by_id || UserInfo.current_user_id || 1
+    history_logged = false
     diff.each do |key, value_ids|
 
       # do not log created_at and updated_at attributes
@@ -137,28 +134,26 @@ class Observer::History < ActiveRecord::Observer
         attribute_name = attribute_name.scan(/^(.*)_id$/).first.first
       end
 
-      # if Ticket::Article has changed, remember ticket to be able
+      history_logged = true
-      # to show article changes in ticket history
+
-      related_o_id              = nil
+      data = {
-      related_history_object_id = nil
-      if record.class.name == 'Ticket::Article'
-        related_o_id              = record.ticket_id
-        related_history_object_id = 'Ticket'
-      end
-      History.add(
-        :o_id                   => current.id,
-        :history_type           => 'updated',
-        :history_object         => record.class.name,
         :history_attribute      => attribute_name,
-        :related_o_id           => related_o_id,
-        :related_history_object => related_history_object_id,
         :value_from             => value[0],
         :value_to               => value[1],
         :id_from                => value_ids[0],
         :id_to                  => value_ids[1],
-        :created_by_id          => record['updated_by_id'] || UserInfo.current_user_id || 1
+      }
-      )
+      # log activity stream
+      if record.respond_to?('history_create')
+        record.history_create( 'updated', user_id, data )
+      end
+    end
 
+    # log activity stream
+    if history_logged
+      if record.respond_to?('activity_stream')
+        record.activity_stream( 'updated', user_id )
+      end
     end
   end
 

app/models/ticket.rb

@@ -23,6 +23,7 @@ class Ticket < ApplicationModel
   include Ticket::Subject
   include Ticket::Permission
   include Ticket::Assets
+  include Ticket::HistoryLog
   extend Ticket::Search
 
   attr_accessor :callback_loop
@@ -118,7 +119,7 @@ returns
   def destroy_dependencies
 
     # delete history
-    History.remove( 'Ticket', self.id )
+    History.remove( self.class.to_s, self.id )
 
     # delete articles
     self.articles.destroy_all

app/models/ticket/article.rb

@@ -2,6 +2,7 @@
 
 class Ticket::Article < ApplicationModel
   include Ticket::Article::Assets
+  include Ticket::Article::HistoryLog
 
   after_create  :attachment_check
   belongs_to    :ticket

app/models/ticket/article/history_log.rb

@@ -0,0 +1,67 @@
+# Copyright (C) 2012-2013 Zammad Foundation, httpdata[://zammad-foundation.org/
+
+module Ticket::Article::HistoryLog
+
+=begin
+
+create log activity for this article
+
+  article = Ticket::Article.find(123)
+  result = article.history_create( 'created', user_id )
+
+returns
+
+  result = true # false
+
+=end
+
+  def history_create (type, user_id, data = {})
+
+    # if Ticketdata[:data[:Article has changed, remember related ticket to be able
+    # to show article changes in ticket history
+    data[:o_id]                   = self['id']
+    data[:history_type]           = type
+    data[:history_object]         = self.class.name
+    data[:related_o_id]           = self['ticket_id']
+    data[:related_history_object] = 'Ticket'
+    data[:created_by_id]          = user_id
+    History.add(data)
+  end
+
+=begin
+
+get log activity for this article
+
+  article = Ticket::Article.find(123)
+  result = article.history_get()
+
+returns
+
+  result = [
+    {
+      :history_type      => 'created',
+      :history_object    => 'Ticket::Article',
+      :created_by_id     => 3,
+      :created_at        => "2013-08-19 20:41:33",
+    },
+    {
+      :history_type      => 'updated',
+      :history_object    => 'Ticket::Article',
+      :history_attribute => 'from',
+      :o_id              => 1,
+      :id_to             => nil,
+      :id_from           => nil,
+      :value_from        => "Some Body",
+      :value_to          => "Some Body Else",
+      :created_by_id     => 3,
+      :created_at        => "2013-08-19 20:41:33",
+    },
+  ]
+
+=end
+
+  def history_get
+    History.list( self.class.name, self['id'] )
+  end
+
+end

app/models/ticket/escalation.rb

@@ -212,7 +212,7 @@ def escalation_calculation_get_sla
       total_time_without_pending = 0
       total_time = 0
       #get history for ticket
-      history_list = History.list( 'Ticket', self.id )
+      history_list = self.history_get
 
       #loop through hist. changes and get time
       last_state            = nil

app/models/ticket/history_log.rb

@@ -0,0 +1,65 @@
+# Copyright (C) 2012-2013 Zammad Foundation, http://zammad-foundation.org/
+
+module Ticket::HistoryLog
+
+=begin
+
+create log activity for this ticket
+
+  ticket = Ticket.find(123)
+  result = ticket.history_create( 'created', user_id )
+
+returns
+
+  result = true # false
+
+=end
+
+  def history_create (type, user_id, data = {})
+
+    data[:o_id]                   = self['id']
+    data[:history_type]           = type
+    data[:history_object]         = self.class.name
+    data[:related_o_id]           = nil
+    data[:related_history_object] = nil
+    data[:created_by_id]          = user_id
+    History.add(data)
+  end
+
+=begin
+
+get log activity for this ticket
+
+  ticket = Ticket.find(123)
+  result = ticket.history_get()
+
+returns
+
+  result = [
+    {
+      :history_type      => 'created',
+      :history_object    => 'Ticket',
+      :created_by_id     => 3,
+      :created_at        => "2013-08-19 20:41:33",
+    },
+    {
+      :history_type      => 'updated',
+      :history_object    => 'Ticket',
+      :history_attribute => 'ticket_priority',
+      :o_id              => 1,
+      :id_to             => 3,
+      :id_from           => 2,
+      :value_from        => "low",
+      :value_to          => "high",
+      :created_by_id     => 3,
+      :created_at        => "2013-08-19 20:41:33",
+    },
+  ]
+
+=end
+
+  def history_get
+    History.list( self.class.name, self['id'], 'Ticket::Article' )
+  end
+
+end

test/unit/history_test.rb

@@ -161,10 +161,160 @@ class HistoryTest < ActiveSupport::TestCase
       # remember ticket
       tickets.push ticket
 
-      # get history
+      # check history
-      history_list = History.list( 'Ticket', ticket.id, 'Ticket::Article' )
+      history_check( ticket.history_get, test[:history_check] )
+    }
+
+    # delete tickets
+    tickets.each { |ticket|
+      ticket_id = ticket.id
+      ticket.destroy
+      found = Ticket.where( :id => ticket_id ).first
+      assert( !found, "Ticket destroyed")
+    }
+  end
+
+  test 'user' do
+    tests = [
+
+      # test 1
+      {
+        :user_create => {
+          :user => {
+            :login              => 'some_login_test',
+            :firstname          => 'Bob',
+            :lastname           => 'Smith',
+            :email              => 'somebody@example.com',
+            :updated_by_id      => User.lookup( :login => 'nicole.braun@zammad.org' ).id,
+            :created_by_id      => User.lookup( :login => 'nicole.braun@zammad.org' ).id,
+          },
+        },
+        :user_update => {
+          :user => {
+            :firstname          => 'Bob',
+            :lastname           => 'Master',
+            :email              => 'master@example.com',          },
+        },
+        :history_check => [
+          {
+            :history_object => 'User',
+            :history_type   => 'created',
+          },
+          {
+            :history_object    => 'User',
+            :history_type      => 'updated',
+            :history_attribute => 'lastname',
+            :value_from        => 'Smith',
+            :value_to          => 'Master',
+          },
+        ],
+      },
+
+    ]
+    users = []
+    tests.each { |test|
+
+      user = nil
+
+      # user transaction
+      ActiveRecord::Base.transaction do
+        user = User.create( test[:user_create][:user])
+
+        assert_equal( user.class.to_s, 'User' )
+
+        # update user
+        if test[:user_update][:user]
+          user.update_attributes( test[:user_update][:user] )
+        end
+      end
+
+      # remember user
+      users.push user
+
+      # check history
+      history_check( user.history_get, test[:history_check] )
+    }
+
+    # delete user
+    users.each { |user|
+      user_id = user.id
+      user.destroy
+      found = User.where( :id => user_id ).first
+      assert( !found, "User destroyed")
+    }
+  end
+
+  test 'organization' do
+    tests = [
+
+      # test 1
+      {
+        :organization_create => {
+          :organization => {
+            :name               => 'Org äöüß',
+            :note               => 'some note',
+            :updated_by_id      => User.lookup( :login => 'nicole.braun@zammad.org' ).id,
+            :created_by_id      => User.lookup( :login => 'nicole.braun@zammad.org' ).id,
+          },
+        },
+        :organization_update => {
+          :organization => {
+            :name               => 'Org 123',
+            :note               => 'some note',
+          },
+        },
+        :history_check => [
+          {
+            :history_object => 'Organization',
+            :history_type   => 'created',
+          },
+          {
+            :history_object    => 'Organization',
+            :history_type      => 'updated',
+            :history_attribute => 'name',
+            :value_from        => 'Org äöüß',
+            :value_to          => 'Org 123',
+          },
+        ],
+      },
+    ]
+    organizations = []
+    tests.each { |test|
+
+      organization = nil
+
+      # user transaction
+      ActiveRecord::Base.transaction do
+        organization = Organization.create( test[:organization_create][:organization])
+
+        assert_equal( organization.class.to_s, 'Organization' )
+
+        # update organization
+        if test[:organization_update][:organization]
+          organization.update_attributes( test[:organization_update][:organization] )
+        end
+      end
+
+      # remember user
+      organizations.push organization
+
+      # check history
+      history_check( organization.history_get, test[:history_check] )
+    }
+
+    # delete user
+    organizations.each { |organization|
+      organization_id = organization.id
+      organization.destroy
+      found = Organization.where( :id => organization_id ).first
+      assert( !found, "Organization destroyed")
+    }
+  end
+
+
+  def history_check( history_list, history_check )
 #      puts history_list.inspect
-      test[:history_check].each { |check_item|
+    history_check.each { |check_item|
 #        puts '+++++++++++'
 #        puts check_item.inspect
       match = false
@@ -172,6 +322,7 @@ class HistoryTest < ActiveSupport::TestCase
         next if match
 #          puts '--------'
 #          puts history_item.inspect
+#          puts history_item.history_object.name
         next if history_item.history_object.name != check_item[:history_object]
         next if history_item.history_type.name != check_item[:history_type]
         if check_item[:history_attribute]
@@ -199,14 +350,6 @@ class HistoryTest < ActiveSupport::TestCase
       }
       assert( match, "history check not matched! #{check_item.inspect}")
     }
-    }
+  end
 
-    # delete tickets
-    tickets.each { |ticket|
-      ticket_id = ticket.id
-      ticket.destroy
-      found = Ticket.where( :id => ticket_id ).first
-      assert( !found, "Ticket destroyed")
-    }
-  end
 end