diff --git a/app/models/observer/ticket/notification.rb b/app/models/observer/ticket/notification.rb
index 98026c81d..61a5bda7d 100644
--- a/app/models/observer/ticket/notification.rb
+++ b/app/models/observer/ticket/notification.rb
@@ -33,13 +33,12 @@ class Observer::Ticket::Notification < ActiveRecord::Observer
:subject => 'New Ticket (#{ticket.title})',
:body => 'Hi #{recipient.firstname},
- a new Ticket (#{ticket.title}) via i18n(#{article.type.name}).
+ a new Ticket (#{ticket.title}) via i18n(#{article.type.name}) by #{ticket.updated_by.fullname}.
Group: #{ticket.group.name}
Owner: #{ticket.owner.firstname} #{ticket.owner.lastname}
State: i18n(#{ticket.state.name})
- From: #{article.from}
#{article.body}
@@ -68,12 +67,11 @@ class Observer::Ticket::Notification < ActiveRecord::Observer
:subject => 'Updated (#{ticket.title})',
:body => 'Hi #{recipient.firstname},
- updated (#{ticket.title}) via i18n(#{article.type.name}).
+ updated (#{ticket.title}) via i18n(#{article.type.name}) by #{ticket.updated_by.fullname}.
Changes:
' + changes + '
- From: #{article.from}
#{article.body}
diff --git a/app/models/ticket.rb b/app/models/ticket.rb
index b9d35ed37..7c7e809db 100644
--- a/app/models/ticket.rb
+++ b/app/models/ticket.rb
@@ -53,6 +53,7 @@ class Ticket < ApplicationModel
belongs_to :owner, :class_name => 'User'
belongs_to :customer, :class_name => 'User'
belongs_to :created_by, :class_name => 'User'
+ belongs_to :updated_by, :class_name => 'User'
belongs_to :create_article_type, :class_name => 'Ticket::Article::Type'
belongs_to :create_article_sender, :class_name => 'Ticket::Article::Sender'
diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb
index ee55a33d6..cf22c08e3 100644
--- a/app/models/ticket/article.rb
+++ b/app/models/ticket/article.rb
@@ -12,6 +12,7 @@ class Ticket::Article < ApplicationModel
belongs_to :type, :class_name => 'Ticket::Article::Type'
belongs_to :sender, :class_name => 'Ticket::Article::Sender'
belongs_to :created_by, :class_name => 'User'
+ belongs_to :updated_by, :class_name => 'User'
after_create :notify_clients_after_create
after_update :notify_clients_after_update
after_destroy :notify_clients_after_destroy
diff --git a/lib/notification_factory.rb b/lib/notification_factory.rb
index 38e15c8ea..9530fc08b 100644
--- a/lib/notification_factory.rb
+++ b/lib/notification_factory.rb
@@ -15,61 +15,70 @@ module NotificationFactory
def self.build(data)
- data[:string].gsub!( / \#\{ \s* ( .+? ) \s* \} /x ) { |placeholder|
+ data[:string].gsub!( / \#\{ \s* ( .+? ) \s* \} /xm ) { |placeholder|
# store possible callback to work with
# and check if it's valid for execution
- original_string = $&
- callback = $1
- callback_valid = nil
+ original_string = $&
+ callback = $1
- # use config params
- callback.gsub!( /\A config\.( [\w\.]+ ) \z/x ) { |item|
- callback_valid = true
- name = $1
- item = "Setting.get('#{name}')"
- }
-
- # use object params
- callback.gsub!( /\A ( [\w]+ )( \.[\w\.]+ ) \z/x ) { |item|
+ object_name = nil
+ object_method = nil
+ if callback =~ /\A ( [\w]+ )\.( [\w\.]+ ) \z/x
object_name = $1
object_method = $2
-
- if data[:objects][object_name.to_sym]
- callback_valid = true
- item = "data[:objects]['#{object_name}'.to_sym]#{object_method}"
- else
- item = item
- end
- }
-
- # use quoted text
- callback.gsub!( /\A ( data\[:objects\]\['article'\.to_sym\] ) \.body \z/x ) { |item|
- callback_valid = true
- if data[:objects][:article].content_type == 'text/html'
- item = item + '.html2text.message_quote.chomp'
- else
- item = item + '.word_wrap( :line_width => 82 ).message_quote.chomp'
- end
- }
+ end
# do validaton, ignore some methodes
- callback.gsub!( /\.(save|destroy|delete|remove|drop|update|create\(|new|all|where|find)/ix ) { |item|
- callback_valid = false
- }
+ if callback =~ /(`|\.(|\s*)(save|destroy|delete|remove|drop|update\(|update_att|create\(|new|all|where|find))/i
+ placeholder = "#{original_string} (not allowed)"
- if callback_valid
- # replace value
- begin
- placeholder = eval callback
- rescue Exception => e
- Rails.logger.error "Evaluation error caused by callback '#{callback}'"
- Rails.logger.error e.inspect
+ # get value based on object_name and object_method
+ elsif object_name && object_method
+
+ # use config params
+ if object_name == 'config'
+ placeholder = Setting.get(object_method)
+
+ # if object_name dosn't exist
+ elsif !data[:objects][object_name.to_sym]
+ placeholder = "\#{#{object_name} / no such object}"
+ else
+ value = nil
+ object_refs = data[:objects][object_name.to_sym]
+ object_methods = object_method.split('.')
+ object_methods_s = ''
+ object_methods.each {|method|
+ if object_methods_s != ''
+ object_methods_s += '.'
+ end
+ object_methods_s += method
+
+ # if method exists
+ if !object_refs.respond_to?( method.to_sym )
+ value = "\#{#{object_name}.#{object_methods_s} / no such method}"
+ break
+ end
+ object_refs = object_refs.send( method.to_sym )
+
+ # add body quote
+ if object_name == 'article' && method == 'body'
+ if data[:objects][:article].content_type == 'text/html'
+ object_refs = object_refs.html2text.message_quote.chomp
+ else
+ object_refs = object_refs.word_wrap( :line_width => 82 ).message_quote.chomp
+ end
+ end
+ }
+ if !value
+ placeholder = object_refs
+ else
+ placeholder = value
+ end
end
- else
- placeholder = original_string
end
+ placeholder
}
# translate
diff --git a/test/unit/notification_factory_test.rb b/test/unit/notification_factory_test.rb
index 0d86e789b..632e2ada3 100644
--- a/test/unit/notification_factory_test.rb
+++ b/test/unit/notification_factory_test.rb
@@ -9,8 +9,8 @@ class NotificationFactoryTest < ActiveSupport::TestCase
:customer_id => 2,
:state => Ticket::State.lookup( :name => 'new' ),
:priority => Ticket::Priority.lookup( :name => '2 normal' ),
- :updated_by_id => 1,
- :created_by_id => 1,
+ :updated_by_id => 2,
+ :created_by_id => 2,
)
article_plain = Ticket::Article.create(
:ticket_id => ticket.id,
@@ -53,6 +53,31 @@ class NotificationFactoryTest < ActiveSupport::TestCase
:string => '\'i18n(#{ticket.state.name})\' ticket state',
:result => '\'neu\' ticket state',
},
+ {
+ :locale => 'de',
+ :string => 'a #{not_existing_object.test}',
+ :result => 'a #{not_existing_object / no such object}',
+ },
+ {
+ :locale => 'de',
+ :string => 'a #{ticket.level1}',
+ :result => 'a #{ticket.level1 / no such method}',
+ },
+ {
+ :locale => 'de',
+ :string => 'a #{ticket.level1.level2}',
+ :result => 'a #{ticket.level1 / no such method}',
+ },
+ {
+ :locale => 'de',
+ :string => 'a #{ticket.title.level2}',
+ :result => 'a #{ticket.title.level2 / no such method}',
+ },
+ {
+ :locale => 'de',
+ :string => 'by #{ticket.updated_by.fullname}',
+ :result => 'by Nicole Braun',
+ },
{
:locale => 'de',
:string => 'Subject #{article.from}, Group: #{ticket.group.name}',
@@ -66,7 +91,7 @@ class NotificationFactoryTest < ActiveSupport::TestCase
{
:locale => 'de',
:string => '\#{puts `ls`}',
- :result => '\#{puts `ls`}',
+ :result => '\#{puts `ls`} (not allowed)',
},
]
tests.each { |test|
@@ -165,44 +190,44 @@ class NotificationFactoryTest < ActiveSupport::TestCase
{
:locale => 'de',
:string => '\#{puts `ls`}',
- :result => '\#{puts `ls`}',
+ :result => '\#{puts `ls`} (not allowed)',
},
{
:locale => 'de',
:string => 'attack#1 #{article.destroy}',
- :result => 'attack#1 #{article.destroy}',
+ :result => 'attack#1 #{article.destroy} (not allowed)',
},
{
:locale => 'de',
:string => 'attack#2 #{Article.where}',
- :result => 'attack#2 #{Article.where}',
+ :result => 'attack#2 #{Article.where} (not allowed)',
},
{
:locale => 'de',
:string => 'attack#1 #{article.
destroy}',
:result => 'attack#1 #{article.
- destroy}',
+ destroy} (not allowed)',
},
{
:locale => 'de',
:string => 'attack#1 #{article.find}',
- :result => 'attack#1 #{article.find}',
+ :result => 'attack#1 #{article.find} (not allowed)',
},
{
:locale => 'de',
- :string => 'attack#1 #{article.update}',
- :result => 'attack#1 #{article.update}',
+ :string => 'attack#1 #{article.update(:name => "test")}',
+ :result => 'attack#1 #{article.update(:name => "test")} (not allowed)',
},
{
:locale => 'de',
:string => 'attack#1 #{article.all}',
- :result => 'attack#1 #{article.all}',
+ :result => 'attack#1 #{article.all} (not allowed)',
},
{
:locale => 'de',
:string => 'attack#1 #{article.delete}',
- :result => 'attack#1 #{article.delete}',
+ :result => 'attack#1 #{article.delete} (not allowed)',
},
]
tests.each { |test|