From 088b315391b696e71ccb19f999da6dd528f8978b Mon Sep 17 00:00:00 2001 From: Rafal Wojsznis Date: Mon, 2 Nov 2015 21:18:53 +0100 Subject: [PATCH] Rethink fetching next page Quite ugly but nothing better came to my mind atm --- lib/workable/client.rb | 29 +++++++++------------ lib/workable/collection.rb | 7 ++--- spec/fixtures/job_candidates.json | 5 +++- spec/fixtures/jobs.json | 2 +- spec/lib/workable/client_spec.rb | 38 ++++++++++++++++++---------- spec/lib/workable/collection_spec.rb | 8 +++--- 6 files changed, 50 insertions(+), 39 deletions(-) diff --git a/lib/workable/client.rb b/lib/workable/client.rb index 0aae628..1a2ba2f 100644 --- a/lib/workable/client.rb +++ b/lib/workable/client.rb @@ -63,12 +63,7 @@ def stages # @option params :created_after [Timestamp|Integer] Returns results created after the specified timestamp. # @option params :updated_after [Timestamp|Integer] Returns results updated after the specified timestamp. def jobs(params = {}) - response = get_request('jobs', params) - - build_collection( - @transform_to.apply(:job, response['jobs']), - __callee__, - response['paging']) + build_collection('jobs', :job, 'jobs', params) end # request detailed information about job @@ -105,12 +100,7 @@ def job_recruiters(shortcode) # @option params :created_after [Timestamp|Integer] Returns results created after the specified timestamp. # @option params :updated_after [Timestamp|Integer] Returns results updated after the specified timestamp. def job_candidates(shortcode, params = {}) - response = get_request("jobs/#{shortcode}/candidates", params) - - build_collection( - @transform_to.apply(:candidate, response['candidates']), - __callee__, - response['paging']) + build_collection("jobs/#{shortcode}/candidates", :candidate, 'candidates', params) end # return the full object of a specific candidate @@ -149,7 +139,7 @@ def api_url # do the get request to api def get_request(url, params = {}) params = URI.encode_www_form(params.keep_if { |k, v| k && v }) - full_url = [url, params].compact.join('?') + full_url = params.empty? ? url : [url, params].join('?') do_request(full_url, Net::HTTP::Get) end @@ -222,11 +212,16 @@ def transform_from(type, input) transform(@transform_from[type], input) end - def build_collection(data, method_name, paging = nil) + def build_collection(url, transform_mapping, root_key, params = {}) + url = url.gsub(/#{api_url}\/?/, '') + response = get_request(url, params) + Collection.new( - data, - method(method_name), - paging) + @transform_to.apply(transform_mapping, response[root_key]), + method(__callee__), + transform_mapping, + root_key, + response['paging']) end def configuration_error(message) diff --git a/lib/workable/collection.rb b/lib/workable/collection.rb index 22d3146..73707e6 100644 --- a/lib/workable/collection.rb +++ b/lib/workable/collection.rb @@ -5,12 +5,14 @@ class Collection attr_reader :data - def initialize(data, next_page_method, paging = nil) + def initialize(data, next_page_method, transform_mapping, root_key, paging = nil) @data = data if paging @next_page = paging['next'] @next_page_method = next_page_method + @transform_mapping = transform_mapping + @root_key = root_key end end @@ -21,8 +23,7 @@ def next_page? def fetch_next_page return unless next_page? - params = CGI.parse(URI.parse(@next_page).query) - @next_page_method.call(params) + @next_page_method.call(@next_page, @transform_mapping, @root_key) end end end diff --git a/spec/fixtures/job_candidates.json b/spec/fixtures/job_candidates.json index d97224c..2d92556 100644 --- a/spec/fixtures/job_candidates.json +++ b/spec/fixtures/job_candidates.json @@ -50,6 +50,9 @@ "created_at": "2015-07-08T00:00:00Z", "updated_at": "2015-07-08T14:46:48Z" } - ] + ], + "paging": { + "next": "https://www.workable.com/spi/v3/accounts/subdomain/jobs/subdomain/candidates?limit=3&since_id=2700d6df" + } } diff --git a/spec/fixtures/jobs.json b/spec/fixtures/jobs.json index 0681e06..16afacd 100644 --- a/spec/fixtures/jobs.json +++ b/spec/fixtures/jobs.json @@ -68,6 +68,6 @@ } ], "paging": { - "next": "https://www.workable.com/spi/v3/accounts/groove-tech/jobs?limit=3&since_id=2700d6df" + "next": "https://www.workable.com/spi/v3/accounts/subdomain/jobs?limit=3&since_id=2700d6df" } } diff --git a/spec/lib/workable/client_spec.rb b/spec/lib/workable/client_spec.rb index 608860c..fb9670b 100644 --- a/spec/lib/workable/client_spec.rb +++ b/spec/lib/workable/client_spec.rb @@ -129,8 +129,6 @@ end describe '#job_questions' do - let(:client) { described_class.new(api_key: 'test', subdomain: 'subdomain') } - it 'returns questions for given job' do stub_request(:get, 'https://www.workable.com/spi/v3/accounts/subdomain/jobs/03FF356C8B/questions') .to_return(status: 200, body: job_questions_json_fixture) @@ -172,22 +170,36 @@ end describe '#job_candidates' do - let(:client) { described_class.new(api_key: 'test', subdomain: 'subdomain') } + context 'happy path' do + let(:candidates){ client.job_candidates('03FF356C8B') } + before do + stub_request(:get, 'https://www.workable.com/spi/v3/accounts/subdomain/jobs/03FF356C8B/candidates') + .to_return(status: 200, body: job_candidates_json_fixture) + end + + it 'returns collection of candidates for given job' do + expect(candidates).to be_kind_of(Workable::Collection) + expect(candidates.data[0]['name']).to eq('Lakita Marrero') + end - it 'returns collection of candidates for given job' do - stub_request(:get, 'https://www.workable.com/spi/v3/accounts/subdomain/jobs/03FF356C8B/candidates') - .to_return(status: 200, body: job_candidates_json_fixture) + it 'includes next page method that returns next collection' do + stub_request(:get, 'https://www.workable.com/spi/v3/accounts/subdomain/jobs/subdomain/candidates?limit=3&since_id=2700d6df') + .with(headers: headers) + .to_return(status: 200, body: job_candidates_json_fixture) - candidates = client.job_candidates('03FF356C8B') - expect(candidates).to be_kind_of(Workable::Collection) - expect(candidates.data[0]['name']).to eq('Lakita Marrero') + next_candidates = candidates.fetch_next_page + expect(next_candidates).to be_kind_of(Workable::Collection) + expect(next_candidates.data).to be_kind_of(Array) + end end - it 'raises exception on to long requests' do - stub_request(:get, 'https://www.workable.com/spi/v3/accounts/subdomain/jobs/03FF356C8B/candidates') - .to_return(status: 503, body: '{"error":"Not authorized"}') + context 'sad path' do + it 'raises exception on to long requests' do + stub_request(:get, 'https://www.workable.com/spi/v3/accounts/subdomain/jobs/03FF356C8B/candidates') + .to_return(status: 503, body: '{"error":"Not authorized"}') - expect { client.job_candidates('03FF356C8B') }.to raise_error(Workable::Errors::RequestToLong) + expect { client.job_candidates('03FF356C8B') }.to raise_error(Workable::Errors::RequestToLong) + end end end diff --git a/spec/lib/workable/collection_spec.rb b/spec/lib/workable/collection_spec.rb index 579b1a1..83983c4 100644 --- a/spec/lib/workable/collection_spec.rb +++ b/spec/lib/workable/collection_spec.rb @@ -2,7 +2,7 @@ describe Workable::Collection do describe 'delegation' do - let(:collection){ described_class.new([1, 2, 3], double) } + let(:collection){ described_class.new([1, 2, 3], double, :job, 'jobs') } it 'delegates size to `data`' do expect(collection.size).to eq(3) @@ -27,13 +27,13 @@ describe '#fetch_next_page' do let(:next_method){ double } - let(:collection){ described_class.new([], next_method, paging) } + let(:collection){ described_class.new([], next_method, :job, 'jobs', paging) } context 'when next page is available' do let(:paging){ { 'next' => 'http://example.com?limit=2&since_id=3' } } - it 'executes next_method with next page url params' do - expect(next_method).to receive(:call).with('limit' => ['2'], 'since_id' => ['3']) + it 'executes next_method with next page url' do + expect(next_method).to receive(:call).with('http://example.com?limit=2&since_id=3', :job, 'jobs') collection.fetch_next_page end end