From 7cf884c7a40f895c80ded44620713b539a043ba9 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 23 May 2016 19:23:06 +0200 Subject: [PATCH] In destination group is invalid, use the oldest valid group. Also notice in admin interface in email account config about invalid groups. --- .../app/controllers/_channel/email.coffee | 34 ++--- .../channel/email_account_overview.jst.eco | 14 +- app/models/channel/email_parser.rb | 13 +- test/unit/email_process_test.rb | 127 +++++++++++++++--- 4 files changed, 151 insertions(+), 37 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_channel/email.coffee b/app/assets/javascripts/app/controllers/_channel/email.coffee index 7cfec2175..876f92d54 100644 --- a/app/assets/javascripts/app/controllers/_channel/email.coffee +++ b/app/assets/javascripts/app/controllers/_channel/email.coffee @@ -222,13 +222,13 @@ class App.ChannelEmailAccountOverview extends App.Controller events: 'click .js-channelNew': 'wizard' 'click .js-channelDelete': 'delete' - 'click .js-channelGroupChange': 'group_change' - 'click .js-editInbound': 'edit_inbound' - 'click .js-editOutbound': 'edit_outbound' - 'click .js-emailAddressNew': 'email_address_new' - 'click .js-emailAddressEdit': 'email_address_edit' - 'click .js-emailAddressDelete': 'email_address_delete', - 'click .js-editNotificationOutbound': 'edit_notification_outbound' + 'click .js-channelGroupChange': 'groupChange' + 'click .js-editInbound': 'editInbound' + 'click .js-editOutbound': 'editOutbound' + 'click .js-emailAddressNew': 'emailAddressNew' + 'click .js-emailAddressEdit': 'emailAddressEdit' + 'click .js-emailAddressDelete': 'emailAddressDelete', + 'click .js-editNotificationOutbound': 'editNotificationOutbound' constructor: -> super @@ -259,13 +259,13 @@ class App.ChannelEmailAccountOverview extends App.Controller for channel_id in data.account_channel_ids account_channel = App.Channel.fullLocal(channel_id) if account_channel.group_id - account_channel.group = App.Group.find(account_channel.group_id).displayName() + account_channel.group = App.Group.find(account_channel.group_id) else account_channel.group = '-' account_channels.push account_channel for channel in account_channels - email_addresses = App.EmailAddress.search( filter: { channel_id: channel.id } ) + email_addresses = App.EmailAddress.search(filter: { channel_id: channel.id }) channel.email_addresses = email_addresses # get all unlinked email addresses @@ -294,7 +294,7 @@ class App.ChannelEmailAccountOverview extends App.Controller channelDriver: @channelDriver ) - edit_inbound: (e) => + editInbound: (e) => e.preventDefault() id = $(e.target).closest('.action').data('id') channel = App.Channel.find(id) @@ -307,7 +307,7 @@ class App.ChannelEmailAccountOverview extends App.Controller channelDriver: @channelDriver ) - edit_outbound: (e) => + editOutbound: (e) => e.preventDefault() id = $(e.target).closest('.action').data('id') channel = App.Channel.find(id) @@ -330,7 +330,7 @@ class App.ChannelEmailAccountOverview extends App.Controller callback: @load ) - group_change: (e) => + groupChange: (e) => e.preventDefault() id = $(e.target).closest('.action').data('id') item = App.Channel.find(id) @@ -340,7 +340,7 @@ class App.ChannelEmailAccountOverview extends App.Controller callback: @load ) - email_address_new: (e) => + emailAddressNew: (e) => e.preventDefault() channel_id = $(e.target).closest('.action').data('id') new App.ControllerGenericNew( @@ -353,7 +353,7 @@ class App.ChannelEmailAccountOverview extends App.Controller callback: @load ) - email_address_edit: (e) => + emailAddressEdit: (e) => e.preventDefault() id = $(e.target).closest('li').data('id') new App.ControllerGenericEdit( @@ -365,7 +365,7 @@ class App.ChannelEmailAccountOverview extends App.Controller callback: @load ) - email_address_delete: (e) => + emailAddressDelete: (e) => e.preventDefault() id = $(e.target).closest('li').data('id') item = App.EmailAddress.find(id) @@ -375,7 +375,7 @@ class App.ChannelEmailAccountOverview extends App.Controller callback: @load ) - edit_notification_outbound: (e) => + editNotificationOutbound: (e) => e.preventDefault() id = $(e.target).closest('.action').data('id') channel = App.Channel.find(id) @@ -395,7 +395,7 @@ class App.ChannelEmailEdit extends App.ControllerModal content: => configureAttributesBase = [ - { name: 'group_id', display: 'Destination Group', tag: 'select', null: false, relation: 'Group', nulloption: true }, + { name: 'group_id', display: 'Destination Group', tag: 'select', null: false, relation: 'Group', nulloption: true, filter: { active: true } }, ] @form = new App.ControllerForm( model: diff --git a/app/assets/javascripts/app/views/channel/email_account_overview.jst.eco b/app/assets/javascripts/app/views/channel/email_account_overview.jst.eco index b962bd953..eb65b6a52 100644 --- a/app/assets/javascripts/app/views/channel/email_account_overview.jst.eco +++ b/app/assets/javascripts/app/views/channel/email_account_overview.jst.eco @@ -72,7 +72,19 @@

<%- @T('Destination Group') %>

