From 3599319ea6af04cdba782b7d00204896f857660d Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Fri, 19 Mar 2021 13:32:03 +0000 Subject: [PATCH] Fixes #3452 - Able to select agents in @@mention feature which do not have access to tickets. --- .../app/controllers/widget/text_module.coffee | 6 +- .../app/lib/base/jquery.textmodule.js | 49 ++++++++------ .../javascripts/app/models/mention.coffee | 6 +- app/controllers/users_controller.rb | 15 +++-- app/models/user/search.rb | 50 ++++++++++----- spec/requests/user_spec.rb | 64 +++++++++++++++++++ 6 files changed, 144 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/app/controllers/widget/text_module.coffee b/app/assets/javascripts/app/controllers/widget/text_module.coffee index 1ff860c67..811c60733 100644 --- a/app/assets/javascripts/app/controllers/widget/text_module.coffee +++ b/app/assets/javascripts/app/controllers/widget/text_module.coffee @@ -55,5 +55,7 @@ class App.WidgetTextModule extends App.Controller # set new data if @bindElements[0] for element in @bindElements - if $(element).data().plugin_textmodule - $(element).data().plugin_textmodule.collection = @all + continue if !$(element).data().plugin_textmodule + + $(element).data().plugin_textmodule.searchCondition = @searchCondition + $(element).data().plugin_textmodule.collection = @all diff --git a/app/assets/javascripts/app/lib/base/jquery.textmodule.js b/app/assets/javascripts/app/lib/base/jquery.textmodule.js index a6c59ecbd..53f607b1d 100644 --- a/app/assets/javascripts/app/lib/base/jquery.textmodule.js +++ b/app/assets/javascripts/app/lib/base/jquery.textmodule.js @@ -622,7 +622,7 @@ var user_id = $(elem).data('id') var user = App.User.find(user_id) if (!user) { - callback('') + return callback('') } fqdn = App.Config.get('fqdn') @@ -653,33 +653,40 @@ App.Delay.set(function() { items = [] - App.Mention.searchUser(term, function(data) { - textmodule.emptyResultsContainer() + if (textmodule.searchCondition.group_id) { + App.Mention.searchUser(term, textmodule.searchCondition.group_id, function(data) { + textmodule.emptyResultsContainer() - activeSet = false - $.each(data.user_ids, function(index, user_id) { - user = App.User.find(user_id) - if (!user) return true - if (!user.active) return true + activeSet = false + $.each(data.user_ids, function(index, user_id) { + user = App.User.find(user_id) + if (!user) return true + if (!user.active) return true - item = $('
  • ', { - 'data-id': user_id, - text: user.firstname + ' ' + user.lastname + ' <' + user.email + '>' + item = $('
  • ', { + 'data-id': user_id, + text: user.firstname + ' ' + user.lastname + ' <' + user.email + '>' + }) + if (!activeSet) { + activeSet = true + item.addClass('is-active') + } + + items.push(item) }) - if (!activeSet) { - activeSet = true - item.addClass('is-active') + + if(items.length == 0) { + items.push($('
  • ').text(App.i18n.translateInline('No results found'))) } - items.push(item) + textmodule.appendResults(items) }) - - if(items.length == 0) { - items.push($('
  • ').text(App.i18n.translateInline('No results found'))) - } - + } + else { + textmodule.emptyResultsContainer() + items.push($('
  • ').text(App.i18n.translateInline('Please select a group first, before you mention a user!'))) textmodule.appendResults(items) - }) + } }, 200, 'textmoduleMentionDelay', 'textmodule') } diff --git a/app/assets/javascripts/app/models/mention.coffee b/app/assets/javascripts/app/models/mention.coffee index 9fcf8c073..4aae73793 100644 --- a/app/assets/javascripts/app/models/mention.coffee +++ b/app/assets/javascripts/app/models/mention.coffee @@ -21,10 +21,13 @@ class App.Mention extends App.Model callback(data) ) - @searchUser: (query, callback) -> + @searchUser: (query, group_id, callback) -> roles = App.Role.withPermissions('ticket.agent') role_ids = roles.map (role) -> role.id + group_ids = {} + group_ids[group_id] = 'read' + App.Ajax.request( type: 'GET' url: "#{@apiPath}/users/search" @@ -32,6 +35,7 @@ class App.Mention extends App.Model limit: 10 query: query role_ids: role_ids + group_ids: group_ids full: true processData: true success: (data, status, xhr) -> diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 6945078fd..71dd0175f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -217,12 +217,13 @@ class UsersController < ApplicationController # The requester has to be in the role 'Admin' or 'Agent' to # be able to search for User records. # - # @parameter query [String] The search query. - # @parameter limit [Integer] The limit of search results. - # @parameter role_ids(multi) [Array] A list of Role identifiers to which the Users have to be allocated to. - # @parameter full [Boolean] Defines if the result should be - # true: { user_ids => [1,2,...], assets => {...} } - # or false: [{:id => user.id, :label => "firstname lastname ", :value => "firstname lastname "},...]. + # @parameter query [String] The search query. + # @parameter limit [Integer] The limit of search results. + # @parameter role_ids(multi) [Array] A list of Role identifiers to which the Users have to be allocated to. + # @parameter group_ids(multi) [HashString,Array>] A list of Group identifiers to which the Users have to be allocated to. + # @parameter full [Boolean] Defines if the result should be + # true: { user_ids => [1,2,...], assets => {...} } + # or false: [{:id => user.id, :label => "firstname lastname ", :value => "firstname lastname "},...]. # # @response_message 200 [Array] A list of User records matching the search term. # @response_message 403 Forbidden / Invalid session. @@ -254,7 +255,7 @@ class UsersController < ApplicationController order_by: params[:order_by], current_user: current_user, } - %i[role_ids permissions].each do |key| + %i[role_ids group_ids permissions].each do |key| next if params[key].blank? query_params[key] = params[key] diff --git a/app/models/user/search.rb b/app/models/user/search.rb index d2988423e..3c40332cd 100644 --- a/app/models/user/search.rb +++ b/app/models/user/search.rb @@ -54,6 +54,7 @@ or with certain role_ids | permissions offset: 100, current_user: user_model, role_ids: [1,2,3], + group_ids: [1,2,3], permissions: ['ticket.agent'] # sort single column @@ -101,8 +102,8 @@ returns if SearchIndexBackend.enabled? query_extension = {} if params[:role_ids].present? - query_extension['bool'] = {} - query_extension['bool']['must'] = [] + query_extension['bool'] ||= {} + query_extension['bool']['must'] ||= [] if !params[:role_ids].is_a?(Array) params[:role_ids] = [params[:role_ids]] end @@ -111,6 +112,18 @@ returns } query_extension['bool']['must'].push access_condition end + if params[:group_ids].present? + query_extension['bool'] ||= {} + query_extension['bool']['must'] ||= [] + user_ids = [] + params[:group_ids].each do |group_id, access| + user_ids |= User.group_access(group_id.to_i, access).pluck(:id) + end + + return [] if user_ids.blank? + + query_extension['bool']['must'].push({ 'terms' => { '_id' => user_ids } }) + end items = SearchIndexBackend.search(query, 'User', limit: limit, query_extension: query_extension, @@ -132,22 +145,29 @@ returns # fallback do sql query # - stip out * we already search for *query* - query.delete! '*' + + statement = User if params[:role_ids] - User.joins(:roles).where('roles.id' => params[:role_ids]).where( - '(users.firstname LIKE ? OR users.lastname LIKE ? OR users.email LIKE ? OR users.login LIKE ?) AND users.id != 1', "%#{query}%", "%#{query}%", "%#{query}%", "%#{query}%" - ) - .order(Arel.sql(order_sql)) - .offset(offset) - .limit(limit) - else - User.where( - '(firstname LIKE ? OR lastname LIKE ? OR email LIKE ? OR login LIKE ?) AND id != 1', "%#{query}%", "%#{query}%", "%#{query}%", "%#{query}%" - ) - .order(Arel.sql(order_sql)) - .offset(offset) - .limit(limit) + statement = statement.joins(:roles).where('roles.id' => params[:role_ids]) + end + if params[:group_ids] + user_ids = [] + params[:group_ids].each do |group_id, access| + user_ids |= User.group_access(group_id.to_i, access).pluck(:id) + end + statement = if user_ids.present? + statement.where(id: user_ids) + else + statement.none + end end + statement.where( + '(users.firstname LIKE ? OR users.lastname LIKE ? OR users.email LIKE ? OR users.login LIKE ?) AND users.id != 1', "%#{query}%", "%#{query}%", "%#{query}%", "%#{query}%" + ) + .order(Arel.sql(order_sql)) + .offset(offset) + .limit(limit) end end end diff --git a/spec/requests/user_spec.rb b/spec/requests/user_spec.rb index 5a76215cb..b86568887 100644 --- a/spec/requests/user_spec.rb +++ b/spec/requests/user_spec.rb @@ -1367,4 +1367,68 @@ RSpec.describe 'User', type: :request do expect(User.last).not_to be_role('Agent') end end + + describe 'GET /api/v1/users/search group ids' do + let(:group1) { create(:group) } + let(:group2) { create(:group) } + let!(:agent1) { create(:agent, firstname: '9U7Z-agent1', groups: [group1]) } + let!(:agent2) { create(:agent, firstname: '9U7Z-agent2', groups: [group2]) } + + def make_request(params) + authenticated_as(agent1) + get '/api/v1/users/search', params: params, as: :json + end + + describe 'without searchindex' do + it 'does find both users' do + make_request(query: '9U7Z') + expect(json_response.count).to eq(2) + end + + it 'does find only agent 1' do + make_request(query: '9U7Z', group_ids: { group1.id => 'read' }) + expect(json_response[0]['firstname']).to eq(agent1.firstname) + expect(json_response.count).to eq(1) + end + + it 'does find only agent 2' do + make_request(query: '9U7Z', group_ids: { group2.id => 'read' }) + expect(json_response[0]['firstname']).to eq(agent2.firstname) + expect(json_response.count).to eq(1) + end + + it 'does find none' do + make_request(query: '9U7Z', group_ids: { 999 => 'read' }) + expect(json_response.count).to eq(0) + end + end + + describe 'with searchindex', searchindex: true do + before do + configure_elasticsearch(rebuild: true) + end + + it 'does find both users' do + make_request(query: '9U7Z') + expect(json_response.count).to eq(2) + end + + it 'does find only agent 1' do + make_request(query: '9U7Z', group_ids: { group1.id => 'read' }) + expect(json_response[0]['firstname']).to eq(agent1.firstname) + expect(json_response.count).to eq(1) + end + + it 'does find only agent 2' do + make_request(query: '9U7Z', group_ids: { group2.id => 'read' }) + expect(json_response[0]['firstname']).to eq(agent2.firstname) + expect(json_response.count).to eq(1) + end + + it 'does find none' do + make_request(query: '9U7Z', group_ids: { 999 => 'read' }) + expect(json_response.count).to eq(0) + end + end + end end