Fixes #2088 User doesn't appear in drag&drop overlay in a group if access is via roles

app/assets/javascripts/app/controllers/ticket_overview.coffee

@@ -34,6 +34,11 @@ class App.TicketOverview extends App.Controller
     @bind 'ui:rerender', =>
       @renderBatchOverlay()
 
+    load = (data) =>
+      App.Collection.loadAssets(data.assets)
+      @formMeta = data.form_meta
+    @bindId = App.TicketCreateCollection.bind(load)
+
   startDragItem: (event) =>
     return if !@batchSupport
     @grabbedItem = $(event.currentTarget)
@@ -413,17 +418,10 @@ class App.TicketOverview extends App.Controller
 
     groupId = @hoveredBatchEntry.attr('data-id')
     group = App.Group.find(groupId)
-    users = []
-
-    for user_id in _.uniq(group.user_ids)
-      if App.User.exists(user_id)
-        user = App.User.find(user_id)
-        if user.active is true
-          users.push user
 
     @batchAssignGroupName.text group.displayName()
     @batchAssignGroupInner.html $(App.view('ticket_overview/batch_overlay_user_group')(
-      users: users
+      users: @usersInGroups([groupId])
       groups: []
       groupId: groupId
     ))
@@ -542,6 +540,19 @@ 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')())
 
@@ -589,47 +600,42 @@ class App.TicketOverview extends App.Controller
     @refreshElements()
 
   renderOptions: =>
-    macros = App.Macro.findAllByAttribute('active', true)
+    @renderOptionsGroups()
-    groups = App.Group.findAllByAttribute('active', true)
+    @renderOptionsMacros()
-    users = []
+
+  renderOptionsGroups: =>
     items = @el.find('[name="bulk"]:checked')
 
-    # find all possible owners for selected tickets
+    # we want to display all users for which we can assign the tickets directly
-    possibleUsers = {}
+    # for this we need to get the groups of all selected tickets
-    possibleUserGroups = {}
+    # after we got those we need to check which users are available in all groups
-    for item in items
+    # users that are not in all groups can't get the tickets assigned
-      #console.log "selected items with id ", $(item).val()
+    ticket_ids       = _.map(items, (el) -> $(el).val() )
-      ticket = App.Ticket.find($(item).val())
+    ticket_group_ids = _.map(App.Ticket.findAll(ticket_ids), (ticket) -> ticket.group_id)
-      if !possibleUserGroups[ticket.group_id.toString()]
+    users            = @usersInGroups(ticket_group_ids)
-        group = App.Group.find(ticket.group_id)
+
-        for user_id in group.user_ids
+    # get the list of possible groups for the current user
-          if !possibleUserGroups[ticket.group_id.toString()]
+    # from the TicketCreateCollection
-            possibleUsers[user_id.toString()] = true
+    # (filled for e.g. the TicketCreation or TicketZoom assignment)
-          else
+    # and order them by name
-            hit = false
+    group_ids     = _.keys(@formMeta?.dependencies?.group_id)
-            for user_id, exists of possibleUsers
+    groups        = App.Group.findAll(group_ids)
-              if possibleUsers[user_id.toString()]
+    groups_sorted = _.sortBy(groups, (group) -> group.name)
-                hit = true
+
-            if !hit
+    # get the number of visible users per group
-              delete possibleUsers[user_id.toString()]
+    # from the TicketCreateCollection
-        possibleUserGroups[ticket.group_id.toString()] = true
+    # (filled for e.g. the TicketCreation or TicketZoom assignment)
-    for user_id, _exists of possibleUsers
-      if App.User.exists(user_id)
-        user = App.User.find(user_id)
-        if user.active is true
-          users.push user
     for group in groups
-      valid_user_ids = []
+      group.valid_users_count = @formMeta?.dependencies?.group_id?[group.id]?.owner_id.length || 0
-      for user_id in _.uniq(group.user_ids)
-        if App.User.exists(user_id)
-          if App.User.find(user_id).active is true
-            valid_user_ids.push user_id
-      group.valid_user_ids = valid_user_ids
 
     @batchAssignInner.html $(App.view('ticket_overview/batch_overlay_user_group')(
       users: users
-      groups: groups
+      groups: groups_sorted
     ))
+
+  renderOptionsMacros: =>
+    macros = App.Macro.search(filter: { active: true }, sortBy:'name', order:'DESC')
+
     @batchMacro.html $(App.view('ticket_overview/batch_overlay_macro')(
       macros: macros
     ))