- <%= channel.group %> + + <% groupName = '' %> + <% if channel.group.displayName: %> + <% groupName = channel.group.displayName() %> + <% else: %> + <% groupName = channel.group %> + <% end %> + <% if channel.group.active is false: %> + <%- @T('%s is inactive, please select a active one.', groupName) %> + <% else: %> + <%= groupName %> + <% end %> +
diff --git a/app/models/channel/email_parser.rb b/app/models/channel/email_parser.rb index d922b8ecd..54660a694 100644 --- a/app/models/channel/email_parser.rb +++ b/app/models/channel/email_parser.rb @@ -427,8 +427,19 @@ retrns } end + # get default group where ticket is created + group = nil + if channel[:group_id] + group = Group.lookup(id: channel[:group_id]) + end + if !group || group && !group.active + group = Group.where(active: true).order('id ASC').first + end + if !group + group = Group.first + end ticket = Ticket.new( - group_id: channel[:group_id] || 1, + group_id: group.id, customer_id: user.id, title: mail[:subject] || '', state_id: Ticket::State.find_by(name: 'new').id, diff --git a/test/unit/email_process_test.rb b/test/unit/email_process_test.rb index cb0f82863..ee4b5c961 100644 --- a/test/unit/email_process_test.rb +++ b/test/unit/email_process_test.rb @@ -11,7 +11,9 @@ To: customer@example.com Subject: some subject Some Text', - trusted: false, + channel: { + trusted: false, + }, success: true, }, { @@ -20,7 +22,9 @@ To: customer@example.com Subject: äöü some subject Some Textäöü", - trusted: false, + channel: { + trusted: false, + }, success: true, result: { 0 => { @@ -116,7 +120,9 @@ Subject: Subject: =?utf-8?B?44CQ5LiT5Lia5Li65oKo5rOo5YaM6aaZ5riv5Y+K5rW35aSW5YWs =?utf-8?B?5YWs5Y+46Zmi5aOr5bel5L2c56uZ5pel5YmN5q2j5byP5bu6Li4uW+ivpue7hl0=?= Some Text", - trusted: false, + channel: { + trusted: false, + }, success: true, result: { 0 => { @@ -1834,7 +1840,9 @@ Cc: any@example.com Subject: some subject Some Text', - trusted: false, + channel: { + trusted: false, + }, success: true, result: { 0 => { @@ -1989,7 +1997,9 @@ Subject: some subject X-Zammad-Ignore: true Some Text', - trusted: true, + channel: { + trusted: true, + }, success: false, }, { @@ -2003,7 +2013,9 @@ x-Zammad-Article-type: phone x-Zammad-Article-Internal: true Some Text', - trusted: true, + channel: { + trusted: true, + }, success: true, result: { 0 => { @@ -2035,7 +2047,9 @@ x-Zammad-Article-Type: phone x-Zammad-Article-Internal: true Some Text', - trusted: false, + channel: { + trusted: false, + }, success: true, result: { 0 => { @@ -2054,9 +2068,86 @@ Some Text', process(files) end + test 'process inactive group - a' do + group3 = Group.create_if_not_exists( + name: 'Test Group Inactive', + active: false, + created_by_id: 1, + updated_by_id: 1, + ) + files = [ + { + data: 'From: me@example.com +To: customer@example.com +Subject: some subject + +Some Text', + channel: { + group_id: group3.id, + }, + success: true, + result: { + 0 => { + state: 'new', + group: 'Users', + priority: '2 normal', + title: 'some subject', + }, + 1 => { + sender: 'Customer', + type: 'email', + internal: false, + }, + }, + }, + ] + process(files) + end + + test 'process inactive group - b' do + group_active_map = {} + Group.all.each {|group| + group_active_map[group.id] = group.active + group.active = false + group.save + } + files = [ + { + data: 'From: me@example.com +To: customer@example.com +Subject: some subject + +Some Text', + channel: {}, + success: true, + result: { + 0 => { + state: 'new', + group: 'Users', + priority: '2 normal', + title: 'some subject', + }, + 1 => { + sender: 'Customer', + type: 'email', + internal: false, + }, + }, + }, + ] + process(files) + + group_active_map = {} + Group.all.each {|group| + next if !group_active_map.key?(group.id) + group.active = group_active_map[group.id] + group.save + } + end + def process(files) files.each { |file| - result = Channel::EmailParser.new.process( { trusted: file[:trusted] }, file[:data] ) + result = Channel::EmailParser.new.process(file[:channel]||{}, file[:data]) if file[:success] if result && result.class == Array && result[1] assert( true ) @@ -2065,9 +2156,9 @@ Some Text', if file[:result][level] file[:result][level].each { |key, value| if result[level].send(key).respond_to?('name') - assert_equal( value.to_s, result[level].send(key).name ) + assert_equal(value.to_s, result[level].send(key).name) else - assert_equal( value, result[level].send(key), "result check #{level}, #{key}") + assert_equal(value, result[level].send(key), "result check #{level}, #{key}") end } end @@ -2079,31 +2170,31 @@ Some Text', file[:verify][:users].each { |user_result| user = User.where(email: user_result[:email]).first if !user - assert( false, "No user '#{user_result[:email]}' found!" ) + assert(false, "No user '#{user_result[:email]}' found!") return end user_result.each { |key, value| - if user.respond_to?( key ) - assert_equal( value, user.send(key), "user check #{ key }" ) + if user.respond_to?( key) + assert_equal(value, user.send(key), "user check #{ key }") else - assert_equal( value, user[key], "user check #{ key }" ) + assert_equal(value, user[key], "user check #{ key }" ) end } } end end else - assert( false, 'ticket not created', file ) + assert(false, 'ticket not created', file) end elsif !file[:success] if result && result.class == Array && result[1] puts result.inspect - assert( false, 'ticket should not be created but is created' ) + assert(false, 'ticket should not be created but is created') else - assert( true, 'ticket not created - nice' ) + assert(true, 'ticket not created - nice') end else - assert( false, 'UNKNOWN!' ) + assert(false, 'UNKNOWN!') end } end