From cc2c5678f0302dea4c1933bb3873c1709a18c0d1 Mon Sep 17 00:00:00 2001 From: Bola Ahmed Buari Date: Mon, 16 Aug 2021 13:34:05 +0200 Subject: [PATCH] Fixes #2485: Add attachment via Trigger and scheduler if wanted by admin --- .../_ui_element/ticket_perform_action.coffee | 10 ++ .../app/views/generic/checkbox.jst.eco | 2 +- .../notification.jst.eco | 6 + app/models/ticket.rb | 5 + public/assets/tests/form_extended.js | 20 ++- .../tests/form_ticket_perform_action.js | 42 ++++-- spec/models/ticket_spec.rb | 111 +++++++++++++++ spec/models/trigger_spec.rb | 133 ++++++++++++++++++ 8 files changed, 308 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee b/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee index 6ad309d16..7a47df79f 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee @@ -455,7 +455,17 @@ class App.UiElement.ticket_perform_action translate: true ) + includeAttachmentsCheckbox = App.UiElement.select.render( + name: "#{name}::include_attachments" + multiple: false + null: false + options: { true: 'Yes', false: 'No' } + value: meta.include_attachments || 'false' + translate: true + ) + notificationElement.find('.js-internal').html(visibilitySelection) + notificationElement.find('.js-include_attachments').html(includeAttachmentsCheckbox) notificationElement.find('.js-body div[contenteditable="true"]').ce( mode: 'richtext' diff --git a/app/assets/javascripts/app/views/generic/checkbox.jst.eco b/app/assets/javascripts/app/views/generic/checkbox.jst.eco index 7e96f92d6..a3ae5d5dc 100644 --- a/app/assets/javascripts/app/views/generic/checkbox.jst.eco +++ b/app/assets/javascripts/app/views/generic/checkbox.jst.eco @@ -1,6 +1,6 @@
<% for row in @attribute.options: %> -
+
+
+ +
+
+
<% end %>
diff --git a/app/models/ticket.rb b/app/models/ticket.rb index d8f4bf6a8..1f89df3b1 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -1755,6 +1755,11 @@ result end original_article = objects[:article] + + if ActiveModel::Type::Boolean.new.cast(value['include_attachments']) == true && original_article&.attachments.present? + original_article.clone_attachments('Ticket::Article', message.id, only_attached_attachments: true) + end + if original_article&.should_clone_inline_attachments? # rubocop:disable Style/GuardClause original_article.clone_attachments('Ticket::Article', message.id, only_inline_attachments: true) original_article.should_clone_inline_attachments = false # cancel the temporary flag after cloning diff --git a/public/assets/tests/form_extended.js b/public/assets/tests/form_extended.js index 7dda19d27..b4ce7be80 100644 --- a/public/assets/tests/form_extended.js +++ b/public/assets/tests/form_extended.js @@ -413,7 +413,8 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", - internal: 'false' + internal: 'false', + include_attachments: 'false', }, }, } @@ -486,7 +487,8 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", - internal: 'false' + internal: 'false', + include_attachments: 'false', }, }, } @@ -548,6 +550,7 @@ test('form checks', function() { subject: 'some subject', body: "some
\nbody", internal: 'false', + include_attachments: 'false', }, }, } @@ -660,7 +663,8 @@ test('form checks', function() { recipient: 'ticket_owner', subject: 'some subject', body: 'lala', - internal: 'false' + internal: 'false', + include_attachments: 'false', }, }, } @@ -681,7 +685,8 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", - internal: 'false' + internal: 'false', + include_attachments: 'false', }, }, } @@ -711,6 +716,7 @@ test('form checks', function() { subject: 'some subject', body: "some
\nbody", internal: 'false', + include_attachments: 'false', }, }, } @@ -730,7 +736,8 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", - internal: 'false' + internal: 'false', + include_attachments: 'true', }, }, } @@ -758,7 +765,8 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", - internal: 'false' + internal: 'false', + include_attachments: 'true', }, }, } diff --git a/public/assets/tests/form_ticket_perform_action.js b/public/assets/tests/form_ticket_perform_action.js index 8c7d43c53..685a39e80 100644 --- a/public/assets/tests/form_ticket_perform_action.js +++ b/public/assets/tests/form_ticket_perform_action.js @@ -51,7 +51,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, }, ticket_perform_action3: { @@ -103,7 +104,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.priority_id': { value: '2' @@ -139,7 +141,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.priority_id': { value: '2' @@ -153,7 +156,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: 'ticket_owner', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.state_id': { value: '3' @@ -177,7 +181,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: 'ticket_customer', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.priority_id': { value: '2' @@ -191,7 +196,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: 'ticket_owner', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.state_id': { value: '3' @@ -215,7 +221,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'true', recipient: 'ticket_customer', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.priority_id': { value: '2' @@ -229,7 +236,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: 'ticket_owner', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.state_id': { value: '3' @@ -262,7 +270,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'true', recipient: 'ticket_customer', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.priority_id': { value: '2' @@ -276,7 +285,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: 'ticket_owner', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.pending_time': { operator: 'static', @@ -314,7 +324,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'true', recipient: 'ticket_customer', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.priority_id': { value: '2' @@ -328,7 +339,8 @@ test( "ticket_perform_action check", function(assert) { body: 'some body', internal: 'false', recipient: 'ticket_owner', - subject: 'some subject' + subject: 'some subject', + include_attachments: 'false', }, 'ticket.pending_time': { operator: 'relative', @@ -354,7 +366,8 @@ test( "ticket_perform_action backwards check after issue #2782", function() { 'notification.email': { body: 'some body', recipient: ['ticket_owner', 'ticket_customer'], - subject: 'some subject' + subject: 'some subject', + include_attachments: 'true', }, }, } @@ -382,7 +395,8 @@ test( "ticket_perform_action backwards check after issue #2782", function() { body: 'some body', internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], - subject: 'some subject' + subject: 'some subject', + include_attachments: 'true', }, } } diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index be495245a..e7223b9e5 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -520,6 +520,117 @@ RSpec.describe Ticket, type: :model do include_examples 'verify log visibility status' end + shared_examples 'add a new article' do + it 'adds a new article' do + expect { ticket.perform_changes(performable, 'trigger', ticket, user) } + .to change { ticket.articles.count }.by(1) + end + end + + shared_examples 'add attachment to new article' do + include_examples 'add a new article' + + it 'adds attachment to the new article' do + ticket.perform_changes(performable, 'trigger', ticket, user) + article = ticket.articles.last + + expect(article.type.name).to eq('email') + expect(article.sender.name).to eq('System') + expect(article.attachments.count).to eq(1) + expect(article.attachments[0].filename).to eq('some_file.pdf') + expect(article.attachments[0].preferences['Content-ID']).to eq('image/pdf@01CAB192.K8H512Y9') + end + end + + shared_examples 'does not add attachment to new article' do + include_examples 'add a new article' + + it 'does not add attachment to the new article' do + ticket.perform_changes(performable, 'trigger', ticket, user) + article = ticket.articles.last + + expect(article.type.name).to eq('email') + expect(article.sender.name).to eq('System') + expect(article.attachments.count).to eq(0) + end + end + + context 'dispatching email with include attachment present' do + let(:notification_type) { :email } + let(:additional_options) do + { + notification_key => { + include_attachments: 'true' + } + } + end + + context 'when ticket has an attachment' do + + before do + UserInfo.current_user_id = 1 + ticket_article = create(:ticket_article, ticket: ticket) + + Store.add( + object: 'Ticket::Article', + o_id: ticket_article.id, + data: 'dGVzdCAxMjM=', + filename: 'some_file.pdf', + preferences: { + 'Content-Type': 'image/pdf', + 'Content-ID': 'image/pdf@01CAB192.K8H512Y9', + }, + created_by_id: 1, + ) + end + + include_examples 'add attachment to new article' + end + + context 'when ticket does not have an attachment' do + + include_examples 'does not add attachment to new article' + end + end + + context 'dispatching email with include attachment not present' do + let(:notification_type) { :email } + let(:additional_options) do + { + notification_key => { + include_attachments: 'false' + } + } + end + + context 'when ticket has an attachment' do + + before do + UserInfo.current_user_id = 1 + ticket_article = create(:ticket_article, ticket: ticket) + + Store.add( + object: 'Ticket::Article', + o_id: ticket_article.id, + data: 'dGVzdCAxMjM=', + filename: 'some_file.pdf', + preferences: { + 'Content-Type': 'image/pdf', + 'Content-ID': 'image/pdf@01CAB192.K8H512Y9', + }, + created_by_id: 1, + ) + end + + include_examples 'does not add attachment to new article' + end + + context 'when ticket does not have an attachment' do + + include_examples 'does not add attachment to new article' + end + end + context 'dispatching SMS' do let(:notification_type) { :sms } diff --git a/spec/models/trigger_spec.rb b/spec/models/trigger_spec.rb index 8caa52494..a16eecbb9 100644 --- a/spec/models/trigger_spec.rb +++ b/spec/models/trigger_spec.rb @@ -55,6 +55,135 @@ RSpec.describe Trigger, type: :model do } end + shared_examples 'include ticket attachment' do + context 'notification.email include_attachments' do + let(:perform) do + { + 'notification.email' => { + 'recipient' => 'ticket_customer', + 'subject' => 'Example subject', + 'body' => 'Example body', + } + }.deep_merge(additional_options).deep_stringify_keys + end + + let(:ticket) { create(:ticket) } + + shared_examples 'add a new article' do + it 'adds a new article' do + expect { TransactionDispatcher.commit } + .to change(ticket.articles, :count).by(1) + end + end + + shared_examples 'add attachment to new article' do + include_examples 'add a new article' + + it 'adds attachment to the new article' do + ticket && trigger + + TransactionDispatcher.commit + article = ticket.articles.last + + expect(article.type.name).to eq('email') + expect(article.sender.name).to eq('System') + expect(article.attachments.count).to eq(1) + expect(article.attachments[0].filename).to eq('some_file.pdf') + expect(article.attachments[0].preferences['Content-ID']).to eq('image/pdf@01CAB192.K8H512Y9') + end + end + + shared_examples 'does not add attachment to new article' do + include_examples 'add a new article' + + it 'does not add attachment to the new article' do + ticket && trigger + + TransactionDispatcher.commit + article = ticket.articles.last + + expect(article.type.name).to eq('email') + expect(article.sender.name).to eq('System') + expect(article.attachments.count).to eq(0) + end + end + + context 'with include attachment present' do + let(:additional_options) do + { + 'notification.email' => { + include_attachments: 'true' + } + } + end + + context 'when ticket has an attachment' do + + before do + UserInfo.current_user_id = 1 + ticket_article = create(:ticket_article, ticket: ticket) + + Store.add( + object: 'Ticket::Article', + o_id: ticket_article.id, + data: 'dGVzdCAxMjM=', + filename: 'some_file.pdf', + preferences: { + 'Content-Type': 'image/pdf', + 'Content-ID': 'image/pdf@01CAB192.K8H512Y9', + }, + created_by_id: 1, + ) + end + + include_examples 'add attachment to new article' + end + + context 'when ticket does not have an attachment' do + + include_examples 'does not add attachment to new article' + end + end + + context 'with include attachment not present' do + let(:additional_options) do + { + 'notification.email' => { + include_attachments: 'false' + } + } + end + + context 'when ticket has an attachment' do + + before do + UserInfo.current_user_id = 1 + ticket_article = create(:ticket_article, ticket: ticket) + + Store.add( + object: 'Ticket::Article', + o_id: ticket_article.id, + data: 'dGVzdCAxMjM=', + filename: 'some_file.pdf', + preferences: { + 'Content-Type': 'image/pdf', + 'Content-ID': 'image/pdf@01CAB192.K8H512Y9', + }, + created_by_id: 1, + ) + end + + include_examples 'does not add attachment to new article' + end + + context 'when ticket does not have an attachment' do + + include_examples 'does not add attachment to new article' + end + end + end + end + context 'for condition "ticket created"' do let(:condition) do { 'ticket.action' => { 'operator' => 'is', 'value' => 'create' } } @@ -417,6 +546,8 @@ RSpec.describe Trigger, type: :model do end end end + + include_examples 'include ticket attachment' end context 'for condition "ticket updated"' do @@ -575,6 +706,8 @@ RSpec.describe Trigger, type: :model do expect(Ticket::Article.where(ticket: ticket).last.to).not_to eq(system_address.email) end end + + include_examples 'include ticket attachment' end end