From ada6efa3bcdc4207dfb095bbef0806f30ad70aa6 Mon Sep 17 00:00:00 2001 From: Mantas Date: Fri, 18 May 2018 13:59:08 +0300 Subject: [PATCH] Fixes #1730 When forwarding a message, prefix FWD is added --- .../app/controllers/ticket_zoom.coffee | 1 + .../article_action/email_reply.coffee | 8 ++- .../app/models/ticket_article.coffee | 2 +- .../app/views/ticket_zoom/article_new.jst.eco | 1 + .../concerns/creates_ticket_articles.rb | 5 ++ .../communicate_email/background_job.rb | 9 ++- app/models/ticket/subject.rb | 60 +++++++++++-------- ...email_forward_prefix_setting_issue_1730.rb | 28 +++++++++ db/seeds/settings.rb | 22 +++++++ test/unit/ticket_test.rb | 21 ++++--- 10 files changed, 115 insertions(+), 42 deletions(-) create mode 100644 db/migrate/20180518100316_email_forward_prefix_setting_issue_1730.rb diff --git a/app/assets/javascripts/app/controllers/ticket_zoom.coffee b/app/assets/javascripts/app/controllers/ticket_zoom.coffee index 4901f2ecc..cc9dd7fe0 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom.coffee @@ -605,6 +605,7 @@ class App.TicketZoom extends App.Controller body: '' internal: internal in_reply_to: '' + subtype: '' if @permissionCheck('ticket.customer') currentStore.article.internal = '' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee index 204dbd7d4..ca2df890c 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee @@ -157,6 +157,8 @@ class EmailReply extends App.Controller type = App.TicketArticleType.findByAttribute(name:'email') + articleNew.subtype = 'reply' + App.Event.trigger('ui::ticket::setArticleType', { ticket: ticket type: type @@ -187,12 +189,14 @@ class EmailReply extends App.Controller if ui.Config.get('ui_ticket_zoom_article_email_subject') if _.isEmpty(article.subject) - articleNew.subject = "FW: #{ticket.title}" + articleNew.subject = ticket.title else - articleNew.subject = "FW: #{article.subject}" + articleNew.subject = article.subject type = App.TicketArticleType.findByAttribute(name:'email') + articleNew.subtype = 'forward' + App.Event.trigger('ui::ticket::setArticleType', { ticket: ticket type: type diff --git a/app/assets/javascripts/app/models/ticket_article.coffee b/app/assets/javascripts/app/models/ticket_article.coffee index 7c3523ace..03ab869b5 100644 --- a/app/assets/javascripts/app/models/ticket_article.coffee +++ b/app/assets/javascripts/app/models/ticket_article.coffee @@ -1,5 +1,5 @@ class App.TicketArticle extends App.Model - @configure 'TicketArticle', 'from', 'to', 'cc', 'subject', 'body', 'content_type', 'ticket_id', 'type_id', 'sender_id', 'internal', 'in_reply_to', 'form_id', 'time_unit', 'preferences', 'updated_at' + @configure 'TicketArticle', 'from', 'to', 'cc', 'subject', 'body', 'content_type', 'ticket_id', 'type_id', 'sender_id', 'internal', 'in_reply_to', 'form_id', 'subtype', 'time_unit', 'preferences', 'updated_at' @extend Spine.Model.Ajax @url: @apiPath + '/ticket_articles' @configure_attributes = [ diff --git a/app/assets/javascripts/app/views/ticket_zoom/article_new.jst.eco b/app/assets/javascripts/app/views/ticket_zoom/article_new.jst.eco index a4d1ae0b3..08e6d13c0 100644 --- a/app/assets/javascripts/app/views/ticket_zoom/article_new.jst.eco +++ b/app/assets/javascripts/app/views/ticket_zoom/article_new.jst.eco @@ -2,6 +2,7 @@ +
diff --git a/app/controllers/concerns/creates_ticket_articles.rb b/app/controllers/concerns/creates_ticket_articles.rb index ca1e2eb6a..6d5ca281b 100644 --- a/app/controllers/concerns/creates_ticket_articles.rb +++ b/app/controllers/concerns/creates_ticket_articles.rb @@ -8,6 +8,7 @@ module CreatesTicketArticles # create article if given form_id = params[:form_id] params.delete(:form_id) + subtype = params.delete(:subtype) # check min. params raise Exceptions::UnprocessableEntity, 'Need at least article: { body: "some text" }' if !params[:body] @@ -59,6 +60,10 @@ module CreatesTicketArticles o_id: form_id, ) end + + # set subtype of present + article.preferences[:subtype] = subtype if subtype.present? + article.save! # store inline attachments diff --git a/app/models/observer/ticket/article/communicate_email/background_job.rb b/app/models/observer/ticket/article/communicate_email/background_job.rb index 87c4ceaef..1b2e14f29 100644 --- a/app/models/observer/ticket/article/communicate_email/background_job.rb +++ b/app/models/observer/ticket/article/communicate_email/background_job.rb @@ -9,11 +9,10 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob # build subject ticket = Ticket.lookup(id: record.ticket_id) article_count = Ticket::Article.where(ticket_id: ticket.id).count - subject = if article_count > 1 - ticket.subject_build(record.subject, true) - else - ticket.subject_build(record.subject) - end + + subject_prefix_mode = record.preferences[:subtype] + + subject = ticket.subject_build(record.subject, subject_prefix_mode) # set retry count if !record.preferences['delivery_retry'] diff --git a/app/models/ticket/subject.rb b/app/models/ticket/subject.rb index 2516b0424..d3267eda2 100644 --- a/app/models/ticket/subject.rb +++ b/app/models/ticket/subject.rb @@ -6,7 +6,8 @@ module Ticket::Subject build new subject with ticket number in there ticket = Ticket.find(123) - result = ticket.subject_build('some subject', is_reply_true_false) + prefix_mode = :reply # :forward, nil + result = ticket.subject_build('some subject', prefix_mode) returns @@ -14,36 +15,25 @@ returns =end - def subject_build(subject, is_reply = false) + def subject_build(subject, prefix_mode = nil) # clena subject - subject = subject_clean(subject) + subject_parts = [subject_clean(subject)] - ticket_hook = Setting.get('ticket_hook') - ticket_hook_divider = Setting.get('ticket_hook_divider') - ticket_subject_re = Setting.get('ticket_subject_re') - - # none position - if Setting.get('ticket_hook_position') == 'none' - if is_reply && ticket_subject_re.present? - subject = "#{ticket_subject_re}: #{subject}" - end - return subject + # add hook + case Setting.get('ticket_hook_position') + when 'left' + subject_parts.unshift subject_build_hook + when 'right' + subject_parts.push subject_build_hook end - # right position - if Setting.get('ticket_hook_position') == 'right' - if is_reply && ticket_subject_re.present? - subject = "#{ticket_subject_re}: #{subject}" - end - return "#{subject} [#{ticket_hook}#{ticket_hook_divider}#{number}]" - end + # add prefix + subject_parts + .unshift(subject_build_prefix(prefix_mode)) + .compact! - # left position - if is_reply && ticket_subject_re.present? - return "#{ticket_subject_re}: [#{ticket_hook}#{ticket_hook_divider}#{number}] #{subject}" - end - "[#{ticket_hook}#{ticket_hook_divider}#{number}] #{subject}" + subject_parts.join ' ' end =begin @@ -85,4 +75,24 @@ returns subject.strip! subject end + + private + + def subject_build_hook + ticket_hook = Setting.get('ticket_hook') + ticket_hook_divider = Setting.get('ticket_hook_divider') + + "[#{ticket_hook}#{ticket_hook_divider}#{number}]" + end + + def subject_build_prefix(prefix_mode) + prefix = case prefix_mode + when 'reply' + Setting.get('ticket_subject_re') + when 'forward' + Setting.get('ticket_subject_fwd') + end + + prefix.present? ? "#{prefix}:" : nil + end end diff --git a/db/migrate/20180518100316_email_forward_prefix_setting_issue_1730.rb b/db/migrate/20180518100316_email_forward_prefix_setting_issue_1730.rb new file mode 100644 index 000000000..dec4b5cf0 --- /dev/null +++ b/db/migrate/20180518100316_email_forward_prefix_setting_issue_1730.rb @@ -0,0 +1,28 @@ +class EmailForwardPrefixSettingIssue1730 < ActiveRecord::Migration[5.1] + def up + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + Setting.create_if_not_exists( + title: 'Ticket Subject Forward', + name: 'ticket_subject_fwd', + area: 'Email::Base', + description: 'The text at the beginning of the subject in an email forward, e. g. FWD.', + options: { + form: [ + { + display: '', + null: true, + name: 'ticket_subject_fwd', + tag: 'input', + }, + ], + }, + state: 'FWD', + preferences: { + permission: ['admin.channel_email'], + }, + frontend: false + ) + end +end diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb index 7166fa0d2..42eeed444 100644 --- a/db/seeds/settings.rb +++ b/db/seeds/settings.rb @@ -2308,6 +2308,28 @@ Setting.create_if_not_exists( frontend: false ) +Setting.create_if_not_exists( + title: 'Ticket Subject Forward', + name: 'ticket_subject_fwd', + area: 'Email::Base', + description: 'The text at the beginning of the subject in an email forward, e. g. FWD.', + options: { + form: [ + { + display: '', + null: true, + name: 'ticket_subject_fwd', + tag: 'input', + }, + ], + }, + state: 'FWD', + preferences: { + permission: ['admin.channel_email'], + }, + frontend: false +) + Setting.create_if_not_exists( title: 'Sender Format', name: 'ticket_define_email_from', diff --git a/test/unit/ticket_test.rb b/test/unit/ticket_test.rb index 90dae91ea..bd6b89952 100644 --- a/test/unit/ticket_test.rb +++ b/test/unit/ticket_test.rb @@ -300,9 +300,10 @@ class TicketTest < ActiveSupport::TestCase ) assert_equal('subject test 1', ticket.title) assert_equal("ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build('ABC subject test 1')) - assert_equal("RE: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build('ABC subject test 1', true)) - assert_equal("RE: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build(' ABC subject test 1', true)) - assert_equal("RE: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build('ABC subject test 1 ', true)) + assert_equal("RE: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build('ABC subject test 1', 'reply')) + assert_equal("RE: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build(' ABC subject test 1', 'reply')) + assert_equal("RE: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build('ABC subject test 1 ', 'reply')) + assert_equal("FWD: ABC subject test 1 [Ticket##{ticket.number}]", ticket.subject_build('ABC subject test 1 ', 'forward')) ticket.destroy Setting.set('ticket_hook_position', 'left') @@ -318,9 +319,10 @@ class TicketTest < ActiveSupport::TestCase ) assert_equal('subject test 1', ticket.title) assert_equal("[Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build('ABC subject test 1')) - assert_equal("RE: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build('ABC subject test 1', true)) - assert_equal("RE: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build(' ABC subject test 1', true)) - assert_equal("RE: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build('ABC subject test 1 ', true)) + assert_equal("RE: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build('ABC subject test 1', 'reply')) + assert_equal("RE: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build(' ABC subject test 1', 'reply')) + assert_equal("RE: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build('ABC subject test 1 ', 'reply')) + assert_equal("FWD: [Ticket##{ticket.number}] ABC subject test 1", ticket.subject_build('ABC subject test 1 ', 'forward')) ticket.destroy Setting.set('ticket_hook_position', 'none') @@ -336,9 +338,10 @@ class TicketTest < ActiveSupport::TestCase ) assert_equal('subject test 1', ticket.title) assert_equal('ABC subject test 1', ticket.subject_build('ABC subject test 1')) - assert_equal('RE: ABC subject test 1', ticket.subject_build('ABC subject test 1', true)) - assert_equal('RE: ABC subject test 1', ticket.subject_build(' ABC subject test 1', true)) - assert_equal('RE: ABC subject test 1', ticket.subject_build('ABC subject test 1 ', true)) + assert_equal('RE: ABC subject test 1', ticket.subject_build('ABC subject test 1', 'reply')) + assert_equal('RE: ABC subject test 1', ticket.subject_build(' ABC subject test 1', 'reply')) + assert_equal('RE: ABC subject test 1', ticket.subject_build('ABC subject test 1 ', 'reply')) + assert_equal('FWD: ABC subject test 1', ticket.subject_build('ABC subject test 1 ', 'forward')) ticket.destroy end