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 3fa4725e0..999881ae5 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 @@ -364,6 +364,19 @@ class App.UiElement.ticket_perform_action meta: meta || {} )) notificationElement.find('.js-recipient select').replaceWith(selection) + + visibilitySelection = App.UiElement.select.render( + name: "#{name}::internal" + multiple: false + null: false + options: { true: 'internal', false: 'public' } + value: meta.internal || 'false' + class: 'form-control--small' + translate: true + ) + + notificationElement.find('.js-internal').html(visibilitySelection) + notificationElement.find('.js-body div[contenteditable="true"]').ce( mode: 'richtext' placeholder: 'message' diff --git a/app/assets/javascripts/app/views/generic/ticket_perform_action/notification.jst.eco b/app/assets/javascripts/app/views/generic/ticket_perform_action/notification.jst.eco index d81224181..4871b4b18 100644 --- a/app/assets/javascripts/app/views/generic/ticket_perform_action/notification.jst.eco +++ b/app/assets/javascripts/app/views/generic/ticket_perform_action/notification.jst.eco @@ -1,4 +1,5 @@
+
<%- @Icon('arrow-down', 'dropdown-arrow') %> diff --git a/app/models/ticket.rb b/app/models/ticket.rb index cfd11bfde..fda7cfc83 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -1481,7 +1481,7 @@ result subject: subject, content_type: 'text/html', body: body, - internal: false, + internal: value['internal'] || false, # default to public if value was not set sender: Ticket::Article::Sender.find_by(name: 'System'), type: Ticket::Article::Type.find_by(name: 'email'), preferences: { @@ -1569,7 +1569,7 @@ result subject: 'SMS notification', to: sms_recipients_to, body: body, - internal: true, + internal: value['internal'] || false, # default to public if value was not set sender: Ticket::Article::Sender.find_by(name: 'System'), type: Ticket::Article::Type.find_by(name: 'sms'), preferences: { diff --git a/public/assets/tests/form_extended.js b/public/assets/tests/form_extended.js index 6e88b8a08..badcce98e 100644 --- a/public/assets/tests/form_extended.js +++ b/public/assets/tests/form_extended.js @@ -407,6 +407,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false' }, }, } @@ -479,6 +480,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false' }, }, } @@ -539,6 +541,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false', }, }, } @@ -651,6 +654,7 @@ test('form checks', function() { recipient: 'ticket_owner', subject: 'some subject', body: 'lala', + internal: 'false' }, }, } @@ -671,6 +675,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false' }, }, } @@ -699,6 +704,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false', }, }, } @@ -718,6 +724,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false' }, }, } @@ -745,6 +752,7 @@ test('form checks', function() { recipient: 'ticket_customer', subject: 'some subject', body: "some
\nbody", + internal: 'false' }, }, } diff --git a/public/assets/tests/form_ticket_perform_action.js b/public/assets/tests/form_ticket_perform_action.js index 3ffa674e2..3c8045e02 100644 --- a/public/assets/tests/form_ticket_perform_action.js +++ b/public/assets/tests/form_ticket_perform_action.js @@ -49,6 +49,7 @@ test( "ticket_perform_action check", function() { }, 'notification.email': { body: 'some body', + internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], subject: 'some subject' }, @@ -100,6 +101,7 @@ test( "ticket_perform_action check", function() { ticket_perform_action2: { 'notification.email': { body: 'some body', + internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], subject: 'some subject' }, @@ -135,6 +137,7 @@ test( "ticket_perform_action check", function() { ticket_perform_action2: { 'notification.email': { body: 'some body', + internal: 'false', recipient: ['ticket_owner', 'ticket_customer'], subject: 'some subject' }, @@ -148,6 +151,7 @@ test( "ticket_perform_action check", function() { ticket_perform_action3: { 'notification.email': { body: 'some body', + internal: 'false', recipient: 'ticket_owner', subject: 'some subject' }, @@ -171,6 +175,7 @@ test( "ticket_perform_action check", function() { ticket_perform_action2: { 'notification.email': { body: 'some body', + internal: 'false', recipient: 'ticket_customer', subject: 'some subject' }, @@ -184,6 +189,7 @@ test( "ticket_perform_action check", function() { ticket_perform_action3: { 'notification.email': { body: 'some body', + internal: 'false', recipient: 'ticket_owner', subject: 'some subject' }, @@ -194,4 +200,88 @@ test( "ticket_perform_action check", function() { } deepEqual(params, test_params, 'form param check') + // set notification to internal + $('[data-attribute-name="ticket_perform_action2"] .js-internal select').val('true').trigger('change') + + params = App.ControllerForm.params(el) + test_params = { + ticket_perform_action1: { + 'ticket.state_id': { + value: '2' + } + }, + ticket_perform_action2: { + 'notification.email': { + body: 'some body', + internal: 'true', + recipient: 'ticket_customer', + subject: 'some subject' + }, + 'ticket.priority_id': { + value: '2' + }, + 'ticket.state_id': { + value: '1' + }, + }, + ticket_perform_action3: { + 'notification.email': { + body: 'some body', + internal: 'false', + recipient: 'ticket_owner', + subject: 'some subject' + }, + 'ticket.state_id': { + value: '3' + } + } + } + deepEqual(params, test_params, 'form param check') +}); + +// Test for backwards compatibility after issue is fixed https://github.com/zammad/zammad/issues/2782 +test( "ticket_perform_action backwards check after issue #2782", function() { + $('#forms').append('

ticket_perform_action check

') + + var el = $('#form2') + + var defaults = { + ticket_perform_action5: { + 'notification.email': { + body: 'some body', + recipient: ['ticket_owner', 'ticket_customer'], + subject: 'some subject' + }, + }, + } + + new App.ControllerForm({ + el: el, + model: { + configure_attributes: [ + { + name: 'ticket_perform_action5', + display: 'TicketPerformAction5', + tag: 'ticket_perform_action', + null: true, + }, + ] + }, + params: defaults, + autofocus: true + }) + + var params = App.ControllerForm.params(el) + var test_params = { + ticket_perform_action5: { + 'notification.email': { + body: 'some body', + internal: 'false', + recipient: ['ticket_owner', 'ticket_customer'], + subject: 'some subject' + }, + } + } + + deepEqual(params, test_params, 'form param check') }); diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 0be3fe940..c37a1a148 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -210,6 +210,95 @@ RSpec.describe Ticket, type: :model do end end end + + context 'with a notification trigger' do + # https://github.com/zammad/zammad/issues/2782 + # + # Notification triggers should log notification as private or public + # according to given configuration + let(:user) { create(:admin_user, mobile: '+37061010000') } + + before { ticket.group.users << user } + + let(:perform) do + { + notification_key => { + body: 'Old programmers never die. They just branch to a new address.', + recipient: 'ticket_agents', + subject: 'Old programmers never die. They just branch to a new address.' + } + }.deep_merge(additional_options).deep_stringify_keys + end + + let(:notification_key) { "notification.#{notification_type}" } + + shared_examples 'verify log visibility status' do + shared_examples 'notification trigger' do + it 'adds Ticket::Article' do + expect { ticket.perform_changes(perform, 'trigger', ticket, user) } + .to change { ticket.articles.count }.by(1) + end + + it 'new Ticket::Article visibility reflects setting' do + ticket.perform_changes(perform, 'trigger', ticket, User.first) + new_article = ticket.articles.reload.last + expect(new_article.internal).to be target_internal_value + end + end + + context 'when set to private' do + let(:additional_options) do + { + notification_key => { + internal: true + } + } + end + + let(:target_internal_value) { true } + + it_behaves_like 'notification trigger' + end + + context 'when set to internal' do + let(:additional_options) do + { + notification_key => { + internal: false + } + } + end + + let(:target_internal_value) { false } + + it_behaves_like 'notification trigger' + end + + context 'when no selection was made' do # ensure previously created triggers default to public + let(:additional_options) do + {} + end + + let(:target_internal_value) { false } + + it_behaves_like 'notification trigger' + end + end + + context 'dispatching email' do + let(:notification_type) { :email } + + include_examples 'verify log visibility status' + end + + context 'dispatching SMS' do + let(:notification_type) { :sms } + + before { create(:channel, area: 'Sms::Notification') } + + include_examples 'verify log visibility status' + end + end end describe '#access?' do