From 4655039cd104c099ebaf05620dd1cbafd45dc657 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 16 Aug 2015 01:27:11 +0200 Subject: [PATCH] Merged /api/v1/search and /api/v1/search/object controller methods. Added controller tests for permission handling. --- .../app/controllers/navigation.js.coffee | 54 ++- .../app/views/navigation/result.jst.eco | 7 +- app/controllers/search_controller.rb | 97 +--- config/routes/search.rb | 4 +- test/controllers/search_controller_test.rb | 425 ++++++++++++++++++ 5 files changed, 467 insertions(+), 120 deletions(-) create mode 100644 test/controllers/search_controller_test.rb diff --git a/app/assets/javascripts/app/controllers/navigation.js.coffee b/app/assets/javascripts/app/controllers/navigation.js.coffee index 56fc6dac4..c0a99af81 100644 --- a/app/assets/javascripts/app/controllers/navigation.js.coffee +++ b/app/assets/javascripts/app/controllers/navigation.js.coffee @@ -142,15 +142,15 @@ class App.Navigation extends App.ControllerWidgetPermanent searchFunction = => # use cache for search result - if @searchResultCache[@term] - @renderResult( @searchResultCache[@term] ) + if @searchResultCache[@query] + @renderResult( @searchResultCache[@query] ) App.Ajax.request( id: 'search' type: 'GET' url: @apiPath + '/search' data: - term: @term + query: @query processData: true, success: (data, status, xhr) => @@ -158,25 +158,21 @@ class App.Navigation extends App.ControllerWidgetPermanent App.Collection.loadAssets( data.assets ) # cache search result - @searchResultCache[@term] = data.result + @searchResultCache[@query] = data.result - result = data.result - for area in result - if area.name is 'Ticket' - area.result = [] - for id in area.ids - ticket = App.Ticket.find( id ) - area.result.push ticket.searchResultAttributes() - else if area.name is 'User' - area.result = [] - for id in area.ids - user = App.User.find( id ) - area.result.push user.searchResultAttributes() - else if area.name is 'Organization' - area.result = [] - for id in area.ids - organization = App.Organization.find( id ) - area.result.push organization.searchResultAttributes() + result = {} + for item in data.result + if App[item.type] && App[item.type].find + if !result[item.type] + result[item.type] = [] + item_object = App[item.type].find(item.id) + if item_object.searchResultAttributes + item_object_search_attributes = item_object.searchResultAttributes() + result[item.type].push item_object_search_attributes + else + @log 'error', "No such model #{item.type.toLocaleLowerCase()}.searchResultAttributes()" + else + @log 'error', "No such model App.#{item.type}" @renderResult(result) @@ -219,9 +215,9 @@ class App.Navigation extends App.ControllerWidgetPermanent removePopovers() # check if search is needed - term = @$('#global-search').val().trim() - return if !term - @term = term + query = @$('#global-search').val().trim() + return if !query + @query = query @delay( searchFunction, 220, 'search' ) ) @@ -239,11 +235,11 @@ class App.Navigation extends App.ControllerWidgetPermanent return # on other keys, show result - term = @$('#global-search').val().trim() - return if !term - return if term is @term - @term = term - @$('.search').toggleClass('filled', !!@term) + query = @$('#global-search').val().trim() + return if !query + return if query is @query + @query = query + @$('.search').toggleClass('filled', !!@query) @delay( searchFunction, 200, 'search' ) ) diff --git a/app/assets/javascripts/app/views/navigation/result.jst.eco b/app/assets/javascripts/app/views/navigation/result.jst.eco index 3c79809d4..ed0d6404d 100644 --- a/app/assets/javascripts/app/views/navigation/result.jst.eco +++ b/app/assets/javascripts/app/views/navigation/result.jst.eco @@ -1,6 +1,7 @@ -<% for area, i in @result: %> - <% if i > 0: %>
  • <% end %> - <% for item in area.result: %> +<% for area, items of @result: %> + <% if done && items.length > 0: %>
  • <% end %> + <% done = true %> + <% for item in items: %>
  • diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 2b8469e8d..66394c20c 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -3,12 +3,13 @@ class SearchController < ApplicationController before_action :authentication_check + # GET|POST /api/v1/search # GET|POST /api/v1/search/:objects def search_generic - # enable search only for agents and admins - if !current_user.role?(Z_ROLENAME_AGENT) && !current_user.role?(Z_ROLENAME_ADMIN) + # enable search only for users with valid session + if !current_user response_access_deny return true end @@ -19,9 +20,15 @@ class SearchController < ApplicationController # convert objects string into array of class names # e.g. user-ticket-another_object = %w( User Ticket AnotherObject ) - objects = params[:objects].split('-').map(&:camelize) + if !params[:objects] + objects_all = %w( Ticket User Organization ) + else + objects_all = params[:objects].split('-').map(&:camelize) + end + objects = objects_all.clone +puts "OBJECTS: #{objects.inspect}" search_tickets = objects.delete('Ticket') - +puts "OBJECTS_a: #{objects_all.inspect}/#{search_tickets.inspect}" # try search index backend assets = {} result = [] @@ -53,7 +60,7 @@ class SearchController < ApplicationController else # do query - objects.each { |object| + objects_all.each { |object| found_objects = object.constantize.search( query: query, @@ -78,84 +85,4 @@ class SearchController < ApplicationController } end - # GET /api/v1/search - def search - - # get params - query = params[:term] - limit = params[:limit] || 10 - - assets = {} - result = [] - objects = %w( Ticket User Organization ) - if SearchIndexBackend.enabled? - - # to ticket search in serparate call - objects.delete('Ticket') - - # to query search index backend (excluse tickets here, see below) - found_objects = {} - items = SearchIndexBackend.search( query, limit, objects ) - items.each { |item| - require item[:type].to_filename - record = Kernel.const_get( item[:type] ).find( item[:id] ) - assets = record.assets(assets) - - found_objects[ item[:type] ] ||= [] - found_objects[ item[:type] ].push item[:id] - } - - # do ticket query by Ticket class to handle ticket permissions - tickets = Ticket.search( - query: query, - limit: limit, - current_user: current_user, - ) - tickets.each do |ticket| - found_objects[ 'Ticket' ] ||= [] - found_objects[ 'Ticket' ].push ticket.id - end - - # generate whole result - found_objects.each { |object, object_ids| - - data = { - name: object, - ids: object_ids, - } - result.push data - } - else - - objects.each { |object| - - found_objects = object.constantize.search( - query: query, - limit: limit, - current_user: current_user, - ) - - object_ids = [] - found_objects.each do |found_object| - object_ids.push found_object.id - assets = found_object.assets(assets) - end - - next if object_ids.empty? - - data = { - name: object, - ids: object_ids, - } - result.push data - } - end - - # return result - render json: { - assets: assets, - result: result, - } - end - end diff --git a/config/routes/search.rb b/config/routes/search.rb index 44eed8c42..fd4b0fb94 100644 --- a/config/routes/search.rb +++ b/config/routes/search.rb @@ -2,8 +2,6 @@ Zammad::Application.routes.draw do api_path = Rails.configuration.api_path # search - match api_path + '/search', to: 'search#search', via: [:get, :post] - - # search_generic + match api_path + '/search', to: 'search#search_generic', via: [:get, :post] match api_path + '/search/:objects', to: 'search#search_generic', via: [:get, :post] end diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb new file mode 100644 index 000000000..faefb8945 --- /dev/null +++ b/test/controllers/search_controller_test.rb @@ -0,0 +1,425 @@ +# encoding: utf-8 +require 'test_helper' + +class SearchControllerTest < ActionDispatch::IntegrationTest + setup do + + # set accept header + @headers = { 'ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/json' } + + # create agent + roles = Role.where( name: %w(Admin Agent) ) + groups = Group.all + + UserInfo.current_user_id = 1 + @admin = User.create_or_update( + login: 'search-admin', + firstname: 'Search', + lastname: 'Admin', + email: 'search-admin@example.com', + password: 'adminpw', + active: true, + roles: roles, + groups: groups, + ) + + # create agent + roles = Role.where( name: 'Agent' ) + @agent = User.create_or_update( + login: 'search-agent@example.com', + firstname: 'Search 1234', + lastname: 'Agent', + email: 'search-agent@example.com', + password: 'agentpw', + active: true, + roles: roles, + groups: groups, + ) + + # create customer without org + roles = Role.where( name: 'Customer' ) + @customer_without_org = User.create_or_update( + login: 'search-customer1@example.com', + firstname: 'Search', + lastname: 'Customer1', + email: 'search-customer1@example.com', + password: 'customer1pw', + active: true, + roles: roles, + ) + + # create orgs + @organization = Organization.create_or_update( + name: 'Rest Org', + ) + @organization2 = Organization.create_or_update( + name: 'Rest Org #2', + ) + @organization3 = Organization.create_or_update( + name: 'Rest Org #3', + ) + + # create customer with org + @customer_with_org2 = User.create_or_update( + login: 'search-customer2@example.com', + firstname: 'Search', + lastname: 'Customer2', + email: 'search-customer2@example.com', + password: 'customer2pw', + active: true, + roles: roles, + organization_id: @organization.id, + ) + + @customer_with_org3 = User.create_or_update( + login: 'search-customer3@example.com', + firstname: 'Search', + lastname: 'Customer3', + email: 'search-customer3@example.com', + password: 'customer3pw', + active: true, + roles: roles, + organization_id: @organization.id, + ) + + Ticket.all.destroy_all + + @ticket1 = Ticket.create( + title: 'test 1234-1', + group: Group.lookup( name: 'Users'), + customer_id: @customer_without_org.id, + state: Ticket::State.lookup( name: 'new' ), + priority: Ticket::Priority.lookup( name: '2 normal' ), + updated_by_id: 1, + created_by_id: 1, + ) + @article1 = Ticket::Article.create( + ticket_id: @ticket1.id, + from: 'some_sender1@example.com', + to: 'some_recipient1@example.com', + subject: 'some subject1', + message_id: 'some@id', + body: 'some message1', + internal: false, + sender: Ticket::Article::Sender.where(name: 'Customer').first, + type: Ticket::Article::Type.where(name: 'email').first, + updated_by_id: 1, + created_by_id: 1, + ) + sleep 1 + @ticket2 = Ticket.create( + title: 'test 1234-2', + group: Group.lookup( name: 'Users'), + customer_id: @customer_with_org2.id, + state: Ticket::State.lookup( name: 'new' ), + priority: Ticket::Priority.lookup( name: '2 normal' ), + updated_by_id: 1, + created_by_id: 1, + ) + @article2 = Ticket::Article.create( + ticket_id: @ticket2.id, + from: 'some_sender2@example.com', + to: 'some_recipient2@example.com', + subject: 'some subject2', + message_id: 'some@id', + body: 'some message2', + internal: false, + sender: Ticket::Article::Sender.where(name: 'Customer').first, + type: Ticket::Article::Type.where(name: 'email').first, + updated_by_id: 1, + created_by_id: 1, + ) + sleep 1 + @ticket3 = Ticket.create( + title: 'test 1234-2', + group: Group.lookup( name: 'Users'), + customer_id: @customer_with_org3.id, + state: Ticket::State.lookup( name: 'new' ), + priority: Ticket::Priority.lookup( name: '2 normal' ), + updated_by_id: 1, + created_by_id: 1, + ) + @article3 = Ticket::Article.create( + ticket_id: @ticket3.id, + from: 'some_sender3@example.com', + to: 'some_recipient3@example.com', + subject: 'some subject3', + message_id: 'some@id', + body: 'some message3', + internal: false, + sender: Ticket::Article::Sender.where(name: 'Customer').first, + type: Ticket::Article::Type.where(name: 'email').first, + updated_by_id: 1, + created_by_id: 1, + ) + end + + test 'settings index with nobody' do + + params = { + query: 'test 1234', + limit: 2, + } + + post '/api/v1/search/ticket', params.to_json, @headers + assert_response(401) + result = JSON.parse(@response.body) + assert_equal(result.class, Hash) + assert_not(result.empty?) + + post '/api/v1/search/user', params.to_json, @headers + assert_response(401) + result = JSON.parse(@response.body) + assert_equal(result.class, Hash) + assert_not(result.empty?) + + post '/api/v1/search', params.to_json, @headers + assert_response(401) + result = JSON.parse(@response.body) + assert_equal(result.class, Hash) + assert_not(result.empty?) + + end + + test 'settings index with admin' do + + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('search-admin@example.com', 'adminpw') + + params = { + query: '1234*', + limit: 1, + } + + post '/api/v1/search', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('User', result['result'][1]['type']) + assert_equal(@agent.id, result['result'][1]['id']) + assert_not(result['result'][2]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('Ticket', result['result'][1]['type']) + assert_equal(@ticket2.id, result['result'][1]['id']) + assert_equal('Ticket', result['result'][2]['type']) + assert_equal(@ticket1.id, result['result'][2]['id']) + assert_equal('User', result['result'][3]['type']) + assert_equal(@agent.id, result['result'][3]['id']) + assert_not(result['result'][4]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/ticket', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('Ticket', result['result'][1]['type']) + assert_equal(@ticket2.id, result['result'][1]['id']) + assert_equal('Ticket', result['result'][2]['type']) + assert_equal(@ticket1.id, result['result'][2]['id']) + assert_not(result['result'][3]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/user', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('User', result['result'][0]['type']) + assert_equal(@agent.id, result['result'][0]['id']) + assert_not(result['result'][1]) + + end + + test 'settings index with agent' do + + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('search-agent@example.com', 'agentpw') + + params = { + query: '1234*', + limit: 1, + } + + post '/api/v1/search', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('User', result['result'][1]['type']) + assert_equal(@agent.id, result['result'][1]['id']) + assert_not(result['result'][2]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('Ticket', result['result'][1]['type']) + assert_equal(@ticket2.id, result['result'][1]['id']) + assert_equal('Ticket', result['result'][2]['type']) + assert_equal(@ticket1.id, result['result'][2]['id']) + assert_equal('User', result['result'][3]['type']) + assert_equal(@agent.id, result['result'][3]['id']) + assert_not(result['result'][4]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/ticket', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('Ticket', result['result'][1]['type']) + assert_equal(@ticket2.id, result['result'][1]['id']) + assert_equal('Ticket', result['result'][2]['type']) + assert_equal(@ticket1.id, result['result'][2]['id']) + assert_not(result['result'][3]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/user', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('User', result['result'][0]['type']) + assert_equal(@agent.id, result['result'][0]['id']) + assert_not(result['result'][1]) + + end + + test 'settings index with customer 1' do + + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('search-customer1@example.com', 'customer1pw') + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket1.id, result['result'][0]['id']) + assert_not(result['result'][1]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/ticket', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket1.id, result['result'][0]['id']) + assert_not(result['result'][1]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/user', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_not(result['result'][0]) + + end + + test 'settings index with customer 2' do + + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('search-customer2@example.com', 'customer2pw') + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('Ticket', result['result'][1]['type']) + assert_equal(@ticket2.id, result['result'][1]['id']) + assert_not(result['result'][2]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/ticket', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert(result) + assert_equal('Ticket', result['result'][0]['type']) + assert_equal(@ticket3.id, result['result'][0]['id']) + assert_equal('Ticket', result['result'][1]['type']) + assert_equal(@ticket2.id, result['result'][1]['id']) + assert_not(result['result'][2]) + + params = { + query: '1234*', + limit: 10, + } + + post '/api/v1/search/user', params.to_json, @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_not(result['result'][0]) + + end + + +end