Skip to content

Commit

Permalink
Remove preprocessing of imminence responses
Browse files Browse the repository at this point in the history
Allow more complex responses by removing preprocessing code. Also remove obsolete location handling code, and update pact tests to include address disambiguation response.
  • Loading branch information
KludgeKML committed Jan 26, 2023
1 parent 0dcd831 commit ad0171f
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 102 deletions.
37 changes: 2 additions & 35 deletions lib/gds_api/imminence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,50 +9,17 @@ def api_url(type, params)

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, 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)
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)
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
21 changes: 18 additions & 3 deletions lib/gds_api/test_helpers/imminence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +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_multiple_authorities_for_postcode(addresses, slug, postcode, limit)
query_hash = { "postcode" => postcode, "limit" => limit }
stub_imminence_places_request(slug, query_hash, addresses)
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
stub_imminence_places_request(slug, query_hash, places)
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)
Expand Down
66 changes: 27 additions & 39 deletions test/imminence_api_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require "test_helper"
require "gds_api/imminence"
require "byebug"

class ImminenceApiTest < Minitest::Test
ROOT = Plek.find("imminence")
Expand All @@ -18,7 +19,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",
Expand All @@ -29,51 +30,35 @@ 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
# Test behaviour when the location field is nil
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]
Expand All @@ -88,21 +73,23 @@ 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
Expand All @@ -111,8 +98,9 @@ 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])
places = c.places_for_postcode("wibble", "MK42 9AA", 10, "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
Expand Down
84 changes: 59 additions & 25 deletions test/pacts/imminence_api_pact_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -19,36 +19,70 @@
)
.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",
},
)

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

0 comments on commit ad0171f

Please sign in to comment.