@@ -695,9 +701,10 @@ class App.TicketOverview extends App.Controller
   changed: ->
     false
 
-  release: ->
+  release: =>
     @keyboardOff()
     super
+    App.TicketCreateCollection.unbindById(@bindId)
 
   keyboardOn: =>
     $(window).off 'keydown.overview_navigation'

app/assets/javascripts/app/views/ticket_overview/batch_overlay_user_group.jst.eco

@@ -8,6 +8,6 @@
 <div class="batch-overlay-assign-entry js-batch-overlay-entry" data-action="group_assign" data-id="<%= group.id %>">
 	<div class="js-batch-hover-target"><%- group.avatar(80) %></div>
   <div class="batch-overlay-assign-entry-name"><%- group.displayName() %></div>
-  <div class="batch-overlay-assign-entry-detail"><%- @T('%s people', group.valid_user_ids.length) %></div>
+  <div class="batch-overlay-assign-entry-detail"><%- @T('%s people', group.valid_users_count) %></div>
 </div>
 <% end %>

app/models/role.rb

@@ -16,6 +16,8 @@ class Role < ApplicationModel
   before_create  :validate_permissions, :check_default_at_signup_permissions
   before_update  :validate_permissions, :last_admin_check_by_attribute, :validate_agent_limit_by_attributes, :check_default_at_signup_permissions
 
+  # ignore Users because this will lead to huge
+  # results for e.g. the Customer role
   association_attributes_ignored :users
 
   activity_stream_permission 'admin.role'

lib/sessions/backend/ticket_create.rb

@@ -1,7 +1,6 @@
 class Sessions::Backend::TicketCreate < Sessions::Backend::Base
 
   def load
