From 9cdb0d5ce4e61eb8220000ff506fbde9747852e0 Mon Sep 17 00:00:00 2001 From: Mantas Date: Fri, 22 Jun 2018 15:06:14 +0300 Subject: [PATCH] Fixes #2088 User doesn't appear in drag&drop overlay in a group if access is via roles --- .../app/controllers/ticket_overview.coffee | 93 ++++---- .../batch_overlay_user_group.jst.eco | 2 +- app/models/role.rb | 2 + lib/sessions/backend/ticket_create.rb | 1 - .../admin_drag_drop_to_new_group_test.rb | 202 +++++++++++++----- 5 files changed, 207 insertions(+), 93 deletions(-) diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index 420f4e9f8..369fea4d5 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/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) - groups = App.Group.findAllByAttribute('active', true) - users = [] + @renderOptionsGroups() + @renderOptionsMacros() + + renderOptionsGroups: => items = @el.find('[name="bulk"]:checked') - # find all possible owners for selected tickets - possibleUsers = {} - possibleUserGroups = {} - for item in items - #console.log "selected items with id ", $(item).val() - ticket = App.Ticket.find($(item).val()) - if !possibleUserGroups[ticket.group_id.toString()] - group = App.Group.find(ticket.group_id) - for user_id in group.user_ids - if !possibleUserGroups[ticket.group_id.toString()] - possibleUsers[user_id.toString()] = true - else - hit = false - for user_id, exists of possibleUsers - if possibleUsers[user_id.toString()] - hit = true - if !hit - delete possibleUsers[user_id.toString()] - possibleUserGroups[ticket.group_id.toString()] = true - 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 + # 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 - valid_user_ids = [] - 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 + 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 + 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' diff --git a/app/assets/javascripts/app/views/ticket_overview/batch_overlay_user_group.jst.eco b/app/assets/javascripts/app/views/ticket_overview/batch_overlay_user_group.jst.eco index 2931f9bd3..db765db71 100644 --- a/app/assets/javascripts/app/views/ticket_overview/batch_overlay_user_group.jst.eco +++ b/app/assets/javascripts/app/views/ticket_overview/batch_overlay_user_group.jst.eco @@ -8,6 +8,6 @@
<%- group.avatar(80) %>
<%- group.displayName() %>
-
<%- @T('%s people', group.valid_user_ids.length) %>
+
<%- @T('%s people', group.valid_users_count) %>
<% end %> \ No newline at end of file diff --git a/app/models/role.rb b/app/models/role.rb index fc62f1c45..5e60cfd31 100644 --- a/app/models/role.rb +++ b/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' diff --git a/lib/sessions/backend/ticket_create.rb b/lib/sessions/backend/ticket_create.rb index c8e757ad7..2f224f626 100644 --- a/lib/sessions/backend/ticket_create.rb +++ b/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, diff --git a/test/browser/admin_drag_drop_to_new_group_test.rb b/test/browser/admin_drag_drop_to_new_group_test.rb index bc6900a50..e4a1c007b 100644 --- a/test/browser/admin_drag_drop_to_new_group_test.rb +++ b/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 - new_group_name = "d_n_d_group#{rand(99_999_999)}" + def test_group_via_role @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];" - 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) } + yield click(css: '.modal button.js-submit') sleep(1) + end - click(css: '.menu-item[href="#ticket/view"]') - click(css: '.overview-header .tabsHolder a.tab[href="#ticket/view/all_unassigned"]') + def scroll_to(offset_y = 'el.scrollHeight') + 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 - .action - .click_and_hold(element) - .move_by(100, 100) - .perform - - sleep(1) - - @browser - .action - .move_to(@browser.find_element(css: '.js-batch-assign-circle')) - .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 + def assign_group(group_name, scroll: false) + group_container = @browser.find_elements(css: '.modal .settings-list tbody tr').find do |el| + el.find_element(css: 'td').text == group_name end - assert_not_nil new_group_container + assert_not_nil(group_container) - group_description = new_group_container.find_element(css: '.batch-overlay-assign-entry-detail').text - assert_equal('1 PEOPLE', group_description) + scroll_to(group_container.location.y) if scroll - @browser - .action - .move_to(new_group_container) - .perform + toggle_checkbox(group_container, 'full') + end - sleep(1) + 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 == group_name + end + + verified = verify_group_and_contents(new_group_container) + return true if verified + + sleep(0.5) + + @browser + .action + .release + .perform + end + + 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