Rethink fetching next page

Quite ugly but nothing better came to my mind atm

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)

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

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"
+  }
 }
 

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"
   }
 }

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
 

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