From 0fb11a116a49653671ed3efca715de53ee519415 Mon Sep 17 00:00:00 2001 From: Stefanni Date: Thu, 19 Jul 2018 19:05:41 -0700 Subject: [PATCH 1/2] Refactor typeahead service --- app/api/srch/shared_params.rb | 5 ++ app/api/srch/typeahead.rb | 84 +++++++++-------------- app/services/execute_typeahead.rb | 28 ++++++++ test/functional/search_api_test.rb | 1 - test/functional/typeahead_api_test.rb | 97 +++++++++++---------------- 5 files changed, 104 insertions(+), 111 deletions(-) create mode 100644 app/services/execute_typeahead.rb diff --git a/app/api/srch/shared_params.rb b/app/api/srch/shared_params.rb index 0a604f7014..fc3c367d0b 100644 --- a/app/api/srch/shared_params.rb +++ b/app/api/srch/shared_params.rb @@ -14,5 +14,10 @@ module SharedParams params :additional do optional :tagName, type: String, documentation: { example: 'awesome' } end + + params :commontypeahead do + requires :srchString, type: String, documentation: { example: 'Spec' } + optional :seq, type: Integer, documentation: { example: 995 } + end end end diff --git a/app/api/srch/typeahead.rb b/app/api/srch/typeahead.rb index 87226a83a3..3f1827a2b0 100644 --- a/app/api/srch/typeahead.rb +++ b/app/api/srch/typeahead.rb @@ -3,6 +3,9 @@ module Srch class Typeahead < Grape::API + # we are using a group of reusable parameters using a shared params helper + # see /app/api/srch/shared_params.rb + helpers SharedParams # Number of top values of each type to return TYPEAHEAD_LIMIT = 10 @@ -14,17 +17,10 @@ class Typeahead < Grape::API is_array: false, nickname: 'typeaheadGetAll' params do - requires :srchString, type: String, documentation: { example: 'Spec' } - optional :seq, type: Integer, documentation: { example: 995 } + use :commontypeahead end get :all do - sresult = TagList.new - sparms = SearchRequest.fromRequest(params) - if sparms.valid? - sresult = TypeaheadService.new.search_all(params[:srchString], TYPEAHEAD_LIMIT) - end - sresult.srchParams = sparms - present sresult, with: TagList::Entity + present Typeahead.execute(:all, params), with: TagList::Entity end # Request URL should be /api/typeahead/profiles?srchString=QRY&seq=KEYCOUNT @@ -33,17 +29,10 @@ class Typeahead < Grape::API is_array: false, nickname: 'typeaheadGetProfiles' params do - requires :srchString, type: String, documentation: { example: 'Spec' } - optional :seq, type: Integer, documentation: { example: 995 } + use :commontypeahead end get :profiles do - sresult = TagList.new - sparms = SearchRequest.fromRequest(params) - if sparms.valid? - sresult = TypeaheadService.new.search_profiles(params[:srchString], TYPEAHEAD_LIMIT) - end - sresult.srchParams = sparms - present sresult, with: TagList::Entity + present Typeahead.execute(:profiles, params), with: TagList::Entity end # Request URL should be /api/typeahead/notes?srchString=QRY&seq=KEYCOUNT @@ -52,17 +41,10 @@ class Typeahead < Grape::API is_array: false, nickname: 'typeaheadGetNotes' params do - requires :srchString, type: String, documentation: { example: 'Spec' } - optional :seq, type: Integer, documentation: { example: 995 } + use :commontypeahead end get :notes do - sresult = TagList.new - sparms = SearchRequest.fromRequest(params) - if sparms.valid? - sresult = TypeaheadService.new.search_notes(params[:srchString], TYPEAHEAD_LIMIT) - end - sresult.srchParams = sparms - present sresult, with: TagList::Entity + present Typeahead.execute(:notes, params), with: TagList::Entity end # Request URL should be /api/typeahead/questions?srchString=QRY&seq=KEYCOUNT @@ -71,17 +53,10 @@ class Typeahead < Grape::API is_array: false, nickname: 'typeaheadGetQuestions' params do - requires :srchString, type: String, documentation: { example: 'Spec' } - optional :seq, type: Integer, documentation: { example: 995 } + use :commontypeahead end get :questions do - sresult = TagList.new - sparms = SearchRequest.fromRequest(params) - if sparms.valid? - sresult = TypeaheadService.new.search_questions(params[:srchString], TYPEAHEAD_LIMIT) - end - sresult.srchParams = sparms - present sresult, with: TagList::Entity + present Typeahead.execute(:questions, params), with: TagList::Entity end # Request URL should be /api/typeahead/tags?srchString=QRY&seq=KEYCOUNT @@ -90,17 +65,10 @@ class Typeahead < Grape::API is_array: false, nickname: 'typeaheadGetTags' params do - requires :srchString, type: String, documentation: { example: 'Spec' } - optional :seq, type: Integer, documentation: { example: 995 } + use :commontypeahead end get :tags do - sresult = TagList.new - sparms = SearchRequest.fromRequest(params) - if sparms.valid? - TypeaheadService.new.search_tags(params[:srchString], TYPEAHEAD_LIMIT) - end - sresult.srchParams = sparms - present sresult, with: TagList::Entity + present Typeahead.execute(:tags, params), with: TagList::Entity end # Request URL should be /api/typeahead/comments?srchString=QRY&seq=KEYCOUNT @@ -109,20 +77,28 @@ class Typeahead < Grape::API is_array: false, nickname: 'typeaheadGetComments' params do - requires :srchString, type: String, documentation: { example: 'Spec' } - optional :seq, type: Integer, documentation: { example: 995 } + use :commontypeahead end get :comments do - sresult = TagList.new - sparms = SearchRequest.fromRequest(params) - if sparms.valid? - TypeaheadService.new.search_comments(params[:srchString], TYPEAHEAD_LIMIT) - end - sresult.srchParams = sparms - present sresult, with: TagList::Entity + present Typeahead.execute(:comments, params), with: TagList::Entity end # end of endpoint definitions end + + def self.execute(endpoint, params) + sresult = TagList.new + search_query = params[:srchString] + search_type = endpoint + search_criteria = SearchCriteria.new(search_query) + + if search_criteria.valid? + sresult = ExecuteTypeahead.new.by(search_type, search_criteria, TYPEAHEAD_LIMIT) + end + + sparms = SearchRequest.fromRequest(params) + sresult.srchParams = sparms + sresult + end end end diff --git a/app/services/execute_typeahead.rb b/app/services/execute_typeahead.rb new file mode 100644 index 0000000000..b065400673 --- /dev/null +++ b/app/services/execute_typeahead.rb @@ -0,0 +1,28 @@ +class ExecuteTypeahead + def by(type, search_criteria, limit) + execute(type, search_criteria, limit) + end + + private + + def execute(type, search_criteria, limit) + sservice = TypeaheadService.new + sresult = TagList.new + case type + when :all + sresult = sservice.search_all(search_criteria.query, limit) + when :profiles + sresult = sservice.search_profiles(search_criteria.query, limit) + when :notes + sresult = sservice.search_notes(search_criteria.query, limit) + when :questions + sresult = sservice.search_questions(search_criteria.query, limit) + when :tags + sresult = sservice.search_tags(search_criteria.query, limit) + when :comments + sresult = sservice.search_comments(search_criteria.query, limit) + else + sresult = [] + end + end +end diff --git a/test/functional/search_api_test.rb b/test/functional/search_api_test.rb index c2f60090a0..074bea3e4a 100644 --- a/test/functional/search_api_test.rb +++ b/test/functional/search_api_test.rb @@ -71,7 +71,6 @@ def app matcher = JsonExpressions::Matcher.new(pattern) json = JSON.parse(last_response.body) - puts json.inspect assert matcher =~ json end diff --git a/test/functional/typeahead_api_test.rb b/test/functional/typeahead_api_test.rb index 5afd8d9473..8bacbc24fb 100644 --- a/test/functional/typeahead_api_test.rb +++ b/test/functional/typeahead_api_test.rb @@ -1,9 +1,5 @@ require 'test_helper' -# TODO: Rework to get better inclusion of JsonExpressions pattern matching -# TODO: Add tests for negative matches--check echo of search parameters and null result -# HACK: Parameterize the 'get' URLs to make passing and changing test values easier - class TypeaheadApiTest < ActiveSupport::TestCase include Rack::Test::Methods @@ -11,128 +7,117 @@ def app Rails.application end - def setup - @stxt = 'lat' - @sprofile = 'adm' - @stags = 'everything' - @sseq = 7 - @scom = 'comm' - end - test 'typeahead all functionality' do - get '/api/typeahead/all?srchString=lat&seq=7' + get '/api/typeahead/all?srchString=Blog' assert last_response.ok? # Expected typeahead pattern pattern = { - # TODO: Need more/better understanding and data for the test database - # return will be nil for now - # items: nil, srchParams: { - srchString: @stxt, - seq: @sseq + srchString: 'Blog', + seq: nil }.ignore_extra_keys! }.ignore_extra_keys! matcher = JsonExpressions::Matcher.new(pattern) - assert matcher =~ JSON.parse(last_response.body) + json = JSON.parse(last_response.body) + + assert matcher =~ json end test 'typeahead profile functionality' do - get '/api/typeahead/profiles?srchString=adm&seq=7' + get '/api/typeahead/profiles?srchString=Jeff' assert last_response.ok? # Expected profile response pattern pattern = { - # TODO: Need more/better understanding and data for the test database - # return will be nil for now - # items: nil, srchParams: { - srchString: @sprofile, - seq: @sseq + srchString: 'Jeff', + seq: nil }.ignore_extra_keys! }.ignore_extra_keys! matcher = JsonExpressions::Matcher.new(pattern) - assert matcher =~ JSON.parse(last_response.body) + json = JSON.parse(last_response.body) + + assert matcher =~ json end test 'typeahead notes functionality' do - get '/api/typeahead/notes?srchString=lat&seq=7' + get '/api/typeahead/notes?srchString=Blog' assert last_response.ok? # Expected notes pattern pattern = { - # TODO: Need more/better understanding and data for the test database - # return will be nil for now - # items: nil, srchParams: { - srchString: @stxt, - seq: @sseq + srchString: 'Blog', + seq: nil }.ignore_extra_keys! }.ignore_extra_keys! matcher = JsonExpressions::Matcher.new(pattern) - assert matcher =~ JSON.parse(last_response.body) + json = JSON.parse(last_response.body) + + assert matcher =~ json end test 'typeahead questions functionality' do - get '/api/typeahead/questions?srchString=lat&seq=7' + get '/api/typeahead/questions?srchString=Question' assert last_response.ok? # Expected question pattern - # Returns null right now for test--need to set a better search sequence on demo seed data pattern = { - # TODO: Need more/better understanding and data for the test database - # return will be nil for now - # items: nil, srchParams: { - srchString: @stxt, - seq: @sseq - }.ignore_extra_keys! - }.ignore_extra_keys! + srchString: 'Question', + seq: nil, + }.ignore_extra_keys! + }.ignore_extra_keys! - matcher = JsonExpressions::Matcher.new(pattern) + matcher = JsonExpressions::Matcher.new(pattern) - assert matcher =~ JSON.parse(last_response.body) + json = JSON.parse(last_response.body) + + assert matcher =~ json end test 'typeahead tags functionality' do - get '/api/typeahead/tags?srchString=everything&seq=7' + get '/api/typeahead/tags?srchString=everything' assert last_response.ok? # Expected tag pattern pattern = { - # TODO: Need more/better understanding and data for the test database - # return will be nil for now - # items: nil, srchParams: { - srchString: @stags, - seq: @sseq + srchString: 'everything', + seq: nil }.ignore_extra_keys! }.ignore_extra_keys! matcher = JsonExpressions::Matcher.new(pattern) - assert matcher =~ JSON.parse(last_response.body) + json = JSON.parse(last_response.body) + + assert matcher =~ json end test 'typeahead comments functionality' do - get '/api/typeahead/comments?srchString=comm&seq=7' + get '/api/typeahead/comments?srchString=comment' assert last_response.ok? # Expected comment pattern pattern = { srchParams: { - srchString: @scom, - seq: @sseq + srchString: 'comment', + seq: nil }.ignore_extra_keys! - }.ignore_extra_keys! + }.ignore_extra_keys! + matcher = JsonExpressions::Matcher.new(pattern) - assert matcher =~ JSON.parse(last_response.body) + json = JSON.parse(last_response.body) + + assert matcher =~ json end end From fffc9ec9da98a9f3ea55c8719900ca23a41f9ad7 Mon Sep 17 00:00:00 2001 From: milaaraujo Date: Thu, 19 Jul 2018 19:56:02 -0700 Subject: [PATCH 2/2] Code climate fix --- app/services/execute_typeahead.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/services/execute_typeahead.rb b/app/services/execute_typeahead.rb index b065400673..a80a6d64fa 100644 --- a/app/services/execute_typeahead.rb +++ b/app/services/execute_typeahead.rb @@ -8,21 +8,21 @@ def by(type, search_criteria, limit) def execute(type, search_criteria, limit) sservice = TypeaheadService.new sresult = TagList.new - case type + result = case type when :all - sresult = sservice.search_all(search_criteria.query, limit) + sservice.search_all(search_criteria.query, limit) when :profiles - sresult = sservice.search_profiles(search_criteria.query, limit) + sservice.search_profiles(search_criteria.query, limit) when :notes - sresult = sservice.search_notes(search_criteria.query, limit) + sservice.search_notes(search_criteria.query, limit) when :questions - sresult = sservice.search_questions(search_criteria.query, limit) + sservice.search_questions(search_criteria.query, limit) when :tags - sresult = sservice.search_tags(search_criteria.query, limit) + sservice.search_tags(search_criteria.query, limit) when :comments - sresult = sservice.search_comments(search_criteria.query, limit) + sservice.search_comments(search_criteria.query, limit) else - sresult = [] + [] end end end