From 250d0bbd5ed787ec2bd18af0739aaca9d665daca Mon Sep 17 00:00:00 2001 From: Romit Choudhary Date: Mon, 20 Dec 2021 15:01:34 +0100 Subject: [PATCH] Fixes #3129 - Deactivation of signature does not clear it from groups --- .../app/controllers/_ui_element/select.coffee | 37 ++++++++++++++++++- .../form_handler_signature.coffee | 2 +- .../article_action/email_reply.coffee | 2 +- .../javascripts/app/models/group.coffee | 5 ++- .../app/views/generic/select.jst.eco | 22 ++++++----- i18n/zammad.pot | 4 ++ spec/system/manage/groups_spec.rb | 29 +++++++++++++++ 7 files changed, 87 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_ui_element/select.coffee b/app/assets/javascripts/app/controllers/_ui_element/select.coffee index 01b0692a3..13a830dd4 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/select.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/select.coffee @@ -35,8 +35,43 @@ class App.UiElement.select extends App.UiElement.ApplicationUiElement # filter attributes @filterOption(attribute, params) + item = $( App.view('generic/select')(attribute: attribute) ) + + # bind event listeners + @bindEventListeners(item, attribute, params) + # return item - $( App.view('generic/select')(attribute: attribute) ) + item + + @bindEventListeners: (item, attribute, params) -> + if attribute.display_warn + item.bind('change', (e) => + @bindWarnDisplayListener(e.target.value, attribute, params, item) + ) + + # initialization for default selection + @bindWarnDisplayListener(attribute.value, attribute, params, item) + + @bindWarnDisplayListener: (selectedVal, attribute, params, item) -> + warn_visible = @shouldDisplayWarn(selectedVal, attribute, params) + @toggleDisplayWarn(warn_visible, attribute, item) + + @shouldDisplayWarn: (selectedVal, attribute, params) -> + return if !selectedVal + return if !params + + params[attribute.name + '_is_display_warning'](selectedVal) + + @toggleDisplayWarn: (warn_visible, attribute, item) -> + if !warn_visible + item.removeClass('display-warn') + item.find('.alert--warning').remove() + return + + item.addClass('display-warn') + warn_elem = $('') + warn_elem.html(attribute.warn) + item.append(warn_elem) # 1. If attribute.value is not among the current options, then search within historical options # 2. If attribute.value is not among current and historical options, then add the value itself as an option diff --git a/app/assets/javascripts/app/controllers/agent_ticket_create/form_handler_signature.coffee b/app/assets/javascripts/app/controllers/agent_ticket_create/form_handler_signature.coffee index 3bf7d56cc..8b1c8f6f7 100644 --- a/app/assets/javascripts/app/controllers/agent_ticket_create/form_handler_signature.coffee +++ b/app/assets/javascripts/app/controllers/agent_ticket_create/form_handler_signature.coffee @@ -11,7 +11,7 @@ class TicketCreateFormHandlerSignature # check if signature needs to be added type = ui.el.closest('.content').find('[name="formSenderType"]').val() - if signature && signature.body && type is 'email-out' + if signature && signature.active && signature.body && type is 'email-out' signatureFinished = App.Utils.replaceTags(signature.body, { user: App.Session.get(), config: App.Config.all() }) currentBody = ui.el.closest('.content').find('[data-name=body]') 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 f77657f0b..3140ef3a7 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 @@ -339,7 +339,7 @@ class EmailReply extends App.Controller signature = App.Signature.find(group.signature_id) # add/replace signature - if signature && signature.body + if signature && signature.active && signature.body # if signature has changed, remove it signature_id = ui.$('[data-signature=true]').data('signature-id') diff --git a/app/assets/javascripts/app/models/group.coffee b/app/assets/javascripts/app/models/group.coffee index 43f6448ef..43d50af87 100644 --- a/app/assets/javascripts/app/models/group.coffee +++ b/app/assets/javascripts/app/models/group.coffee @@ -9,7 +9,7 @@ class App.Group extends App.Model { 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 }, + { name: 'signature_id', display: __('Signature'), tag: 'select', multiple: false, null: true, relation: 'Signature', nulloption: true, do_not_log: true, display_warn: true, warn: __('This signature is inactive, it won\'t be included in the reply. Change state here') }, { name: 'note', display: __('Note'), tag: 'textarea', note: __('Notes are visible to agents only, never to customers.'), limit: 250, null: true }, { name: 'updated_at', display: __('Updated'), tag: 'datetime', readonly: 1 }, { name: 'active', display: __('Active'), tag: 'active', default: true }, @@ -43,3 +43,6 @@ class App.Group extends App.Model change: 'Change' overview: 'Overview' full: 'Full' + + signature_id_is_display_warning: (signature_id) -> + !App.Signature.find(signature_id).active diff --git a/app/assets/javascripts/app/views/generic/select.jst.eco b/app/assets/javascripts/app/views/generic/select.jst.eco index d60ebcc05..bc0420544 100644 --- a/app/assets/javascripts/app/views/generic/select.jst.eco +++ b/app/assets/javascripts/app/views/generic/select.jst.eco @@ -1,12 +1,14 @@
- - <% if not @attribute.multiple: %> - <%- @Icon('arrow-down') %> - <% end %> +
+ + <% if not @attribute.multiple: %> + <%- @Icon('arrow-down') %> + <% end %> +
diff --git a/i18n/zammad.pot b/i18n/zammad.pot index 252d5bd9d..fd47d198e 100644 --- a/i18n/zammad.pot +++ b/i18n/zammad.pot @@ -9016,6 +9016,10 @@ msgstr "" msgid "This service shows you contacts of incoming calls and a caller list in realtime." msgstr "" +#: app/assets/javascripts/app/models/group.coffee +msgid "This signature is inactive, it won't be included in the reply. Change state here" +msgstr "" + #: app/assets/javascripts/app/controllers/_application_controller/generic_history.coffee msgid "This ticket was merged into" msgstr "" diff --git a/spec/system/manage/groups_spec.rb b/spec/system/manage/groups_spec.rb index 703b30991..8608ff8eb 100644 --- a/spec/system/manage/groups_spec.rb +++ b/spec/system/manage/groups_spec.rb @@ -9,6 +9,35 @@ RSpec.describe 'Manage > Groups', type: :system do include_examples 'pagination', model: :group, klass: Group, path: 'manage/groups' end + # Fixes GitHub Issue#3129 - Deactivation of signature does not clear it from groups + describe 'When active status of signature assigned to a group is changed', authenticated_as: -> { user } do + let(:user) { create(:admin, groups: [group]) } + let(:group) { create(:group, signature_id: signature.id) } + let(:signature) { create(:signature) } + + it 'does not display warning, when signature is active' do + visit '#manage/groups' + + click "tr[data-id='#{group.id}']" + + expect(page).to have_select('signature_id', selected: signature.name) + .and have_no_css('.alert--warning') + end + + context 'When signature is marked inactive' do + let(:signature) { create(:signature, active: false) } + + it 'displays warning' do + visit '#manage/groups' + + click "tr[data-id='#{group.id}']" + + expect(page).to have_select('signature_id', selected: signature.name) + .and have_css('.alert--warning') + end + end + end + describe 'Core Workflow' do include_examples 'core workflow' do let(:object_name) { 'Group' }