From 5b0b10ce596e617ff40e733ad14e8659682bf8d0 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 22 May 2018 00:57:26 +0200 Subject: [PATCH] Fixed issue #1930 - bulk action is not executed for a pending-state. --- .../app/controllers/ticket_overview.coffee | 123 ++++++++--- .../app/lib/bootstrap/bootstrap-datepicker.js | 11 +- .../app/views/generic/date.jst.eco | 2 +- .../app/views/generic/datetime.jst.eco | 2 +- app/assets/stylesheets/zammad.scss | 54 ++++- .../agent_ticket_overview_level0_test.rb | 206 ++++++++++++++++-- 6 files changed, 339 insertions(+), 59 deletions(-) diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index 66f654c0a..420f4e9f8 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/app/assets/javascripts/app/controllers/ticket_overview.coffee @@ -1162,9 +1162,10 @@ class Table extends App.Controller # start organization popups @organizationPopups() - @bulkForm = new BulkForm + @bulkForm = new BulkForm( holder: @el view: @view + ) # start bulk action observ @el.append(@bulkForm.el) @@ -1226,12 +1227,16 @@ class BulkForm extends App.Controller constructor: -> super - @configure_attributes_ticket = [ - { name: 'state_id', display: 'State', tag: 'select', multiple: false, null: true, relation: 'TicketState', translate: true, nulloption: true, default: '' }, - { name: 'priority_id', display: 'Priority', tag: 'select', multiple: false, null: true, relation: 'TicketPriority', translate: true, nulloption: true, default: '' }, - { name: 'group_id', display: 'Group', tag: 'select', multiple: false, null: true, relation: 'Group', nulloption: true }, - { name: 'owner_id', display: 'Owner', tag: 'select', multiple: false, null: true, relation: 'User', nulloption: true } - ] + @configure_attributes_ticket = [] + used_attributes = ['state_id', 'pending_time', 'priority_id', 'group_id', 'owner_id'] + attributesClean = App.Ticket.attributesGet('edit') + for attributeName, attribute of attributesClean + if _.contains(used_attributes, attributeName) + localAttribute = clone(attribute) + localAttribute.nulloption = true + localAttribute.default = '' + localAttribute.null = true + @configure_attributes_ticket.push localAttribute @holder = @options.holder @visible = false @@ -1246,9 +1251,9 @@ class BulkForm extends App.Controller App.TicketCreateCollection.unbind(@bindId) render: -> - @el.css 'right', App.Utils.getScrollBarWidth() + @el.css('right', App.Utils.getScrollBarWidth()) - @html App.view('agent_ticket_view/bulk')() + @html(App.view('agent_ticket_view/bulk')()) handlers = @Config.get('TicketZoomFormHandler') @@ -1304,12 +1309,15 @@ class BulkForm extends App.Controller setTimeout ( => @$('.textarea.form-group textarea').focus() ), 0 reset: => - @$('.js-action-step').removeClass('hide') - @$('.js-confirm-step').addClass('hide') + @cancel() if @visible @makeSpaceForTableRows() + cancel: => + @$('.js-action-step').removeClass('hide') + @$('.js-confirm-step').addClass('hide') + show: => @el.removeClass('hide') @visible = true @@ -1325,30 +1333,84 @@ class BulkForm extends App.Controller scrollParent = @holder.scrollParent() isScrolledToBottom = scrollParent.prop('scrollHeight') is scrollParent.scrollTop() + scrollParent.outerHeight() - @holder.css 'margin-bottom', height + @holder.css('margin-bottom', height) if isScrolledToBottom scrollParent.scrollTop scrollParent.prop('scrollHeight') - scrollParent.outerHeight() removeSpaceForTableRows: => - @holder.css 'margin-bottom', 0 + @holder.css('margin-bottom', 0) + + ticketMergeParams: (params) -> + ticketUpdate = {} + for item of params + if params[item] != '' && params[item] != null + ticketUpdate[item] = params[item] + + # in case if a group is selected, set also the selected owner (maybe nobody) + if params.group_id != '' && params.group_id != null + ticketUpdate.owner_id = params.owner_id + ticketUpdate submit: (e) => e.preventDefault() - @bulk_count = @holder.find('.table-overview').find('[name="bulk"]:checked').length - @bulk_count_index = 0 - @holder.find('.table-overview').find('[name="bulk"]:checked').each( (index, element) => - @log 'notice', '@bulk_count_index', @bulk_count, @bulk_count_index + @bulkCount = @holder.find('.table-overview').find('[name="bulk"]:checked').length + + if @bulkCount is 0 + App.Event.trigger 'notify', { + type: 'error' + msg: App.i18n.translateContent('At least one object must be selected.') + } + return + + ticket_ids = [] + @holder.find('.table-overview').find('[name="bulk"]:checked').each( (index, element) -> ticket_id = $(element).val() + ticket_ids.push ticket_id + ) + + params = @formParam(e.target) + + for ticket_id in ticket_ids + ticket = App.Ticket.find(ticket_id) + + ticketUpdate = @ticketMergeParams(params) + ticket.load(ticketUpdate) + + # if title is empty - ticket can't processed, set ? + if _.isEmpty(ticket.title) + ticket.title = '-' + + # validate ticket + errors = ticket.validate( + screen: 'edit' + ) + if errors + @log 'error', 'update', errors + errorString = '' + for key, error of errors + errorString += "#{key}: #{error}" + + @formValidate( + form: e.target + errors: errors + screen: 'edit' + ) + + App.Event.trigger 'notify', { + type: 'error' + msg: App.i18n.translateContent('Bulk action stopped %s!', errorString) + } + @cancel() + return + + @bulkCountIndex = 0 + for ticket_id in ticket_ids ticket = App.Ticket.find(ticket_id) - params = @formParam(e.target) # update ticket - ticket_update = {} - for item of params - if params[item] != '' - ticket_update[item] = params[item] + ticketUpdate = @ticketMergeParams(params) # validate article if params['body'] @@ -1372,7 +1434,7 @@ class BulkForm extends App.Controller @formEnable(e) return - ticket.load(ticket_update) + ticket.load(ticketUpdate) # if title is empty - ticket can't processed, set ? if _.isEmpty(ticket.title) @@ -1380,7 +1442,7 @@ class BulkForm extends App.Controller ticket.save( done: (r) => - @bulk_count_index++ + @bulkCountIndex++ # reset form after save if article @@ -1390,13 +1452,22 @@ class BulkForm extends App.Controller ) # refresh view after all tickets are proceeded - if @bulk_count_index == @bulk_count + if @bulkCountIndex == @bulkCount + @render() @hide() # fetch overview data again App.Event.trigger('overview:fetch') + + fail: (r) => + @bulkCountIndex++ + @log 'error', 'update ticket', r + App.Event.trigger 'notify', { + type: 'error' + msg: App.i18n.translateContent('Can\'t update Ticket %s!', ticket.number) + } ) - ) + @holder.find('.table-overview').find('[name="bulk"]:checked').prop('checked', false) App.Event.trigger 'notify', { type: 'success' diff --git a/app/assets/javascripts/app/lib/bootstrap/bootstrap-datepicker.js b/app/assets/javascripts/app/lib/bootstrap/bootstrap-datepicker.js index 3e56e7700..7e0f13eb8 100755 --- a/app/assets/javascripts/app/lib/bootstrap/bootstrap-datepicker.js +++ b/app/assets/javascripts/app/lib/bootstrap/bootstrap-datepicker.js @@ -27,6 +27,7 @@ - fix that place method doesn't think that the container is the window, but rather the real window is the window - added rerender method to show correct today if task is longer open the 24 hours - scroll into view + - fix vertical auto position */ (function(factory){ @@ -689,7 +690,9 @@ visualPadding = 10, container = $(this.o.container), windowWidth = $(window).width(), - scrollTop = container.scrollTop(), + scrollTop = container.scrollParent().scrollTop(), + bottomEdge = container.offset().top + container.height(), + scrollHeight = container.scrollParent().prop('scrollHeight'), appendOffset = container.offset(); var parentsZindex = []; @@ -738,10 +741,10 @@ // auto y orientation is best-situation: top or bottom, no fudging, // decision based on which shows more of the calendar var yorient = this.o.orientation.y, - top_overflow; + space_below; if (yorient === 'auto'){ - top_overflow = -scrollTop + top - calendarHeight; - yorient = top_overflow < 0 ? 'bottom' : 'top'; + space_below = scrollHeight - bottomEdge - scrollTop; + yorient = space_below > calendarHeight ? 'bottom' : 'top'; } this.picker.addClass('datepicker-orient-' + yorient); diff --git a/app/assets/javascripts/app/views/generic/date.jst.eco b/app/assets/javascripts/app/views/generic/date.jst.eco index 2318af8ba..b12a620ea 100644 --- a/app/assets/javascripts/app/views/generic/date.jst.eco +++ b/app/assets/javascripts/app/views/generic/date.jst.eco @@ -1,4 +1,4 @@ -
+
\ No newline at end of file diff --git a/app/assets/javascripts/app/views/generic/datetime.jst.eco b/app/assets/javascripts/app/views/generic/datetime.jst.eco index 51f281020..ac195d58d 100644 --- a/app/assets/javascripts/app/views/generic/datetime.jst.eco +++ b/app/assets/javascripts/app/views/generic/datetime.jst.eco @@ -1,4 +1,4 @@ -
+
<%- @T('at') %>
diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 3778bf2ec..013a61106 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -1634,6 +1634,15 @@ fieldset > .form-group { position: relative; } +.controls--datetime { + position: relative; + display: flex; +} + +.controls--date { + position: relative; +} + .controls-label { margin: 11px 10px 0; flex-shrink: 0; @@ -2980,6 +2989,11 @@ footer { align-items: center; } + .bulkAction-firstStep .has-error { + border-color: red !important; + border: 1px solid; + } + .bulkAction-secondStep { display: flex; flex-direction: column; @@ -4570,6 +4584,11 @@ footer { position: relative; height: 60px; flex: 1 1 auto; + + &.datetime { + min-width: 140px; + overflow: visible; // datepicker popup needs to be visible + } } .form-group.is-changed { @@ -4638,15 +4657,44 @@ footer { left: 0; top: 0; position: absolute; - padding: 28px 18px 12px; + padding: 28px 5px 12px 20px; float: none; display: block; border-radius: 0; background: none; } - .form-inline .controls--select { - position: static; + .form-inline { + .controls--datetime, + .controls--date, + .controls--select { + position: static; + } + + .controls--datetime { + position: absolute; + bottom: 12px; + left: 0; + padding: 0 5px 0 20px; + width: 100%; + + .controls-label { + display: none; + } + + .form-control { + width: 70px; + line-height: inherit; + position: static; + padding: 0; + height: auto; + + &.time { + margin-left: 5px; + width: 38px; + } + } + } } .bulkAction-secondStep .form-group { diff --git a/test/browser/agent_ticket_overview_level0_test.rb b/test/browser/agent_ticket_overview_level0_test.rb index 8c1fc1e72..0f2f9b038 100644 --- a/test/browser/agent_ticket_overview_level0_test.rb +++ b/test/browser/agent_ticket_overview_level0_test.rb @@ -2,7 +2,7 @@ require 'browser_test_helper' class AgentTicketOverviewLevel0Test < TestCase - def test_i + def test_bulk_close @browser = browser_instance login( username: 'master@example.com', @@ -38,56 +38,63 @@ class AgentTicketOverviewLevel0Test < TestCase ) click(text: 'Unassigned & Open') - sleep 8 # till overview is rendered + watch_for( + css: '.content.active', + value: 'overview count test #2', + ) # select both via bulk action click( - css: '.active table tr td input[value="' + ticket1[:id] + '"] + .icon-checkbox.icon-unchecked', + css: '.content.active table tr td input[value="' + ticket1[:id] + '"] + .icon-checkbox.icon-unchecked', fast: true, ) # scroll to reply - needed for chrome scroll_to( position: 'top', - css: '.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', ) click( - css: '.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', fast: true, ) exists( - css: '.active table tr td input[value="' + ticket1[:id] + '"][type="checkbox"]:checked', + css: '.content.active table tr td input[value="' + ticket1[:id] + '"][type="checkbox"]:checked', ) exists( - css: '.active table tr td input[value="' + ticket2[:id] + '"][type="checkbox"]:checked', + css: '.content.active table tr td input[value="' + ticket2[:id] + '"][type="checkbox"]:checked', ) # select close state & submit select( - css: '.active .bulkAction [name="state_id"]', + css: '.content.active .bulkAction [name="state_id"]', value: 'closed', ) click( - css: '.active .bulkAction .js-confirm', + css: '.content.active .bulkAction .js-confirm', ) click( - css: '.active .bulkAction .js-submit', + css: '.content.active .bulkAction .js-submit', + ) + + watch_for_disappear( + css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + timeout: 6, ) - sleep 6 exists_not( - css: '.active table tr td input[value="' + ticket1[:id] + '"]', + css: '.content.active table tr td input[value="' + ticket1[:id] + '"]', ) exists_not( - css: '.active table tr td input[value="' + ticket2[:id] + '"]', + css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', ) # remember current overview count overview_counter_before = overview_counter() # click options and enable number and article count - click(css: '.active [data-type="settings"]') + click(css: '.content.active [data-type="settings"]') watch_for( css: '.modal h1', @@ -116,15 +123,15 @@ class AgentTicketOverviewLevel0Test < TestCase # check if number and article count is shown match( - css: '.active table th:nth-child(3)', + css: '.content.active table th:nth-child(3)', value: '#', ) match( - css: '.active table th:nth-child(4)', + css: '.content.active table th:nth-child(4)', value: 'Title', ) match( - css: '.active table th:nth-child(7)', + css: '.content.active table th:nth-child(7)', value: 'Article#', ) @@ -134,20 +141,20 @@ class AgentTicketOverviewLevel0Test < TestCase # check if number and article count is shown match( - css: '.active table th:nth-child(3)', + css: '.content.active table th:nth-child(3)', value: '#', ) match( - css: '.active table th:nth-child(4)', + css: '.content.active table th:nth-child(4)', value: 'Title', ) match( - css: '.active table th:nth-child(7)', + css: '.content.active table th:nth-child(7)', value: 'Article#', ) # disable number and article count - click(css: '.active [data-type="settings"]') + click(css: '.content.active [data-type="settings"]') watch_for( css: '.modal h1', @@ -164,15 +171,15 @@ class AgentTicketOverviewLevel0Test < TestCase # check if number and article count is gone match_not( - css: '.active table th:nth-child(3)', + css: '.content.active table th:nth-child(3)', value: '#', ) match( - css: '.active table th:nth-child(3)', + css: '.content.active table th:nth-child(3)', value: 'Title', ) exists_not( - css: '.active table th:nth-child(8)', + css: '.content.active table th:nth-child(8)', ) # create new ticket @@ -211,4 +218,155 @@ class AgentTicketOverviewLevel0Test < TestCase # cleanup tasks_close_all() end + + def test_bulk_pending + @browser = browser_instance + login( + username: 'master@example.com', + password: 'test', + url: browser_url, + ) + tasks_close_all() + + # test bulk action + + # create new ticket + ticket1 = ticket_create( + data: { + customer: 'nico', + group: 'Users', + title: 'overview count test #3', + body: 'overview count test #3', + } + ) + ticket2 = ticket_create( + data: { + customer: 'nico', + group: 'Users', + title: 'overview count test #4', + body: 'overview count test #4', + } + ) + click(text: 'Overviews') + + # enable full overviews + execute( + js: '$(".content.active .sidebar").css("display", "block")', + ) + + click(text: 'Unassigned & Open') + watch_for( + css: '.content.active', + value: 'overview count test #4', + timeout: 8, + ) + + # remember current overview count + overview_counter_before = overview_counter() + + # select both via bulk action + click( + css: '.content.active table tr td input[value="' + ticket1[:id] + '"] + .icon-checkbox.icon-unchecked', + fast: true, + ) + + # scroll to reply - needed for chrome + scroll_to( + position: 'top', + css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + ) + click( + css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + fast: true, + ) + + exists( + css: '.content.active table tr td input[value="' + ticket1[:id] + '"][type="checkbox"]:checked', + ) + exists( + css: '.content.active table tr td input[value="' + ticket2[:id] + '"][type="checkbox"]:checked', + ) + + exists( + displayed: false, + css: '.content.active .bulkAction [data-name="pending_time"]', + ) + + select( + css: '.content.active .bulkAction [name="state_id"]', + value: 'pending close', + ) + + exists( + displayed: true, + css: '.content.active .bulkAction [data-name="pending_time"]', + ) + + set( + css: '.content.active .bulkAction [data-item="date"]', + value: '05/23/2088', + ) + + select( + css: '.content.active .bulkAction [name="group_id"]', + value: 'Users', + ) + + select( + css: '.content.active .bulkAction [name="owner_id"]', + value: 'Test Master Agent', + ) + + click( + css: '.content.active .bulkAction .js-confirm', + ) + click( + css: '.content.active .bulkAction .js-submit', + ) + + watch_for_disappear( + css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + timeout: 12, + ) + + exists_not( + css: '.content.active table tr td input[value="' + ticket1[:id] + '"]', + ) + exists_not( + css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + ) + + # get new overview count + overview_counter_new = overview_counter() + assert_equal(overview_counter_before['#ticket/view/all_unassigned'] - 2, overview_counter_new['#ticket/view/all_unassigned']) + + # open ticket by search + ticket_open_by_search( + number: ticket1[:number], + ) + sleep 1 + + # close ticket + ticket_update( + data: { + state: 'closed', + } + ) + + # open ticket by search + ticket_open_by_search( + number: ticket2[:number], + ) + sleep 1 + + # close ticket + ticket_update( + data: { + state: 'closed', + } + ) + + # cleanup + tasks_close_all() + end end