From 9c600bbfe53bc256fd7fdadc23db66904eface09 Mon Sep 17 00:00:00 2001 From: Billy Zhou Date: Mon, 6 Aug 2018 21:02:16 +0800 Subject: [PATCH 1/4] Fixed issue #1864 - Bulk-Action: Not possible to change owner (fixes #1864) --- .../app/controllers/ticket_overview.coffee | 30 +++++ .../owner_handler_dependencies.coffee | 26 ++++ .../agent_ticket_overview_level0_test.rb | 114 ++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index 7a70edd5d..e22924fd0 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/app/assets/javascripts/app/controllers/ticket_overview.coffee @@ -1089,6 +1089,7 @@ class Table extends App.Controller if @$('table').find('input[name="bulk"]:checked').length == 0 @bulkForm.hide() else + @bulkForm.render() @bulkForm.show() if @lastChecked && e.shiftKey @@ -1266,6 +1267,10 @@ class BulkForm extends App.Controller handlers = @Config.get('TicketZoomFormHandler') + for attribute in @configure_attributes_ticket + continue if attribute.name != 'owner_id' + attribute.alt_options = @userOptionsForSelection() + new App.ControllerForm( el: @$('#form-ticket-bulk') model: @@ -1302,6 +1307,31 @@ class BulkForm extends App.Controller noFieldset: true ) + userOptionsForSelection: => + items = $('.content.active .table-overview .table').find('[name="bulk"]:checked') + + # we want to display all users for which we can assign the tickets directly + # for this we need to get the groups of all selected tickets + # after we got those we need to check which users are available in all groups + # users that are not in all groups can't get the tickets assigned + ticket_ids = _.map(items, (el) -> $(el).val() ) + ticket_group_ids = _.map(App.Ticket.findAll(ticket_ids), (ticket) -> ticket.group_id) + users = @usersInGroups(ticket_group_ids) + users.map( (user) -> {value: user.id, name: user.displayName()} ) + + usersInGroups: (group_ids) -> + ids_by_group = _.chain(@formMeta?.dependencies?.group_id) + .pick(group_ids) + .values() + .map( (e) -> e.owner_id) + .value() + + # Underscore's intersection doesn't work when chained + ids_in_all_groups = _.intersection(ids_by_group...) + + users = App.User.findAll(ids_in_all_groups) + _.sortBy(users, (user) -> user.firstname) + articleTypeFilter: (items) -> for item in items if item.name is 'note' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee new file mode 100644 index 000000000..f657563d2 --- /dev/null +++ b/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee @@ -0,0 +1,26 @@ +class OwnerFormHandlerDependencies + + # central method, is getting called on every ticket form change + @run: (params, attribute, attributes, classname, form, ui) -> + return if 'group_id' not of params || 'owner_id' not of params + + owner_attribute = _.find(attributes, (o) -> o.name == 'owner_id') + return if !owner_attribute + return if 'alt_options' not of owner_attribute + + if !params.group_id + # if no group is chosen, then we use the alt_options to populate the owner_id field + owner_attribute.options = owner_attribute.alt_options + delete owner_attribute['relation'] + else + # if a group is chosen, then populate owner_id using attribute.relation + owner_attribute.relation = 'User' + delete owner_attribute['options'] + + # replace new option list + owner_attribute.default = params[owner_attribute.name] + owner_attribute.newValue = params[owner_attribute.name] + newElement = ui.formGenItem(owner_attribute, classname, form) + form.find('select[name="owner_id"]').closest('.form-group').replaceWith(newElement) + +App.Config.set('150-ticketFormChanges', OwnerFormHandlerDependencies, 'TicketZoomFormHandler') \ No newline at end of file diff --git a/test/browser/agent_ticket_overview_level0_test.rb b/test/browser/agent_ticket_overview_level0_test.rb index 1c72eef83..cd7a63b5d 100644 --- a/test/browser/agent_ticket_overview_level0_test.rb +++ b/test/browser/agent_ticket_overview_level0_test.rb @@ -365,4 +365,118 @@ class AgentTicketOverviewLevel0Test < TestCase # cleanup tasks_close_all() end + + # verify correct behaviour for issue #1864 - Bulk-Action: Not possible to change owner + def test_bulk_owner_change + @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 owner change test #1', + body: 'overview owner change #1', + } + ) + ticket2 = ticket_create( + data: { + customer: 'nico', + group: 'Users', + title: 'overview owner change #2', + body: 'overview owner change #2', + } + ) + + # open Overview menu tab + click( + css: '.js-menu .js-overviewsMenuItem', + ) + + # enable full overviews + execute( + js: '$(".content.active .sidebar").css("display", "block")', + ) + + # click Unassigned & Open tab + click( + css: '.content.active [href="#ticket/view/all_unassigned"]', + ) + + watch_for( + css: '.content.active', + value: 'overview owner change #2', + 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', + ) + + select( + css: '.content.active .bulkAction [name="owner_id"]', + value: 'Test Master Agent', + ) + + select( + css: '.content.active .bulkAction [name="state_id"]', + value: 'closed', + ) + + 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']) + + # cleanup + tasks_close_all() + end end From 54055bcc5d028bf21a8f59d8e0dc6c910c350924 Mon Sep 17 00:00:00 2001 From: Billy Zhou Date: Mon, 20 Aug 2018 19:53:44 +0800 Subject: [PATCH 2/4] Switch to overview_open --- test/browser/agent_ticket_overview_level0_test.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/browser/agent_ticket_overview_level0_test.rb b/test/browser/agent_ticket_overview_level0_test.rb index cd7a63b5d..2727f75fd 100644 --- a/test/browser/agent_ticket_overview_level0_test.rb +++ b/test/browser/agent_ticket_overview_level0_test.rb @@ -396,9 +396,8 @@ class AgentTicketOverviewLevel0Test < TestCase } ) - # open Overview menu tab - click( - css: '.js-menu .js-overviewsMenuItem', + overview_open( + link: '#ticket/view/all_unassigned', ) # enable full overviews @@ -406,11 +405,6 @@ class AgentTicketOverviewLevel0Test < TestCase js: '$(".content.active .sidebar").css("display", "block")', ) - # click Unassigned & Open tab - click( - css: '.content.active [href="#ticket/view/all_unassigned"]', - ) - watch_for( css: '.content.active', value: 'overview owner change #2', From d42d0ff469b066e37e90526cb486e0834e460e22 Mon Sep 17 00:00:00 2001 From: Billy Zhou Date: Tue, 21 Aug 2018 12:58:54 +0800 Subject: [PATCH 3/4] Addressed QA comments on merge request 8 --- .../app/controllers/ticket_overview.coffee | 117 ++++++++---------- .../owner_handler_dependencies.coffee | 18 +-- .../agent_ticket_overview_level0_test.rb | 5 - 3 files changed, 61 insertions(+), 79 deletions(-) diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index e22924fd0..edc64398a 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/app/assets/javascripts/app/controllers/ticket_overview.coffee @@ -1,3 +1,47 @@ +ValidUsersForTicketSelectionMethods = + validUsersForTicketSelection: -> + items = $('.content.active .table-overview .table').find('[name="bulk"]:checked') + + # we want to display all users for which we can assign the tickets directly + # for this we need to get the groups of all selected tickets + # after we got those we need to check which users are available in all groups + # users that are not in all groups can't get the tickets assigned + ticket_ids = _.map(items, (el) -> $(el).val() ) + ticket_group_ids = _.map(App.Ticket.findAll(ticket_ids), (ticket) -> ticket.group_id) + users = @usersInGroups(ticket_group_ids) + + # get the list of possible groups for the current user + # from the TicketCreateCollection + # (filled for e.g. the TicketCreation or TicketZoom assignment) + # and order them by name + group_ids = _.keys(@formMeta?.dependencies?.group_id) + groups = App.Group.findAll(group_ids) + groups_sorted = _.sortBy(groups, (group) -> group.name) + + # get the number of visible users per group + # from the TicketCreateCollection + # (filled for e.g. the TicketCreation or TicketZoom assignment) + for group in groups + group.valid_users_count = @formMeta?.dependencies?.group_id?[group.id]?.owner_id.length || 0 + + { + users: users + groups: groups_sorted + } + + usersInGroups: (group_ids) -> + ids_by_group = _.chain(@formMeta?.dependencies?.group_id) + .pick(group_ids) + .values() + .map( (e) -> e.owner_id) + .value() + + # Underscore's intersection doesn't work when chained + ids_in_all_groups = _.intersection(ids_by_group...) + + users = App.User.findAll(ids_in_all_groups) + _.sortBy(users, (user) -> user.firstname) + class App.TicketOverview extends App.Controller className: 'overviews' activeFocus: 'nav' @@ -25,6 +69,8 @@ class App.TicketOverview extends App.Controller 'mouseenter .js-batch-hover-target': 'highlightBatchEntry' 'mouseleave .js-batch-hover-target': 'unhighlightBatchEntry' + @include ValidUsersForTicketSelectionMethods + constructor: -> super @batchSupport = @permissionCheck('ticket.agent') @@ -540,19 +586,6 @@ class App.TicketOverview extends App.Controller .velocity({ opacity: [0.5, 1] }, { duration: 120 }) .velocity({ opacity: [1, 0.5] }, { duration: 60, delay: 40 }) - usersInGroups: (group_ids) -> - ids_by_group = _.chain(@formMeta?.dependencies?.group_id) - .pick(group_ids) - .values() - .map( (e) -> e.owner_id) - .value() - - # Underscore's intersection doesn't work when chained - ids_in_all_groups = _.intersection(ids_by_group...) - - users = App.User.findAll(ids_in_all_groups) - _.sortBy(users, (user) -> user.firstname) - render: -> elLocal = $(App.view('ticket_overview/index')()) @@ -604,33 +637,8 @@ class App.TicketOverview extends App.Controller @renderOptionsMacros() renderOptionsGroups: => - items = @el.find('[name="bulk"]:checked') - - # we want to display all users for which we can assign the tickets directly - # for this we need to get the groups of all selected tickets - # after we got those we need to check which users are available in all groups - # users that are not in all groups can't get the tickets assigned - ticket_ids = _.map(items, (el) -> $(el).val() ) - ticket_group_ids = _.map(App.Ticket.findAll(ticket_ids), (ticket) -> ticket.group_id) - users = @usersInGroups(ticket_group_ids) - - # get the list of possible groups for the current user - # from the TicketCreateCollection - # (filled for e.g. the TicketCreation or TicketZoom assignment) - # and order them by name - group_ids = _.keys(@formMeta?.dependencies?.group_id) - groups = App.Group.findAll(group_ids) - groups_sorted = _.sortBy(groups, (group) -> group.name) - - # get the number of visible users per group - # from the TicketCreateCollection - # (filled for e.g. the TicketCreation or TicketZoom assignment) - for group in groups - group.valid_users_count = @formMeta?.dependencies?.group_id?[group.id]?.owner_id.length || 0 - @batchAssignInner.html $(App.view('ticket_overview/batch_overlay_user_group')( - users: users - groups: groups_sorted + @validUsersForTicketSelection() )) renderOptionsMacros: => @@ -1234,6 +1242,8 @@ class BulkForm extends App.Controller 'click .js-confirm': 'confirm' 'click .js-cancel': 'reset' + @include ValidUsersForTicketSelectionMethods + constructor: -> super @@ -1269,7 +1279,9 @@ class BulkForm extends App.Controller for attribute in @configure_attributes_ticket continue if attribute.name != 'owner_id' - attribute.alt_options = @userOptionsForSelection() + {users, groups} = @validUsersForTicketSelection() + options = _.map(users, (user) -> {value: user.id, name: user.displayName()} ) + attribute.alternative_user_options = options new App.ControllerForm( el: @$('#form-ticket-bulk') @@ -1307,31 +1319,6 @@ class BulkForm extends App.Controller noFieldset: true ) - userOptionsForSelection: => - items = $('.content.active .table-overview .table').find('[name="bulk"]:checked') - - # we want to display all users for which we can assign the tickets directly - # for this we need to get the groups of all selected tickets - # after we got those we need to check which users are available in all groups - # users that are not in all groups can't get the tickets assigned - ticket_ids = _.map(items, (el) -> $(el).val() ) - ticket_group_ids = _.map(App.Ticket.findAll(ticket_ids), (ticket) -> ticket.group_id) - users = @usersInGroups(ticket_group_ids) - users.map( (user) -> {value: user.id, name: user.displayName()} ) - - usersInGroups: (group_ids) -> - ids_by_group = _.chain(@formMeta?.dependencies?.group_id) - .pick(group_ids) - .values() - .map( (e) -> e.owner_id) - .value() - - # Underscore's intersection doesn't work when chained - ids_in_all_groups = _.intersection(ids_by_group...) - - users = App.User.findAll(ids_in_all_groups) - _.sortBy(users, (user) -> user.firstname) - articleTypeFilter: (items) -> for item in items if item.name is 'note' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee index f657563d2..ae57b7c2c 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee @@ -2,20 +2,20 @@ class OwnerFormHandlerDependencies # central method, is getting called on every ticket form change @run: (params, attribute, attributes, classname, form, ui) -> - return if 'group_id' not of params || 'owner_id' not of params + return if 'group_id' not of params + return if 'owner_id' not of params owner_attribute = _.find(attributes, (o) -> o.name == 'owner_id') return if !owner_attribute - return if 'alt_options' not of owner_attribute + return if 'alternative_user_options' not of owner_attribute - if !params.group_id - # if no group is chosen, then we use the alt_options to populate the owner_id field - owner_attribute.options = owner_attribute.alt_options - delete owner_attribute['relation'] - else - # if a group is chosen, then populate owner_id using attribute.relation + # fetch contents using User relation if a Group has been selected, otherwise render alternative_user_options + if params.group_id owner_attribute.relation = 'User' delete owner_attribute['options'] + else + owner_attribute.options = owner_attribute.alternative_user_options + delete owner_attribute['relation'] # replace new option list owner_attribute.default = params[owner_attribute.name] @@ -23,4 +23,4 @@ class OwnerFormHandlerDependencies newElement = ui.formGenItem(owner_attribute, classname, form) form.find('select[name="owner_id"]').closest('.form-group').replaceWith(newElement) -App.Config.set('150-ticketFormChanges', OwnerFormHandlerDependencies, 'TicketZoomFormHandler') \ No newline at end of file +App.Config.set('150-ticketFormChanges', OwnerFormHandlerDependencies, 'TicketZoomFormHandler') diff --git a/test/browser/agent_ticket_overview_level0_test.rb b/test/browser/agent_ticket_overview_level0_test.rb index 2727f75fd..bd8475b02 100644 --- a/test/browser/agent_ticket_overview_level0_test.rb +++ b/test/browser/agent_ticket_overview_level0_test.rb @@ -400,11 +400,6 @@ class AgentTicketOverviewLevel0Test < TestCase link: '#ticket/view/all_unassigned', ) - # enable full overviews - execute( - js: '$(".content.active .sidebar").css("display", "block")', - ) - watch_for( css: '.content.active', value: 'overview owner change #2', From 16a4c2dcd18e0758ed0d95d0ca363cca8eab44cf Mon Sep 17 00:00:00 2001 From: Billy Zhou Date: Tue, 21 Aug 2018 16:22:04 +0800 Subject: [PATCH 4/4] Renamed alternative_user_options to possible_groups_owners --- .../javascripts/app/controllers/ticket_overview.coffee | 2 +- .../ticket_zoom/owner_handler_dependencies.coffee | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index edc64398a..1bc014fca 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/app/assets/javascripts/app/controllers/ticket_overview.coffee @@ -1281,7 +1281,7 @@ class BulkForm extends App.Controller continue if attribute.name != 'owner_id' {users, groups} = @validUsersForTicketSelection() options = _.map(users, (user) -> {value: user.id, name: user.displayName()} ) - attribute.alternative_user_options = options + attribute.possible_groups_owners = options new App.ControllerForm( el: @$('#form-ticket-bulk') diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee index ae57b7c2c..74a853876 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/owner_handler_dependencies.coffee @@ -7,14 +7,14 @@ class OwnerFormHandlerDependencies owner_attribute = _.find(attributes, (o) -> o.name == 'owner_id') return if !owner_attribute - return if 'alternative_user_options' not of owner_attribute + return if 'possible_groups_owners' not of owner_attribute - # fetch contents using User relation if a Group has been selected, otherwise render alternative_user_options + # fetch contents using User relation if a Group has been selected, otherwise render possible_groups_owners if params.group_id owner_attribute.relation = 'User' delete owner_attribute['options'] else - owner_attribute.options = owner_attribute.alternative_user_options + owner_attribute.options = owner_attribute.possible_groups_owners delete owner_attribute['relation'] # replace new option list