Browse Source

Fixes #3452 - Able to select agents in @@mention feature which do not have access to tickets.

Rolf Schmidt 4 years ago
parent
commit
3599319ea6

+ 4 - 2
app/assets/javascripts/app/controllers/widget/text_module.coffee

@@ -55,5 +55,7 @@ class App.WidgetTextModule extends App.Controller
     # set new data
     # set new data
     if @bindElements[0]
     if @bindElements[0]
       for element in @bindElements
       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

+ 28 - 21
app/assets/javascripts/app/lib/base/jquery.textmodule.js

@@ -622,7 +622,7 @@
     var user_id = $(elem).data('id')
     var user_id = $(elem).data('id')
     var user    = App.User.find(user_id)
     var user    = App.User.find(user_id)
     if (!user) {
     if (!user) {
-      callback('')
+      return callback('')
     }
     }
 
 
     fqdn      = App.Config.get('fqdn')
     fqdn      = App.Config.get('fqdn')
@@ -653,33 +653,40 @@
     App.Delay.set(function() {
     App.Delay.set(function() {
       items = []
       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 = $('<li>', {
+              'data-id': user_id,
+              text: user.firstname + ' ' + user.lastname + ' <' + user.email + '>'
+            })
+            if (!activeSet) {
+              activeSet = true
+              item.addClass('is-active')
+            }
 
 
-          item = $('<li>', {
-            'data-id': user_id,
-            text: user.firstname + ' ' + user.lastname + ' <' + user.email + '>'
+            items.push(item)
           })
           })
-          if (!activeSet) {
-            activeSet = true
-            item.addClass('is-active')
+
+          if(items.length == 0) {
+            items.push($('<li>').text(App.i18n.translateInline('No results found')))
           }
           }
 
 
-          items.push(item)
+          textmodule.appendResults(items)
         })
         })
-
-        if(items.length == 0) {
-          items.push($('<li>').text(App.i18n.translateInline('No results found')))
-        }
-
+      }
+      else {
+        textmodule.emptyResultsContainer()
+        items.push($('<li>').text(App.i18n.translateInline('Please select a group first, before you mention a user!')))
         textmodule.appendResults(items)
         textmodule.appendResults(items)
-      })
+      }
     }, 200, 'textmoduleMentionDelay', 'textmodule')
     }, 200, 'textmoduleMentionDelay', 'textmodule')
   }
   }
 
 

+ 5 - 1
app/assets/javascripts/app/models/mention.coffee

@@ -21,10 +21,13 @@ class App.Mention extends App.Model
         callback(data)
         callback(data)
     )
     )
 
 
-  @searchUser: (query, callback) ->
+  @searchUser: (query, group_id, callback) ->
     roles    = App.Role.withPermissions('ticket.agent')
     roles    = App.Role.withPermissions('ticket.agent')
     role_ids = roles.map (role) -> role.id
     role_ids = roles.map (role) -> role.id
 
 
+    group_ids = {}
+    group_ids[group_id] = 'read'
+
     App.Ajax.request(
     App.Ajax.request(
       type: 'GET'
       type: 'GET'
       url:  "#{@apiPath}/users/search"
       url:  "#{@apiPath}/users/search"
@@ -32,6 +35,7 @@ class App.Mention extends App.Model
         limit: 10
         limit: 10
         query: query
         query: query
         role_ids: role_ids
         role_ids: role_ids
+        group_ids: group_ids
         full: true
         full: true
       processData: true
       processData: true
       success: (data, status, xhr) ->
       success: (data, status, xhr) ->

+ 8 - 7
app/controllers/users_controller.rb

@@ -217,12 +217,13 @@ class UsersController < ApplicationController
   #                   The requester has to be in the role 'Admin' or 'Agent' to
   #                   The requester has to be in the role 'Admin' or 'Agent' to
   #                   be able to search for User records.
   #                   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<String>] 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 <email>", :value => "firstname lastname <email>"},...].
+  # @parameter        query            [String]                             The search query.
+  # @parameter        limit            [Integer]                            The limit of search results.
+  # @parameter        role_ids(multi)  [Array<String>]                      A list of Role identifiers to which the Users have to be allocated to.
+  # @parameter        group_ids(multi) [Hash<String=>String,Array<String>>] 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 <email>", :value => "firstname lastname <email>"},...].
   #
   #
   # @response_message 200 [Array<User>] A list of User records matching the search term.
   # @response_message 200 [Array<User>] A list of User records matching the search term.
   # @response_message 403               Forbidden / Invalid session.
   # @response_message 403               Forbidden / Invalid session.
@@ -254,7 +255,7 @@ class UsersController < ApplicationController
       order_by:     params[:order_by],
       order_by:     params[:order_by],
       current_user: current_user,
       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?
       next if params[key].blank?
 
 
       query_params[key] = params[key]
       query_params[key] = params[key]

+ 35 - 15
app/models/user/search.rb

@@ -54,6 +54,7 @@ or with certain role_ids | permissions
     offset: 100,
     offset: 100,
     current_user: user_model,
     current_user: user_model,
     role_ids: [1,2,3],
     role_ids: [1,2,3],
+    group_ids: [1,2,3],
     permissions: ['ticket.agent']
     permissions: ['ticket.agent']
 
 
     # sort single column
     # sort single column
@@ -101,8 +102,8 @@ returns
         if SearchIndexBackend.enabled?
         if SearchIndexBackend.enabled?
           query_extension = {}
           query_extension = {}
           if params[:role_ids].present?
           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)
             if !params[:role_ids].is_a?(Array)
               params[:role_ids] = [params[:role_ids]]
               params[:role_ids] = [params[:role_ids]]
             end
             end
@@ -111,6 +112,18 @@ returns
             }
             }
             query_extension['bool']['must'].push access_condition
             query_extension['bool']['must'].push access_condition
           end
           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,
           items = SearchIndexBackend.search(query, 'User', limit:           limit,
                                                            query_extension: query_extension,
                                                            query_extension: query_extension,
@@ -132,22 +145,29 @@ returns
         # fallback do sql query
         # fallback do sql query
         # - stip out * we already search for *query* -
         # - stip out * we already search for *query* -
         query.delete! '*'
         query.delete! '*'
+
+        statement = User
         if params[:role_ids]
         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
         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
     end
   end
   end

+ 64 - 0
spec/requests/user_spec.rb

@@ -1367,4 +1367,68 @@ RSpec.describe 'User', type: :request do
       expect(User.last).not_to be_role('Agent')
       expect(User.last).not_to be_role('Agent')
     end
     end
   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
 end