-
     # get attributes to update
     ticket_create_attributes = Ticket::ScreenOptions.attributes_to_change(
       current_user: @user,

test/browser/admin_drag_drop_to_new_group_test.rb

@@ -1,15 +1,43 @@
 require 'browser_test_helper'
 
 class AdminDragDropToNewGroupTest < TestCase
-  def test_new_group
+  def test_group_via_role
-    new_group_name = "d_n_d_group#{rand(99_999_999)}"
     @browser = browser_instance
     login(
       username: 'master@example.com',
       password: 'test',
       url: browser_url,
     )
-    tasks_close_all()
+
+    new_group_name = add_group
+    new_role_name = add_role(new_group_name)
+    open_user_modal do
+      assign_role(new_role_name)
+    end
+    list_tickets
+    assert get_group_element(new_group_name)
+  end
+
+  def test_new_group
+    @browser = browser_instance
+    login(
+      username: 'master@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+
+    new_group_name = add_group
+    open_user_modal do
+      assign_group(new_group_name, scroll: true)
+    end
+    list_tickets
+    assert get_group_element(new_group_name)
+  end
+
+  private
+
+  def add_group
+    name = "dndgroup-#{rand(99_999_999)}"
 
     click(css: '.user-menu a[title=Admin')
     click(css: '.content.active a[href="#manage/groups"]')
@@ -19,75 +47,153 @@ class AdminDragDropToNewGroupTest < TestCase
 
     element = @browser.find_element(css: '.modal input[name=name]')
     element.clear
-    element.send_keys(new_group_name)
+    element.send_keys(name)
     click(css: '.modal button.js-submit')
 
     sleep(1)
 
+    name
+  end
+
+  def add_role(group_name)
+    role_name = "#{group_name}-role"
+
+    click(css: '.content.active a[href="#manage/roles"]')
+    click(css: '.content.active a[data-type="new"]')
+
+    modal_ready
+
+    element = @browser.find_element(css: '.modal input[name=name]')
+    element.clear
+    element.send_keys(role_name)
+
+    agent_permission = @browser.find_element(css: '.modal input[data-permission-name="ticket.agent"]')
+    permission_id = agent_permission.attribute(:value)
+
+    scroll_to(agent_permission.location.y)
+
+    toggle_checkbox(@browser.find_element(css: '.modal'), "\"#{permission_id}\"") #digit-only selector fails
+
+    assign_group(group_name)
+
+    click(css: '.modal button.js-submit')
+
+    sleep(1)
+
+    role_name
+  end
+
+  def open_user_modal
     click(css: '.content.active a[href="#manage/users"]')
 
     user_css = '.user-list .js-tableBody tr td'
     watch_for(css: user_css)
-    click(css: user_css)
+
+    user_element = @browser.find_elements(css: user_css).find do |el|
+      el.text.strip == 'master@example.com'
+    end
+
+    user_element.click
 
     modal_ready
 
-    scroll_script = "var el = document.getElementsByClassName('modal')[0];"
+    yield
-    scroll_script += 'el.scrollTo(0, el.scrollHeight);'
-
-    @browser.execute_script scroll_script
-
-    group = @browser.find_elements(css: '.modal .settings-list tbody tr').find do |el|
-      el.find_element(css: 'td').text == new_group_name
-    end
-
-    assert_not_nil(group)
-
-    %w[read create].each { |val| toggle_checkbox(group, val) }
 
     click(css: '.modal button.js-submit')
 
     sleep(1)
+  end
 
-    click(css: '.menu-item[href="#ticket/view"]')
+  def scroll_to(offset_y = 'el.scrollHeight')
-    click(css: '.overview-header .tabsHolder a.tab[href="#ticket/view/all_unassigned"]')
+    scroll_script = "var el = document.getElementsByClassName('modal')[0];"
+    scroll_script += "el.scrollTo(0, #{offset_y});"
 
-    element = @browser.find_element(css: '.js-tableBody .item')
+    @browser.execute_script scroll_script
+  end
 
-    @browser
+  def assign_group(group_name, scroll: false)
-      .action
+    group_container = @browser.find_elements(css: '.modal .settings-list tbody tr').find do |el|
-      .click_and_hold(element)
+      el.find_element(css: 'td').text == group_name
-      .move_by(100, 100)
+    end
-      .perform
 
-    sleep(1)
+    assert_not_nil(group_container)
 
-    @browser
+    scroll_to(group_container.location.y) if scroll
-      .action
+
-      .move_to(@browser.find_element(css: '.js-batch-assign-circle'))
+    toggle_checkbox(group_container, 'full')
-      .perform
+  end
+
+  def assign_role(role_name)
+    role_container = @browser.find_elements(css: '.modal .checkbox > .inline-label').find do |el|
+      el.find_elements(css: '.label-text').first&.text == role_name
+    end
+
+    scroll_to role_container.location.y
+
+    assert_not_nil role_container
+
+    role_id = role_container.find_element(css: 'input').attribute(:value)
+    toggle_checkbox(role_container, "\"#{role_id}\"") #digit-only selector fails
+  end
+
+  def get_group_element(group_name)
+    # wait until the scheduler pushes
+    # the changes to the FE
+    sleep(1.5)
+
+    10.times do
+      dnd_element = @browser.find_element(css: '.content.active .js-tableBody .item')
+
+      window_height = @browser.execute_script('return window.innerHeight')
+
+      offset = window_height - dnd_element.location.y - dnd_element.rect.height - 50
+
+      @browser.action.click_and_hold(dnd_element).perform
+
+      @browser.action.move_by(0, 100).perform
+
+      sleep(0.5)
+
+      @browser.action.move_by(0, offset - 100).perform
 
       sleep(1)
 
       group_containers = @browser.find_elements(css: '.batch-overlay-assign-entry[data-action=group_assign]')
 
       new_group_container = group_containers.find do |g|
-      g.find_element(css: '.batch-overlay-assign-entry-name').text.downcase == new_group_name
+        g.find_element(css: '.batch-overlay-assign-entry-name').text.downcase == group_name
       end
 
-    assert_not_nil new_group_container
+      verified = verify_group_and_contents(new_group_container)
+      return true if verified
 
-    group_description = new_group_container.find_element(css: '.batch-overlay-assign-entry-detail').text
+      sleep(0.5)
-    assert_equal('1 PEOPLE', group_description)
 
       @browser
         .action
-      .move_to(new_group_container)
+        .release
         .perform
+    end
 
-    sleep(1)
+    false
+  end
+
+  def verify_group_and_contents(group_container)
+    return false if group_container.nil?
+
+    group_description = group_container.find_element(css: '.batch-overlay-assign-entry-detail').text
+
+    return false if group_description != '1 PEOPLE'
+
+    @browser.action.move_to(group_container).perform
 
     users_in_group = @browser.find_elements(css: '.js-batch-assign-group-inner .batch-overlay-assign-entry[data-action=user_assign]')
-    assert_equal(1, users_in_group.count)
+
+    users_in_group.count == 1
+  end
+
+  def list_tickets
+    click(css: '.menu-item[href="#ticket/view"]')
+    click(css: '.overview-header .tabsHolder a.tab[href="#ticket/view/all_unassigned"]')
   end
 end