From c03d1580f00cbf0eaf958024afdc6a9f2d1a5151 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sat, 3 Jan 2015 23:53:07 +0100 Subject: [PATCH] Moved to html notifications. --- .../controllers/_profile/language.js.coffee | 5 +- app/controllers/tickets_controller.rb | 2 +- app/controllers/users_controller.rb | 2 +- app/models/observer/ticket/notification.rb | 100 +-------- .../ticket/notification/background_job.rb | 208 +++++++++++++++++- app/models/user.rb | 3 +- db/migrate/20120101000001_create_base.rb | 1 - lib/core_ext/string.rb | 13 ++ lib/notification_factory.rb | 9 +- test/unit/ticket_notification_test.rb | 34 +-- 10 files changed, 246 insertions(+), 131 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_profile/language.js.coffee b/app/assets/javascripts/app/controllers/_profile/language.js.coffee index 3cc8ede1c..f46b07df2 100644 --- a/app/assets/javascripts/app/controllers/_profile/language.js.coffee +++ b/app/assets/javascripts/app/controllers/_profile/language.js.coffee @@ -12,7 +12,7 @@ class Index extends App.Controller html = $( App.view('profile/language')() ) configure_attributes = [ - { name: 'locale', display: '', tag: 'select', null: false, class: 'input span4', options: { de: 'Deutsch', en: 'English (United States)', 'en-CA': 'English (Canada)', 'en-GB': 'English (United Kingdom)' }, default: App.i18n.get() }, + { name: 'locale', display: '', tag: 'select', null: false, class: 'input', options: { de: 'Deutsch', en: 'English (United States)', 'en-CA': 'English (Canada)', 'en-GB': 'English (United Kingdom)' }, default: App.i18n.get() }, ] @form = new App.ControllerForm( @@ -67,5 +67,4 @@ class Index extends App.Controller msg: App.i18n.translateContent( data.message ) ) -App.Config.set( 'Language', { prio: 1000, name: 'Language', parent: '#profile', target: '#profile/language', controller: Index }, 'NavBarProfile' ) - +App.Config.set( 'Language', { prio: 1000, name: 'Language', parent: '#profile', target: '#profile/language', controller: Index }, 'NavBarProfile' ) \ No newline at end of file diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 88fb57941..696181dbd 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -235,7 +235,7 @@ class TicketsController < ApplicationController # replace tags signature['body'] = NotificationFactory.build( - :locale => current_user.locale, + :locale => current_user.preferences[:locale], :string => signature['body'], :objects => { :ticket => ticket, diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8d7e7ac87..085093302 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -156,7 +156,7 @@ class UsersController < ApplicationController # prepare subject & body [:subject, :body].each { |key| data[key.to_sym] = NotificationFactory.build( - :locale => user.locale, + :locale => user.preferences[:locale], :string => data[key.to_sym], :objects => { :token => token, diff --git a/app/models/observer/ticket/notification.rb b/app/models/observer/ticket/notification.rb index 15de17895..3c9ab26e5 100644 --- a/app/models/observer/ticket/notification.rb +++ b/app/models/observer/ticket/notification.rb @@ -20,80 +20,9 @@ class Observer::Ticket::Notification < ActiveRecord::Observer # get uniq objects listObjects = get_uniq_changes(list) listObjects.each {|ticket_id, item| - ticket = item[:ticket] - article = item[:article] || ticket.articles[-1] - # if create, send create message / block update messages - if item[:type] == 'create' - puts 'send ticket create notify to agent' - - article_content = '' - if item[:article] - article_content = ' -#{article.body} -' - end - send_notify( - { - :event => item[:type], - :recipient => 'to_work_on', # group|owner|to_work_on|customer - :subject => 'New Ticket (#{ticket.title})', - :body => 'Hi #{recipient.firstname}, - - 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}) - - ' + article_content + ' - - #{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}/#{article.id} - ' - }, - ticket, - article, - 'new ticket' - ) - end - - # if update, send update message, add changes to message - if item[:type] == 'update' - #puts "CHANGESSS #{item[:changes].inspect}" - puts 'send ticket update notify to agent' - changes = '' - item[:changes].each {|key,value| - changes = "#{key}: #{value[0]} -> #{value[1]}" - } - article_content = '' - if item[:article] - article_content = ' -#{article.body} -' - end - - send_notify( - { - :event => item[:type], - :recipient => 'to_work_on', # group|owner|to_work_on - :subject => 'Updated (#{ticket.title})', - :body => 'Hi #{recipient.firstname}, - - updated (#{ticket.title}) by #{ticket.updated_by.fullname}. - - Changes: - ' + changes + ' - - ' + article_content + ' - - #{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}/#{article.id} - ' - }, - ticket, - article, - 'update ticket', - ) - end + # send background job + Delayed::Job.enqueue( Observer::Ticket::Notification::BackgroundJob.new( item ) ) } end @@ -103,11 +32,14 @@ class Observer::Ticket::Notification < ActiveRecord::Observer result = { :1 => { - :type => 'create', + :type => 'create', + :ticket_id => 123, + :article_id => 123, }, :9 = { - :type => 'update', - :changes => { + :type => 'update', + :ticket_id => 123, + :changes => { :attribute1 => [before,now], :attribute2 => [before,now], } @@ -131,7 +63,7 @@ class Observer::Ticket::Notification < ActiveRecord::Observer if !listObjects[ticket.id] listObjects[ticket.id] = {} end - listObjects[ticket.id][:article] = article + listObjects[ticket.id][:article_id] = article.id elsif event[:name] == 'Ticket' ticket = Ticket.lookup( :id => event[:id] ) @@ -143,7 +75,7 @@ class Observer::Ticket::Notification < ActiveRecord::Observer if !listObjects[ticket.id] listObjects[ticket.id] = {} end - listObjects[ticket.id][:ticket] = ticket + listObjects[ticket.id][:ticket_id] = ticket.id if !listObjects[ticket.id][:type] || listObjects[ticket.id][:type] == 'update' listObjects[ticket.id][:type] = event[:type] @@ -170,18 +102,6 @@ class Observer::Ticket::Notification < ActiveRecord::Observer listObjects end - def self.send_notify(data, ticket, article, type) - - # send background job - params = { - :ticket_id => ticket.id, - :article_id => article.id, - :type => type, - :data => data, - } - Delayed::Job.enqueue( Observer::Ticket::Notification::BackgroundJob.new( params ) ) - end - def after_create(record) # return if we run import mode diff --git a/app/models/observer/ticket/notification/background_job.rb b/app/models/observer/ticket/notification/background_job.rb index e8ed89545..94df09672 100644 --- a/app/models/observer/ticket/notification/background_job.rb +++ b/app/models/observer/ticket/notification/background_job.rb @@ -1,18 +1,22 @@ +# encoding: utf-8 + class Observer::Ticket::Notification::BackgroundJob def initialize(params) @ticket_id = params[:ticket_id] @article_id = params[:article_id] @type = params[:type] - @data = params[:data] + @changes = params[:changes] end def perform ticket = Ticket.find(@ticket_id) - article = Ticket::Article.find(@article_id) - data = @data + if @article_id + article = Ticket::Article.find(@article_id) + end # find recipients recipients = [] +=begin # group of agents to work on if data[:recipient] == 'group' recipients = ticket.agent_of_group() @@ -38,10 +42,16 @@ class Observer::Ticket::Notification::BackgroundJob recipients = ticket.agent_of_group() end end +=end + + if ticket.owner_id != 1 + recipients.push ticket.owner + else + recipients = ticket.agent_of_group() + end # send notifications - recipient_list = '' - notification_subject = '' + recipient_list = '' recipients.each do |user| next if ticket.updated_by_id == user.id @@ -49,14 +59,13 @@ class Observer::Ticket::Notification::BackgroundJob # create desktop notification - # create online notification OnlineNotification.add( :type => @type, :object => 'Ticket', :o_id => ticket.id, :seen => false, - :created_by_id => article.created_by_id || 1, + :created_by_id => ticket.created_by_id || 1, :user_id => user.id, ) @@ -69,12 +78,22 @@ class Observer::Ticket::Notification::BackgroundJob end recipient_list += user.email.to_s + # get user based notification template + # if create, send create message / block update messages + if @type == 'create' + template = self.template_create(user.preferences[:locale], ticket, article, @changes) + elsif @type == 'update' + template = self.template_update(user.preferences[:locale], ticket, article, @changes) + else + raise "unknown type for notification #{@type}" + end + # prepare subject & body notification = {} [:subject, :body].each { |key| notification[key.to_sym] = NotificationFactory.build( - :locale => user.locale, - :string => data[key.to_sym], + :locale => user.preferences[:locale], + :string => template[key], :objects => { :ticket => ticket, :article => article, @@ -82,12 +101,13 @@ class Observer::Ticket::Notification::BackgroundJob } ) } - notification_subject = notification[:subject] # rebuild subject notification[:subject] = ticket.subject_build( notification[:subject] ) # send notification + puts "send ticket notifiaction to agent (#{@type}/#{ticket.id}/#{user.email})" + NotificationFactory.send( :recipient => user, :subject => notification[:subject], @@ -101,10 +121,174 @@ class Observer::Ticket::Notification::BackgroundJob :o_id => ticket.id, :history_type => 'notification', :history_object => 'Ticket', - :value_from => notification_subject, :value_to => recipient_list, - :created_by_id => article.created_by_id || 1 + :created_by_id => ticket.updated_by_id || 1 ) end end + + def template_create(lang, ticket, article, ticket_changes) + article_content = '' + if article + article_content = ' +#{article.body} +' + end + + if lang =~ /^de/i + subject = 'Neues Ticket (#{ticket.title})' + body = 'Hallo #{recipient.firstname}, + +es wurde ein neues Ticket (#{ticket.title}) von #{ticket.updated_by.fullname} erstellt. + +Gruppe: #{ticket.group.name} +Besitzer: #{ticket.owner.fullname} +Status: i18n(#{ticket.state.name}) + +' + article_content + ' + +' + else + + subject = 'New Ticket (#{ticket.title})' + body = 'Hi #{recipient.firstname}, + +a new Ticket (#{ticket.title}) has been created by #{ticket.updated_by.fullname}. + +Group: #{ticket.group.name} +Owner: #{ticket.owner.fullname} +State: i18n(#{ticket.state.name}) + +' + article_content + ' + +' + + end + + body = template_header(lang) + body.chomp.text2html + body += template_footer(lang, ticket, article) + + template = { + :subject => subject, + :body => body, + } + template + end + + def template_update(lang, ticket, article, ticket_changes) + changes = '' + ticket_changes.each {|key,value| + changes += "#{key}: #{value[0]} -> #{value[1]}\n" + } + article_content = '' + if article + article_content = ' +#{article.body} +' + end + if lang =~ /^de/i + subject = 'Ticket aktualisiert (#{ticket.title})' + body = 'Hallo #{recipient.firstname}, + +Ticket (#{ticket.title}) wurde von #{ticket.updated_by.fullname} aktualisiert. + +Änderungen: +' + changes + ' + +' + article_content + ' + +' + else + subject = 'Updated Ticket (#{ticket.title})' + body = 'Hi #{recipient.firstname}, + +Ticket (#{ticket.title}) has been updated by #{ticket.updated_by.fullname}. + +Changes: +' + changes + ' + +' + article_content + ' + +' + end + + body = template_header(lang) + body.chomp.text2html + body += template_footer(lang,ticket, article) + + template = { + :subject => subject, + :body => body, + } + template + end + + def template_header(lang) + ' + + +
+ #{config.product_name} i18n(Notification) +
+' + end + + def template_footer(lang, ticket, article) + ' +i18n(View the Ticket directly here) + +
+ i18n(Manage your notifications settings) +
+' + end end \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 695d66f18..c1aafd077 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -68,7 +68,6 @@ class User < ApplicationModel :source => true, :login_failed => true, :preferences => true, - :locale => true, } ) @@ -311,7 +310,7 @@ returns # prepare subject & body [:subject, :body].each { |key| data[key.to_sym] = NotificationFactory.build( - :locale => user.locale, + :locale => user.preferences[:locale], :string => data[key.to_sym], :objects => { :token => token, diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index c2d77576d..b41f51df2 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -32,7 +32,6 @@ class CreateBase < ActiveRecord::Migration t.column :note, :string, :limit => 250, :null => true t.column :last_login, :timestamp, :null => true t.column :source, :string, :limit => 200, :null => true - t.column :locale, :string, :limit => 10, :null => true t.column :login_failed, :integer, :null => false, :default => 0 t.column :preferences, :string, :limit => 8000,:null => true t.column :updated_by_id, :integer, :null => false diff --git a/lib/core_ext/string.rb b/lib/core_ext/string.rb index 41eb02e59..f2c19dd84 100644 --- a/lib/core_ext/string.rb +++ b/lib/core_ext/string.rb @@ -77,4 +77,17 @@ class String links = nil text.chomp end + +=begin + + html = text_string.text2html + +=end + + def text2html + text = CGI.escapeHTML( self ) + text.gsub!(/\n/, '
') + text.chomp + end + end \ No newline at end of file diff --git a/lib/notification_factory.rb b/lib/notification_factory.rb index b7db22bd2..47439b1c4 100644 --- a/lib/notification_factory.rb +++ b/lib/notification_factory.rb @@ -108,10 +108,11 @@ module NotificationFactory Channel::EmailSend.send( { # :in_reply_to => self.in_reply_to, - :from => sender, - :to => data[:recipient][:email], - :subject => data[:subject], - :body => data[:body], + :from => sender, + :to => data[:recipient][:email], + :subject => data[:subject], + :body => data[:body], + :content_type => 'text/html', }, true ) diff --git a/test/unit/ticket_notification_test.rb b/test/unit/ticket_notification_test.rb index 26ea3d34e..b69f0fbc9 100644 --- a/test/unit/ticket_notification_test.rb +++ b/test/unit/ticket_notification_test.rb @@ -77,7 +77,7 @@ class TicketNotificationTest < ActiveSupport::TestCase :updated_by_id => customer.id, :created_by_id => customer.id, ) - assert( ticket1, "ticket created" ) + assert( ticket1, "ticket created - ticket notification simple" ) # execute ticket events Observer::Ticket::Notification.transaction @@ -85,8 +85,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal( 1, notification_check(ticket1, agent1), 'agent 1 notification count check' ) - assert_equal( 1, notification_check(ticket1, agent2), 'agent 2 notification count check' ) + assert_equal( 1, notification_check(ticket1, agent1), ticket1.id ) + assert_equal( 1, notification_check(ticket1, agent2), ticket1.id ) # update ticket attributes ticket1.title = "#{ticket1.title} - #2" @@ -99,8 +99,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal( 2, notification_check(ticket1, agent1), 'agent 1 notification count check' ) - assert_equal( 2, notification_check(ticket1, agent2), 'agent 2 notification count check' ) + assert_equal( 2, notification_check(ticket1, agent1), ticket1.id ) + assert_equal( 2, notification_check(ticket1, agent2), ticket1.id ) # add article to ticket article_note = Ticket::Article.create( @@ -151,8 +151,8 @@ class TicketNotificationTest < ActiveSupport::TestCase assert( ticket2, "ticket created" ) # verify notifications to no one - assert_equal( 0, notification_check(ticket2, agent1), 'agent 1 notification count check' ) - assert_equal( 0, notification_check(ticket2, agent2), 'agent 2 notification count check' ) + assert_equal( 0, notification_check(ticket2, agent1), ticket2.id ) + assert_equal( 0, notification_check(ticket2, agent2), ticket2.id ) # update ticket ticket2.title = "#{ticket2.title} - #2" @@ -166,8 +166,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to no one - assert_equal( 0, notification_check(ticket2, agent1), 'agent 1 notification count check' ) - assert_equal( 0, notification_check(ticket2, agent2), 'agent 2 notification count check' ) + assert_equal( 0, notification_check(ticket2, agent1), ticket2.id ) + assert_equal( 0, notification_check(ticket2, agent2), ticket2.id ) # update ticket ticket2.title = "#{ticket2.title} - #3" @@ -181,8 +181,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 and not to agent2 - assert_equal( 1, notification_check(ticket2, agent1), 'agent 1 notification count check' ) - assert_equal( 0, notification_check(ticket2, agent2), 'agent 2 notification count check' ) + assert_equal( 1, notification_check(ticket2, agent1), ticket2.id ) + assert_equal( 0, notification_check(ticket2, agent2), ticket2.id ) @@ -218,8 +218,8 @@ class TicketNotificationTest < ActiveSupport::TestCase assert( ticket3, "ticket created" ) # verify notifications to agent1 and not to agent2 - assert_equal( 1, notification_check(ticket3, agent1), 'agent 1 notification count check' ) - assert_equal( 0, notification_check(ticket3, agent2), 'agent 2 notification count check' ) + assert_equal( 1, notification_check(ticket3, agent1), ticket3.id ) + assert_equal( 0, notification_check(ticket3, agent2), ticket3.id ) # update ticket ticket3.title = "#{ticket3.title} - #2" @@ -233,8 +233,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to no one - assert_equal( 1, notification_check(ticket3, agent1), 'agent 1 notification count check' ) - assert_equal( 0, notification_check(ticket3, agent2), 'agent 2 notification count check' ) + assert_equal( 1, notification_check(ticket3, agent1), ticket3.id ) + assert_equal( 0, notification_check(ticket3, agent2), ticket3.id ) # update ticket ticket3.title = "#{ticket3.title} - #3" @@ -248,8 +248,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 and not to agent2 - assert_equal( 2, notification_check(ticket3, agent1), 'agent 1 notification count check' ) - assert_equal( 0, notification_check(ticket3, agent2), 'agent 2 notification count check' ) + assert_equal( 2, notification_check(ticket3, agent1), ticket3.id ) + assert_equal( 0, notification_check(ticket3, agent2), ticket3.id ) delete = ticket1.destroy