From 4b85c1f662dd41b9128056ebdc244d3c16af7552 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 31 Jan 2017 09:56:59 +0100 Subject: [PATCH] Fixed issue #643 - Group follow up check for closed tickets does not work. --- .../javascripts/app/models/group.coffee | 2 +- .../filter/follow_up_possible_check.rb | 22 ++++++ ...0130000001_follow_up_possible_check_643.rb | 54 ++++++++++++++ db/seeds.rb | 10 ++- .../email_process_follow_up_possible_test.rb | 72 +++++++++++++++++++ 5 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 app/models/channel/filter/follow_up_possible_check.rb create mode 100644 db/migrate/20170130000001_follow_up_possible_check_643.rb create mode 100644 test/unit/email_process_follow_up_possible_test.rb diff --git a/app/assets/javascripts/app/models/group.coffee b/app/assets/javascripts/app/models/group.coffee index da4114b61..b9b2925a2 100644 --- a/app/assets/javascripts/app/models/group.coffee +++ b/app/assets/javascripts/app/models/group.coffee @@ -6,7 +6,7 @@ class App.Group extends App.Model @configure_attributes = [ { name: 'name', display: 'Name', tag: 'input', type: 'text', limit: 100, null: false }, { name: 'assignment_timeout', display: 'Assignment Timeout', tag: 'input', note: 'Assignment timeout in minutes if assigned agent is not working on it. Ticket will be shown as unassigend.', type: 'text', limit: 100, null: true }, - { name: 'follow_up_possible', display: 'Follow up possible',tag: 'select', default: 'yes', options: { yes: 'yes', reject: 'reject follow up/do not reopen Ticket', 'new_ticket': 'do not reopen Ticket but create new Ticket' }, null: false, note: 'Follow up for closed ticket possible or not.', translate: true }, + { name: 'follow_up_possible', display: 'Follow up possible',tag: 'select', default: 'yes', options: { yes: 'yes', 'new_ticket': 'do not reopen Ticket but create new Ticket' }, null: false, note: 'Follow up for closed ticket possible or not.', translate: true }, { name: 'follow_up_assignment', display: 'Assign Follow Ups', tag: 'select', default: 'yes', options: { true: 'yes', false: 'no' }, null: false, note: 'Assign follow up to latest agent again.', translate: true }, { name: 'email_address_id', display: 'Email', tag: 'select', multiple: false, null: true, relation: 'EmailAddress', nulloption: true, do_not_log: true }, { name: 'signature_id', display: 'Signature', tag: 'select', multiple: false, null: true, relation: 'Signature', nulloption: true, do_not_log: true }, diff --git a/app/models/channel/filter/follow_up_possible_check.rb b/app/models/channel/filter/follow_up_possible_check.rb new file mode 100644 index 000000000..8e5ab4e85 --- /dev/null +++ b/app/models/channel/filter/follow_up_possible_check.rb @@ -0,0 +1,22 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +module Channel::Filter::FollowUpPossibleCheck + + def self.run(_channel, mail) + ticket_id = mail['x-zammad-ticket-id'.to_sym] + return true if !ticket_id + ticket = Ticket.lookup(id: ticket_id) + return true if !ticket + return true if ticket.state.state_type.name !~ /^(closed|merged|removed)/i + + # in case of closed tickets, remove follow up information + case ticket.group.follow_up_possible + when 'new_ticket' + mail[:subject] = ticket.subject_clean(mail[:subject]) + mail['x-zammad-ticket-id'.to_sym] = nil + mail['x-zammad-ticket-number'.to_sym] = nil + end + + true + end +end diff --git a/db/migrate/20170130000001_follow_up_possible_check_643.rb b/db/migrate/20170130000001_follow_up_possible_check_643.rb new file mode 100644 index 000000000..f2a6658be --- /dev/null +++ b/db/migrate/20170130000001_follow_up_possible_check_643.rb @@ -0,0 +1,54 @@ +class FollowUpPossibleCheck643 < ActiveRecord::Migration + def up + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + Setting.create_if_not_exists( + title: 'Define postmaster filter.', + name: '0200_postmaster_filter_follow_up_possible_check', + area: 'Postmaster::PreFilter', + description: 'Define postmaster filter to check if follow ups get created (based on admin settings).', + options: {}, + state: 'Channel::Filter::FollowUpPossibleCheck', + frontend: false + ) + + ObjectManager::Attribute.add( + force: true, + object: 'Group', + name: 'follow_up_possible', + display: 'Follow up possible', + data_type: 'select', + data_option: { + default: 'yes', + options: { + yes: 'yes', + new_ticket: 'do not reopen Ticket but create new Ticket' + }, + null: false, + note: 'Follow up for closed ticket possible or not.', + translate: true + }, + editable: false, + active: true, + screens: { + create: { + '-all-' => { + null: true, + }, + }, + edit: { + '-all-' => { + null: true, + }, + }, + }, + to_create: false, + to_migrate: false, + to_delete: false, + position: 400, + ) + + end +end diff --git a/db/seeds.rb b/db/seeds.rb index bf1b538e4..4977fc58e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -2236,6 +2236,15 @@ Setting.create_if_not_exists( state: 'Channel::Filter::FollowUpCheck', frontend: false ) +Setting.create_if_not_exists( + title: 'Define postmaster filter.', + name: '0200_postmaster_filter_follow_up_possible_check', + area: 'Postmaster::PreFilter', + description: 'Define postmaster filter to check if follow ups get created (based on admin settings).', + options: {}, + state: 'Channel::Filter::FollowUpPossibleCheck', + frontend: false +) Setting.create_if_not_exists( title: 'Define postmaster filter.', name: '0900_postmaster_filter_bounce_check', @@ -5064,7 +5073,6 @@ ObjectManager::Attribute.add( default: 'yes', options: { yes: 'yes', - reject: 'reject follow up/do not reopen Ticket', new_ticket: 'do not reopen Ticket but create new Ticket' }, null: false, diff --git a/test/unit/email_process_follow_up_possible_test.rb b/test/unit/email_process_follow_up_possible_test.rb new file mode 100644 index 000000000..cf8def07d --- /dev/null +++ b/test/unit/email_process_follow_up_possible_test.rb @@ -0,0 +1,72 @@ +# encoding: utf-8 +require 'test_helper' + +class EmailProcessFollowUpPossibleTest < ActiveSupport::TestCase + + test 'process with follow up possible check' do + + users_group = Group.lookup(name: 'Users') + + ticket = Ticket.create( + title: 'follow up check', + group: users_group, + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + article = Ticket::Article.create( + ticket_id: ticket.id, + from: 'some_sender@example.com', + to: 'some_recipient@example.com', + subject: 'follow up check', + message_id: '<20150830145601.30.608882@edenhofer.zammad.com>', + body: 'some message article', + internal: false, + sender: Ticket::Article::Sender.lookup(name: 'Agent'), + type: Ticket::Article::Type.lookup(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + + follow_up_raw = "From: me@example.com +To: customer@example.com +Subject: #{ticket.subject_build('some new subject')} + +Some Text" + + users_group.update_attribute('follow_up_possible', 'new_ticket') + + travel 1.second + ticket_p, article_p, user_p = Channel::EmailParser.new.process({}, follow_up_raw) + assert_equal(ticket.id, ticket_p.id) + assert_equal('follow up check', ticket_p.title) + assert_match('some new subject', article_p.subject) + + # close ticket + ticket.state = Ticket::State.find_by(name: 'closed') + ticket.save! + + follow_up_raw = "From: me@example.com +To: customer@example.com +Subject: #{ticket.subject_build('some new subject2')} + +Some Text" + + travel 1.second + ticket_p, article_p, user_p = Channel::EmailParser.new.process({}, follow_up_raw) + assert_not_equal(ticket.id, ticket_p.id) + assert_equal('some new subject2', ticket_p.title) + assert_equal('some new subject2', article_p.subject) + + users_group.update_attribute('follow_up_possible', 'yes') + + travel 1.second + ticket_p, article_p, user_p = Channel::EmailParser.new.process({}, follow_up_raw) + assert_equal(ticket.id, ticket_p.id) + assert_equal('follow up check', ticket_p.title) + assert_match('some new subject', article_p.subject) + travel_back + end +end