diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf39c05..b33dc1ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased + +* BREAKING: Changes to Imminence API to support split-postcodes. In practice, the only consumer app for this API is Frontend, which is prepared for the change. + # 85.0.1 * Remove expectation that Imminence will return the OID field in the response to #places. This is an internal Mongoid field that we shouldn't be returning anyway, and frontend (the consumer) isn't using it. diff --git a/lib/gds_api/imminence.rb b/lib/gds_api/imminence.rb index 0c9e0ce4..70c9861a 100644 --- a/lib/gds_api/imminence.rb +++ b/lib/gds_api/imminence.rb @@ -2,55 +2,24 @@ class GdsApi::Imminence < GdsApi::Base def api_url(type, params) - vals = %i[limit lat lng postcode].select { |p| params.include? p } + vals = %i[limit lat lng postcode local_authority_slug].select { |p| params.include? p } querystring = URI.encode_www_form(vals.map { |p| [p, params[p]] }) "#{@endpoint}/places/#{type}.json?#{querystring}" end def places(type, lat, lon, limit = 5) url = api_url(type, lat: lat, lng: lon, limit: limit) - places = get_json(url) || [] - places.map { |p| self.class.parse_place_hash(p) } + get_json(url) end - def places_for_postcode(type, postcode, limit = 5) - url = api_url(type, postcode: postcode, limit: limit) - places = get_json(url) || [] - places.map { |p| self.class.parse_place_hash(p) } - end - - def self.parse_place_hash(place_hash) - location = extract_location_hash(place_hash["location"]) - address = extract_address_hash(place_hash) - - place_hash.merge(location).merge(address) + def places_for_postcode(type, postcode, limit = 5, local_authority_slug = nil) + options = { postcode: postcode, limit: limit } + options.merge!(local_authority_slug: local_authority_slug) if local_authority_slug + url = api_url(type, options) + get_json(url) || [] end def places_kml(type) get_raw("#{@endpoint}/places/#{type}.kml").body end - - # @private - def self.extract_location_hash(location) - # Deal with all known location formats: - # Old style: [latitude, longitude]; empty array for no location - # New style: hash with keys "longitude", "latitude"; nil for no location - case location - when Array - { "latitude" => location[0], "longitude" => location[1] } - when Hash - location - when nil - { "latitude" => nil, "longitude" => nil } - end - end - - # @private - def self.extract_address_hash(place_hash) - address_fields = [ - place_hash["address1"], - place_hash["address2"], - ].reject { |a| a.nil? || a == "" } - { "address" => address_fields.map(&:strip).join(", ") } - end end diff --git a/lib/gds_api/test_helpers/imminence.rb b/lib/gds_api/test_helpers/imminence.rb index ba12e97c..7d970bf5 100644 --- a/lib/gds_api/test_helpers/imminence.rb +++ b/lib/gds_api/test_helpers/imminence.rb @@ -9,12 +9,33 @@ module Imminence def stub_imminence_has_places(latitude, longitude, details) query_hash = { "lat" => latitude, "lng" => longitude, "limit" => "5" } - stub_imminence_places_request(details["slug"], query_hash, details["details"]) + response_data = { + status: "ok", + content: "places", + places: details["details"], + } + stub_imminence_places_request(details["slug"], query_hash, response_data) end - def stub_imminence_has_places_for_postcode(places, slug, postcode, limit) + def stub_imminence_has_multiple_authorities_for_postcode(addresses, slug, postcode, limit) query_hash = { "postcode" => postcode, "limit" => limit } - stub_imminence_places_request(slug, query_hash, places) + response_data = { + status: "address-information-required", + content: "addresses", + addresses: addresses, + } + stub_imminence_places_request(slug, query_hash, response_data) + end + + def stub_imminence_has_places_for_postcode(places, slug, postcode, limit, local_authority_slug) + query_hash = { "postcode" => postcode, "limit" => limit } + query_hash.merge!(local_authority_slug: local_authority_slug) if local_authority_slug + response_data = { + status: "ok", + content: "places", + places: places, + } + stub_imminence_places_request(slug, query_hash, response_data) end def stub_imminence_places_request(slug, query_hash, return_data, status_code = 200) diff --git a/test/imminence_api_test.rb b/test/imminence_api_test.rb index 2a897afd..8d45448e 100644 --- a/test/imminence_api_test.rb +++ b/test/imminence_api_test.rb @@ -18,7 +18,7 @@ def dummy_place "fax" => nil, "general_notes" => nil, "geocode_error" => nil, - "location" => [LATITUDE, LONGITUDE], + "location" => { "latitude" => LATITUDE, "longitude" => LONGITUDE }, "name" => "Town Hall", "phone" => nil, "postcode" => "MK42 9AP", @@ -29,42 +29,25 @@ def dummy_place } end - def test_no_second_address_line - c = api_client - url = "#{ROOT}/places/wibble.json?limit=5&lat=52&lng=0" - place_info = dummy_place.merge "address2" => nil - c.expects(:get_json).with(url).returns([place_info]) - places = c.places("wibble", 52, 0) - - assert_equal 1, places.size - assert_equal "Cauldwell Street", places[0]["address"] + def dummy_place_response(place_array) + { + "status" => "ok", + "contents" => "places", + "places" => place_array, + } end def test_search_for_places c = api_client url = "#{ROOT}/places/wibble.json?limit=5&lat=52&lng=0" - c.expects(:get_json).with(url).returns([dummy_place]) - places = c.places("wibble", 52, 0) + c.expects(:get_json).with(url).returns(dummy_place_response([dummy_place])) + output = c.places("wibble", 52, 0) + places = output["places"] assert_equal 1, places.size place = places[0] - assert_equal LATITUDE, place["latitude"] - assert_equal LONGITUDE, place["longitude"] - assert_equal "Cauldwell Street, Bedford", place["address"] - end - - def test_empty_location - # Test behaviour when the location field is an empty array - c = api_client - url = "#{ROOT}/places/wibble.json?limit=5&lat=52&lng=0" - place_info = dummy_place.merge("location" => []) - c.expects(:get_json).with(url).returns([place_info]) - places = c.places("wibble", 52, 0) - - assert_equal 1, places.size - place = places[0] - assert_nil place["latitude"] - assert_nil place["longitude"] + assert_equal LATITUDE, place["location"]["latitude"] + assert_equal LONGITUDE, place["location"]["longitude"] end def test_nil_location @@ -72,8 +55,9 @@ def test_nil_location c = api_client url = "#{ROOT}/places/wibble.json?limit=5&lat=52&lng=0" place_info = dummy_place.merge("location" => nil) - c.expects(:get_json).with(url).returns([place_info]) - places = c.places("wibble", 52, 0) + c.expects(:get_json).with(url).returns(dummy_place_response([place_info])) + output = c.places("wibble", 52, 0) + places = output["places"] assert_equal 1, places.size place = places[0] @@ -88,21 +72,34 @@ def test_hash_location place_info = dummy_place.merge( "location" => { "longitude" => LONGITUDE, "latitude" => LATITUDE }, ) - c.expects(:get_json).with(url).returns([place_info]) - places = c.places("wibble", 52, 0) + c.expects(:get_json).with(url).returns(dummy_place_response([place_info])) + output = c.places("wibble", 52, 0) + places = output["places"] assert_equal 1, places.size place = places[0] - assert_equal LATITUDE, place["latitude"] - assert_equal LONGITUDE, place["longitude"] + assert_equal LATITUDE, place["location"]["latitude"] + assert_equal LONGITUDE, place["location"]["longitude"] end def test_postcode_search # Test behaviour when searching by postcode c = api_client url = "#{ROOT}/places/wibble.json?limit=5&postcode=MK42+9AA" - c.expects(:get_json).with(url).returns([dummy_place]) - places = c.places_for_postcode("wibble", "MK42 9AA") + c.expects(:get_json).with(url).returns(dummy_place_response([dummy_place])) + output = c.places_for_postcode("wibble", "MK42 9AA") + places = output["places"] + + assert_equal 1, places.size + end + + def test_postcode_with_local_authority_search + # Test behaviour when searching by postcode + c = api_client + url = "#{ROOT}/places/wibble.json?limit=10&postcode=MK42+9AA&local_authority_slug=broadlands" + c.expects(:get_json).with(url).returns(dummy_place_response([dummy_place])) + output = c.places_for_postcode("wibble", "MK42 9AA", 10, "broadlands") + places = output["places"] assert_equal 1, places.size end diff --git a/test/pacts/imminence_api_pact_test.rb b/test/pacts/imminence_api_pact_test.rb index 02bbe89c..361e891c 100644 --- a/test/pacts/imminence_api_pact_test.rb +++ b/test/pacts/imminence_api_pact_test.rb @@ -10,7 +10,7 @@ it "responds with all responses for the given dataset" do imminence_api .given("a service exists called number-plate-supplier with places") - .upon_receiving("the request to retrieve all places for the current dataset") + .upon_receiving("the request to retrieve relevant places for the current dataset for a lat/lon") .with( method: :get, path: "/places/number-plate-supplier.json", @@ -19,30 +19,34 @@ ) .will_respond_with( status: 200, - body: Pact.each_like( - { - access_notes: nil, - address1: "Yarrow Road Tower Park", - address2: nil, - data_set_version: 473, - email: nil, - fax: nil, - general_notes: nil, - geocode_error: nil, - location: { longitude: -1.9552618901330387, latitude: 50.742754933617285 }, - name: "Breeze Motor Co Ltd", - override_lat: nil, - override_lng: nil, - phone: "01202 713000", - postcode: "BH12 4QY", - service_slug: "number-plate-supplier", - snac: nil, - source_address: "Yarrow Road Tower Park Poole BH12 4QY", - text_phone: nil, - town: "Yarrow", - url: nil, - }, - ), + body: { + status: "ok", + contents: "places", + places: Pact.each_like( + { + access_notes: nil, + address1: "Yarrow Road Tower Park", + address2: nil, + data_set_version: 473, + email: nil, + fax: nil, + general_notes: nil, + geocode_error: nil, + location: { longitude: -1.9552618901330387, latitude: 50.742754933617285 }, + name: "Breeze Motor Co Ltd", + override_lat: nil, + override_lng: nil, + phone: "01202 713000", + postcode: "BH12 4QY", + service_slug: "number-plate-supplier", + snac: nil, + source_address: "Yarrow Road Tower Park Poole BH12 4QY", + text_phone: nil, + town: "Yarrow", + url: nil, + }, + ), + }, headers: { "Content-Type" => "application/json; charset=utf-8", }, @@ -50,5 +54,35 @@ api_client.places("number-plate-supplier", "-2.01", "53.1", "5") end + + it "responds with a choice of addresses for disambiguation of split postcodes" do + imminence_api + .given("a service exists called register office exists with places, and CH25 9BJ is a split postcode") + .upon_receiving("the request to retrieve relevant places for the current dataset for CH25 9BJ") + .with( + method: :get, + path: "/places/register-office.json", + headers: GdsApi::JsonClient.default_request_headers, + query: { postcode: "CH25 9BJ", limit: "5" }, + ) + .will_respond_with( + status: 200, + body: { + status: "address-information-required", + contents: "addresses", + addresses: Pact.each_like( + { + address: "HOUSE 1", + local_authority_slug: "achester", + }, + ), + }, + headers: { + "Content-Type" => "application/json; charset=utf-8", + }, + ) + + api_client.places_for_postcode("register-office", "CH25 9BJ") + end end end