Addressed QA comments on merge request 8

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
+      @validUsersForTicketSelection()
-      groups: groups_sorted
     ))
 
   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'

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
+    # fetch contents using User relation if a Group has been selected, otherwise render alternative_user_options
-      # if no group is chosen, then we use the alt_options to populate the owner_id field
+    if params.group_id
-      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']
+    else
+      owner_attribute.options = owner_attribute.alternative_user_options
+      delete owner_attribute['relation']
 
     # replace new option list
     owner_attribute.default = params[owner_attribute.name]

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',