From 2bf4558b960d7f02673a677960762a7f45d08b61 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sat, 5 Oct 2013 14:56:03 +0200 Subject: [PATCH] Improved unit tests. --- app/models/application_model.rb | 28 +++ .../application_model/activity_stream_base.rb | 1 - app/models/ticket.rb | 10 +- app/models/ticket/article.rb | 9 +- test/unit/activity_stream_test.rb | 192 ++++++++++++++++-- test/unit/history_test.rb | 64 ++++-- 6 files changed, 261 insertions(+), 43 deletions(-) diff --git a/app/models/application_model.rb b/app/models/application_model.rb index 97743f156..c47c3542b 100644 --- a/app/models/application_model.rb +++ b/app/models/application_model.rb @@ -479,6 +479,7 @@ log object create activity stream, if configured - will be executed automaticall =end def activity_stream_create + return if !self.class.activity_stream_support_config activity_stream_log( 'created', self['created_by_id'] ) end @@ -492,7 +493,34 @@ log object update activity stream, if configured - will be executed automaticall =end def activity_stream_update + return if !self.class.activity_stream_support_config + return if !self.changed? + + # default ignored attributes + ignore_attributes = { + :created_at => true, + :updated_at => true, + :created_by_id => true, + :updated_by_id => true, + } + if self.class.activity_stream_support_config[:ignore_attributes] + self.class.activity_stream_support_config[:ignore_attributes].each {|key, value| + ignore_attributes[key] = value + } + end + + log = false + self.changes.each {|key, value| + + # do not log created_at and updated_at attributes + next if ignore_attributes[key.to_sym] == true + + log = true + } + + return if !log + activity_stream_log( 'updated', self['updated_by_id'] ) end diff --git a/app/models/application_model/activity_stream_base.rb b/app/models/application_model/activity_stream_base.rb index da6a415cf..8e7aaefe6 100644 --- a/app/models/application_model/activity_stream_base.rb +++ b/app/models/application_model/activity_stream_base.rb @@ -16,7 +16,6 @@ returns =end def activity_stream_log (type, user_id) - return if !self.class.activity_stream_support_config role = self.class.activity_stream_support_config[:role] ActivityStream.add( :o_id => self['id'], diff --git a/app/models/ticket.rb b/app/models/ticket.rb index fd92504f3..742917411 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -16,10 +16,16 @@ class Ticket < ApplicationModel after_update :notify_clients_after_update after_destroy :notify_clients_after_destroy - activity_stream_support + activity_stream_support :ignore_attributes => { + :create_article_type_id => true, + :create_article_sender_id => true, + :article_count => true, + } history_support :ignore_attributes => { - :article_count => true, + :create_article_type_id => true, + :create_article_sender_id => true, + :article_count => true, } belongs_to :group diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index c292e7111..698b00c49 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -14,11 +14,14 @@ class Ticket::Article < ApplicationModel after_update :notify_clients_after_update after_destroy :notify_clients_after_destroy - activity_stream_support + activity_stream_support :ignore_attributes => { + :ticket_article_type_id => true, + :ticket_article_sender_id => true, + } history_support :ignore_attributes => { - :create_article_type_id => true, - :create_article_sender_id => true, + :ticket_article_type_id => true, + :ticket_article_sender_id => true, } attr_accessor :attachments diff --git a/test/unit/activity_stream_test.rb b/test/unit/activity_stream_test.rb index 62940df4e..6d2bb0fe4 100644 --- a/test/unit/activity_stream_test.rb +++ b/test/unit/activity_stream_test.rb @@ -4,7 +4,7 @@ require 'test_helper' class ActivityStreamTest < ActiveSupport::TestCase role = Role.lookup( :name => 'Admin' ) group = Group.lookup( :name => 'Users' ) - user = User.create_or_update( + admin_user = User.create_or_update( :login => 'admin', :firstname => 'Bob', :lastname => 'Smith', @@ -16,6 +16,7 @@ class ActivityStreamTest < ActiveSupport::TestCase :updated_by_id => 1, :created_by_id => 1 ) + current_user = User.lookup( :login => 'nicole.braun@zammad.org' ) test 'ticket+user' do tests = [ @@ -25,17 +26,17 @@ class ActivityStreamTest < ActiveSupport::TestCase :create => { :ticket => { :group_id => Group.lookup( :name => 'Users' ).id, - :customer_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :customer_id => current_user.id, :owner_id => User.lookup( :login => '-' ).id, :title => 'Unit Test 1 (äöüß)!', :ticket_state_id => Ticket::State.lookup( :name => 'new' ).id, :ticket_priority_id => Ticket::Priority.lookup( :name => '2 normal' ).id, - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, }, :article => { - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, :ticket_article_type_id => Ticket::Article::Type.lookup( :name => 'phone' ).id, :ticket_article_sender_id => Ticket::Article::Sender.lookup( :name => 'Customer' ).id, :from => 'Unit Test ', @@ -52,17 +53,20 @@ class ActivityStreamTest < ActiveSupport::TestCase }, :check => [ { + :result => true, :object => 'Ticket', :type => 'created', }, { + :result => true, :object => 'Ticket::Article', :type => 'created', }, { + :result => false, :object => 'User', :type => 'updated', - :o_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :o_id => current_user.id, }, ] }, @@ -78,14 +82,14 @@ class ActivityStreamTest < ActiveSupport::TestCase ticket = Ticket.create( test[:create][:ticket] ) test[:check][0][:o_id] = ticket.id test[:check][0][:created_at] = ticket.created_at - test[:check][0][:created_by_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id + test[:check][0][:created_by_id] = current_user.id sleep 2 test[:create][:article][:ticket_id] = ticket.id article = Ticket::Article.create( test[:create][:article] ) test[:check][1][:o_id] = article.id test[:check][1][:created_at] = article.created_at - test[:check][1][:created_by_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id + test[:check][1][:created_by_id] = current_user.id assert_equal( ticket.class.to_s, 'Ticket' ) assert_equal( article.class.to_s, 'Ticket::Article' ) @@ -95,9 +99,9 @@ class ActivityStreamTest < ActiveSupport::TestCase ticket.update_attributes( test[:update][:ticket] ) # check updated user - test[:check][2][:o_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id + test[:check][2][:o_id] = current_user.id test[:check][2][:created_at] = ticket.created_at - test[:check][2][:created_by_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id + test[:check][2][:created_by_id] = current_user.id end if test[:update][:article] article.update_attributes( test[:update][:article] ) @@ -108,7 +112,7 @@ class ActivityStreamTest < ActiveSupport::TestCase tickets.push ticket # check activity_stream - activity_stream_check( user.activity_stream(3), test[:check] ) + activity_stream_check( admin_user.activity_stream(3), test[:check] ) } # delete tickets @@ -128,8 +132,8 @@ class ActivityStreamTest < ActiveSupport::TestCase :create => { :organization => { :name => 'some name', - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, }, }, :update1 => { @@ -144,10 +148,12 @@ class ActivityStreamTest < ActiveSupport::TestCase }, :check => [ { + :result => true, :object => 'Organization', :type => 'created', }, { + :result => true, :object => 'Organization', :type => 'updated', }, @@ -160,7 +166,7 @@ class ActivityStreamTest < ActiveSupport::TestCase organization = Organization.create( test[:create][:organization] ) test[:check][0][:o_id] = organization.id test[:check][0][:created_at] = organization.created_at - test[:check][0][:created_by_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id + test[:check][0][:created_by_id] = current_user.id sleep 11 assert_equal( organization.class.to_s, 'Organization' ) @@ -169,7 +175,7 @@ class ActivityStreamTest < ActiveSupport::TestCase organization.update_attributes( test[:update1][:organization] ) test[:check][1][:o_id] = organization.id test[:check][1][:updated_at] = organization.updated_at - test[:check][1][:created_by_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id + test[:check][1][:created_by_id] = current_user.id sleep 2 end @@ -181,7 +187,7 @@ class ActivityStreamTest < ActiveSupport::TestCase organizations.push organization # check activity_stream - activity_stream_check( user.activity_stream(2), test[:check] ) + activity_stream_check( admin_user.activity_stream(2), test[:check] ) } # delete tickets @@ -194,6 +200,152 @@ class ActivityStreamTest < ActiveSupport::TestCase end + test 'user with update check false' do + tests = [ + + # test 1 + { + :create => { + :user => { + :login => 'someemail@example.com', + :email => 'Bob Smith II ', + :updated_by_id => current_user.id, + :created_by_id => current_user.id, + }, + }, + :update1 => { + :user => { + :firstname => 'Bob U', + :lastname => 'Smith U', + }, + }, + :check => [ + { + :result => true, + :object => 'User', + :type => 'created', + }, + { + :result => false, + :object => 'User', + :type => 'updated', + }, + ] + }, + ] + users = [] + tests.each { |test| + + user = User.create( test[:create][:user] ) + test[:check][0][:o_id] = user.id + test[:check][0][:created_at] = user.created_at + test[:check][0][:created_by_id] = current_user.id + + assert_equal( user.class.to_s, 'User' ) + + if test[:update1][:user] + user.update_attributes( test[:update1][:user] ) + test[:check][1][:o_id] = user.id + test[:check][1][:updated_at] = user.updated_at + test[:check][1][:created_by_id] = current_user.id + end + + # remember organization + users.push user + + # check activity_stream + activity_stream_check( admin_user.activity_stream(2), test[:check] ) + } + + # delete tickets + users.each { |user| + user_id = user.id + user.destroy + found = User.where( :id => user_id ).first + assert( !found, "User destroyed") + } + end + + + + test 'user with update check true' do + tests = [ + + # test 1 + { + :create => { + :user => { + :login => 'someemail@example.com', + :email => 'Bob Smith II ', + :updated_by_id => current_user.id, + :created_by_id => current_user.id, + }, + }, + :update1 => { + :user => { + :firstname => 'Bob U', + :lastname => 'Smith U', + }, + }, + :update2 => { + :user => { + :firstname => 'Bob', + :lastname => 'Smith', }, + }, + :check => [ + { + :result => true, + :object => 'User', + :type => 'created', + }, + { + :result => true, + :object => 'User', + :type => 'updated', + }, + ] + }, + ] + users = [] + tests.each { |test| + + user = User.create( test[:create][:user] ) + test[:check][0][:o_id] = user.id + test[:check][0][:created_at] = user.created_at + test[:check][0][:created_by_id] = current_user.id + + assert_equal( user.class.to_s, 'User' ) + + if test[:update1][:user] + user.update_attributes( test[:update1][:user] ) + test[:check][1][:o_id] = user.id + test[:check][1][:updated_at] = user.updated_at + test[:check][1][:created_by_id] = current_user.id + end + + # to verify update which need to be logged + sleep 14 + + if test[:update2][:user] + user.update_attributes( test[:update2][:user] ) + end + + # remember organization + users.push user + + # check activity_stream + activity_stream_check( admin_user.activity_stream(2), test[:check] ) + } + + # delete tickets + users.each { |user| + user_id = user.id + user.destroy + found = User.where( :id => user_id ).first + assert( !found, "User destroyed") + } + end + def activity_stream_check( activity_stream_list, checks ) puts 'AS ' + activity_stream_list.inspect checks.each { |check_item| @@ -210,7 +362,11 @@ class ActivityStreamTest < ActiveSupport::TestCase next if item['o_id'] != check_item[:o_id] match = true } - assert( match, "activity stream check not matched! #{check_item.inspect}") + if check_item[:result] + assert( match, "activity stream check not matched! #{check_item.inspect}") + else + assert( !match, "activity stream check matched but should not! #{check_item.inspect}") + end } end diff --git a/test/unit/history_test.rb b/test/unit/history_test.rb index 84e42c823..62f3090c6 100644 --- a/test/unit/history_test.rb +++ b/test/unit/history_test.rb @@ -2,6 +2,8 @@ require 'test_helper' class HistoryTest < ActiveSupport::TestCase + current_user = User.lookup( :login => 'nicole.braun@zammad.org' ) + test 'ticket' do tests = [ @@ -10,17 +12,17 @@ class HistoryTest < ActiveSupport::TestCase :ticket_create => { :ticket => { :group_id => Group.lookup( :name => 'Users' ).id, - :customer_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :customer_id => current_user.id, :owner_id => User.lookup( :login => '-' ).id, :title => 'Unit Test 1 (äöüß)!', :ticket_state_id => Ticket::State.lookup( :name => 'new' ).id, :ticket_priority_id => Ticket::Priority.lookup( :name => '2 normal' ).id, - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, }, :article => { - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, :ticket_article_type_id => Ticket::Article::Type.lookup( :name => 'phone' ).id, :ticket_article_sender_id => Ticket::Article::Sender.lookup( :name => 'Customer' ).id, :from => 'Unit Test ', @@ -37,10 +39,12 @@ class HistoryTest < ActiveSupport::TestCase }, :history_check => [ { + :result => true, :history_object => 'Ticket', :history_type => 'created', }, { + :result => true, :history_object => 'Ticket', :history_type => 'updated', :history_attribute => 'title', @@ -48,6 +52,7 @@ class HistoryTest < ActiveSupport::TestCase :value_to => 'Unit Test 1 (äöüß) - update!', }, { + :result => true, :history_object => 'Ticket', :history_type => 'updated', :history_attribute => 'ticket_state', @@ -57,8 +62,14 @@ class HistoryTest < ActiveSupport::TestCase :id_to => Ticket::State.lookup( :name => 'open' ).id, }, { - :history_object => 'Ticket::Article', - :history_type => 'created', + :result => true, + :history_object => 'Ticket::Article', + :history_type => 'created', + }, + { + :result => false, + :history_object => 'User', + :history_type => 'updated', }, ] }, @@ -68,17 +79,17 @@ class HistoryTest < ActiveSupport::TestCase :ticket_create => { :ticket => { :group_id => Group.lookup( :name => 'Users' ).id, - :customer_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :customer_id => current_user.id, :owner_id => User.lookup( :login => '-' ).id, :title => 'Unit Test 2 (äöüß)!', :ticket_state_id => Ticket::State.lookup( :name => 'new' ).id, :ticket_priority_id => Ticket::Priority.lookup( :name => '2 normal' ).id, - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, }, :article => { - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :created_by_id => current_user.id, + :updated_by_id => current_user.id, :ticket_article_type_id => Ticket::Article::Type.lookup(:name => 'phone' ).id, :ticket_article_sender_id => Ticket::Article::Sender.lookup(:name => 'Customer' ).id, :from => 'Unit Test ', @@ -90,7 +101,7 @@ class HistoryTest < ActiveSupport::TestCase :ticket => { :title => 'Unit Test 2 (äöüß) - update!', :ticket_state_id => Ticket::State.lookup( :name => 'open' ).id, - :owner_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :owner_id => current_user.id, }, :article => { :from => 'Unit 2 Test 2 ', @@ -98,10 +109,12 @@ class HistoryTest < ActiveSupport::TestCase }, :history_check => [ { + :result => true, :history_object => 'Ticket', :history_type => 'created', }, { + :result => true, :history_object => 'Ticket', :history_type => 'updated', :history_attribute => 'title', @@ -109,19 +122,22 @@ class HistoryTest < ActiveSupport::TestCase :value_to => 'Unit Test 2 (äöüß) - update!', }, { + :result => true, :history_object => 'Ticket', :history_type => 'updated', :history_attribute => 'owner', :value_from => '-', :value_to => 'Nicole Braun', :id_from => User.lookup( :login => '-' ).id, - :id_to => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :id_to => current_user.id, }, { + :result => true, :history_object => 'Ticket::Article', :history_type => 'created', }, { + :result => true, :history_object => 'Ticket::Article', :history_type => 'updated', :history_attribute => 'from', @@ -186,8 +202,8 @@ class HistoryTest < ActiveSupport::TestCase :lastname => 'Smith', :email => 'somebody@example.com', :active => true, - :updated_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, - :created_by_id => User.lookup( :login => 'nicole.braun@zammad.org' ).id, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, }, }, :user_update => { @@ -199,10 +215,12 @@ class HistoryTest < ActiveSupport::TestCase }, :history_check => [ { + :result => true, :history_object => 'User', :history_type => 'created', }, { + :result => true, :history_object => 'User', :history_type => 'updated', :history_attribute => 'lastname', @@ -210,6 +228,7 @@ class HistoryTest < ActiveSupport::TestCase :value_to => 'Master', }, { + :result => true, :history_object => 'User', :history_type => 'updated', :history_attribute => 'email', @@ -217,6 +236,7 @@ class HistoryTest < ActiveSupport::TestCase :value_to => 'master@example.com', }, { + :result => true, :history_object => 'User', :history_type => 'updated', :history_attribute => 'active', @@ -270,8 +290,8 @@ class HistoryTest < ActiveSupport::TestCase :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, + :updated_by_id => current_user.id, + :created_by_id => current_user.id, }, }, :organization_update => { @@ -282,10 +302,12 @@ class HistoryTest < ActiveSupport::TestCase }, :history_check => [ { + :result => true, :history_object => 'Organization', :history_type => 'created', }, { + :result => true, :history_object => 'Organization', :history_type => 'updated', :history_attribute => 'name', @@ -365,7 +387,11 @@ class HistoryTest < ActiveSupport::TestCase assert_equal( check_item[:id_to], history_item['id_to'], "check history :id_to #{history_item['id_to']} ok") end } - assert( match, "history check not matched! #{check_item.inspect}") + if check_item[:result] + assert( match, "history check not matched! #{check_item.inspect}") + else + assert( !match, "history check matched but should not! #{check_item.inspect}") + end } end