Skip to content

Commit

Permalink
Fixes zammad#3129 - Deactivation of signature does not clear it from …
Browse files Browse the repository at this point in the history
…groups
  • Loading branch information
Romit Choudhary authored and mantas committed Dec 20, 2021
1 parent a5c7286 commit 250d0bb
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 14 deletions.
37 changes: 36 additions & 1 deletion app/assets/javascripts/app/controllers/_ui_element/select.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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 = $('<div class="alert alert--warning" role="alert"></div>')
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
5 changes: 4 additions & 1 deletion app/assets/javascripts/app/models/group.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href="#channels/email">here</a>') },
{ 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 },
Expand Down Expand Up @@ -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
22 changes: 12 additions & 10 deletions app/assets/javascripts/app/views/generic/select.jst.eco
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
<div class="controls controls--select">
<select <% if @attribute.id: %>id="<%= @attribute.id %>"<% end %> class="form-control<%= " #{ @attribute.class }" if @attribute.class %>" name="<%= @attribute.name %>" <%= @attribute.multiple %> <%= @attribute.required %> <%= @attribute.autofocus %> <% if @attribute.disabled: %> disabled<% end %>>
<% if @attribute.options: %>
<% for row in @attribute.options: %>
<option value="<%= row.value %>" <%= row.selected %> <%= row.disabled %>><%= row.name %></option>
<% end %>
<% end %>
</select>
<% if not @attribute.multiple: %>
<%- @Icon('arrow-down') %>
<% end %>
<div class='relative'>
<select <% if @attribute.id: %>id="<%= @attribute.id %>"<% end %> class="form-control<%= " #{ @attribute.class }" if @attribute.class %>" name="<%= @attribute.name %>" <%= @attribute.multiple %> <%= @attribute.required %> <%= @attribute.autofocus %> <% if @attribute.disabled: %> disabled<% end %>>
<% if @attribute.options: %>
<% for row in @attribute.options: %>
<option value="<%= row.value %>" <%= row.selected %> <%= row.disabled %>><%= row.name %></option>
<% end %>
<% end %>
</select>
<% if not @attribute.multiple: %>
<%- @Icon('arrow-down') %>
<% end %>
</div>
</div>
4 changes: 4 additions & 0 deletions i18n/zammad.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href=\"#channels/email\">here</a>"
msgstr ""

#: app/assets/javascripts/app/controllers/_application_controller/generic_history.coffee
msgid "This ticket was merged into"
msgstr ""
Expand Down
29 changes: 29 additions & 0 deletions spec/system/manage/groups_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,35 @@
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' }
Expand Down

0 comments on commit 250d0bb

Please sign in to comment.