-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Do not merge] [CPDNPQ-1078] [SPIKE] Implement geocoded local authority search #787
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
module Services | ||
class GeojsonLoader | ||
def self.reload_geojsons(geojson_path) | ||
empty_database | ||
load_geojsons(geojson_path) | ||
end | ||
|
||
def self.empty_database | ||
GeoLocalAuthority.delete_all | ||
end | ||
|
||
def self.read_file(geojson_path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would move this method below |
||
file = File.read(geojson_path) | ||
features = RGeo::GeoJSON.decode(file, json_parser: :json) | ||
end | ||
|
||
def self.load_geojsons(geojson_path) | ||
Rails.logger.debug("Loading #{geojson_path}") | ||
|
||
features = read_file(geojson_path) | ||
|
||
features.each do |feature| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to wrap it into transaction? |
||
geo_local_authority = GeoLocalAuthority.new | ||
geo_local_authority.name = feature.properties["LAD22NM"] | ||
geo_local_authority.geometry = feature.geometry | ||
geo_local_authority.save! | ||
end | ||
end | ||
|
||
class Shape | ||
attr_reader :geojson_record | ||
|
||
def initialize(geojson_record) | ||
@geojson_record = geojson_record | ||
end | ||
|
||
delegate :geometry, | ||
to: :geojson_record | ||
|
||
def name | ||
geojson_record.attributes["LAD22NM"] | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
class GeoLocalAuthority < ApplicationRecord | ||
include RGeo::ActiveRecord::GeometryMixin | ||
|
||
SRID = 27_700 | ||
|
||
def self.nearest_three_to(string) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||
location = Geocoder.search(string).first | ||
by_distance(location.longitude.to_f, location.latitude.to_f).limit(3) | ||
end | ||
|
||
def self.by_distance(longitude, latitude) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How many results does it returns by default? Could it turn out to be expensive as do extensive search? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't really return any results - it returns AR scope. The scope itslef uses some of the postgis magic which is rathere quite performant. I wouldn't expect any delays based on the size of the database in this service |
||
# Transform the given lon/lat to have an SRID of 27700 (the SRID of the GeoLocalAuthority geometries) | ||
transformed_point = RGeo::CoordSys::Proj4.transform_coords( | ||
RGeo::CoordSys::Proj4.new("EPSG:4326"), | ||
RGeo::CoordSys::Proj4.new("EPSG:27700"), | ||
longitude, | ||
latitude, | ||
) | ||
|
||
target_latitude = transformed_point[1] | ||
target_longitude = transformed_point[0] | ||
|
||
# Create a point representing the target coordinates, with the SRID specified | ||
# If you don't specify the SRID it will default to 0, which will cause an error | ||
point_sql = "SRID=#{SRID};POINT(#{target_longitude} #{target_latitude})" | ||
|
||
arel = Arel.sql("geometry <-> '#{point_sql}'::geometry") | ||
|
||
order(arel) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class EnablePostgis < ActiveRecord::Migration[6.1] | ||
def change | ||
enable_extension "postgis" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
class CreateGeoLocalAuthorities < ActiveRecord::Migration[6.1] | ||
def change | ||
create_table :geo_local_authorities do |t| | ||
t.string :name | ||
t.geometry :geometry, srid: 27_700 | ||
|
||
t.timestamps | ||
end | ||
|
||
add_index :geo_local_authorities, :geometry, using: :gist | ||
end | ||
end |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
namespace :geojson do | ||
desc "Load geojsons into the database, will empty local authorities table first" | ||
task load: :environment do |_t, _args| | ||
geojson_path = "lib/local_authority_geojson/Local_Authority_Districts_December_2022_Boundaries_UK_BUC_143497700576642915.geojson" | ||
|
||
puts("Loading geojsons from #{geojson_path}") | ||
|
||
Services::GeojsonLoader.reload_geojsons(geojson_path) | ||
end | ||
|
||
desc "Load geojson file and output local authorities found" | ||
task test: :environment do |_t, _args| | ||
geojson_path = "lib/local_authority_geojson/Local_Authority_Districts_December_2022_Boundaries_UK_BUC_143497700576642915.geojson" | ||
|
||
features = Services::GeojsonLoader.read_file(geojson_path) | ||
|
||
puts "File contains #{features.count} local authority geometries" | ||
|
||
local_authorities_found = features.map { |feature| feature.properties["LAD22NM"] }.sort.join(", ") | ||
|
||
puts "Local Authorities found: #{local_authorities_found}" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
require "rails_helper" | ||
|
||
RSpec.describe GeoLocalAuthority, type: :model do | ||
before do | ||
geojson_path = "lib/local_authority_geojsons/Local_Authority_Districts_December_2022_Boundaries_UK_BUC_143497700576642915.geojson" | ||
|
||
Services::GeojsonLoader.reload_geojsons(geojson_path) | ||
|
||
Geocoder.configure(lookup: :test) | ||
Geocoder::Lookup::Test.add_stub( | ||
"Department for Education, Westminster", [{ | ||
"latitude" => 51.4979314, | ||
"longitude" => -0.13016544222858017, | ||
"address" => "Department for Education, 20, Great Smith Street, Westminster, Millbank, London, Greater London, England, SW1P 3BT, United Kingdom", | ||
"state" => "England", | ||
"country" => "United Kingdom", | ||
"country_code" => "gb", | ||
}] | ||
) | ||
Geocoder::Lookup::Test.add_stub( | ||
"Manchester Airport", [{ | ||
"latitude" => 53.350342049999995, | ||
"longitude" => -2.280369252664295, | ||
"address" => "Manchester Airport, Castle Mill Lane, Ashley, Manchester, Cheshire East, England, M90 1QX, United Kingdom", | ||
"state" => "England", | ||
"country" => "United Kingdom", | ||
"country_code" => "gb", | ||
}] | ||
) | ||
Geocoder::Lookup::Test.add_stub( | ||
"Belfast City Airport", [{ | ||
"latitude" => 54.614828, | ||
"longitude" => -5.8703437, | ||
"address" => "Bushmills Bar, Sydenham Bypass, Sydenham, Belfast, County Down, Ulster, Northern Ireland, BT3 9JH, United Kingdom", | ||
"state" => "Northern Ireland", | ||
"country" => "United Kingdom", | ||
"country_code" => "gb", | ||
}] | ||
) | ||
Geocoder::Lookup::Test.add_stub( | ||
"Edinburgh Airport", [{ | ||
"latitude" => 55.950128899999996, | ||
"longitude" => -3.3595079855289756, | ||
"address" => "Edinburgh Airport, Meadowfield Road, Gogar, City of Edinburgh, Scotland, EH12 0AU, United Kingdom", | ||
"state" => "Scotland", | ||
"country" => "United Kingdom", | ||
"country_code" => "gb", | ||
}] | ||
) | ||
Geocoder::Lookup::Test.add_stub( | ||
"Cardiff Airport", [{ | ||
"latitude" => 51.397871550000005, | ||
"longitude" => -3.3445890119919994, | ||
"address" => "Cardiff Airport, B4265, Fonmon, Rhoose, Penmark, Vale of Glamorgan, Wales, CF62 3BL, United Kingdom", | ||
"state" => "Wales", | ||
"country" => "United Kingdom", | ||
"country_code" => "gb", | ||
}] | ||
) | ||
|
||
Geocoder::Lookup::Test.add_stub( | ||
"Sydney Opera House", [{ | ||
"latitude" => -33.85719805, | ||
"longitude" => 151.21512338473752, | ||
"address" => "Sydney Opera House, 2, Macquarie Street, Quay Quarter, Sydney, Council of the City of Sydney, New South Wales, 2000, Australia", | ||
"state" => "New South Wales", | ||
"country" => "Australia", | ||
"country_code" => "au", | ||
}] | ||
) | ||
end | ||
|
||
describe "#nearest_three_to" do | ||
def check_nearest_three_to(location) | ||
described_class.nearest_three_to(location).pluck(:name) | ||
end | ||
|
||
it "returns the three nearest local authorities" do | ||
# Do some quick checks around the UK to check it works in various areas | ||
expect(check_nearest_three_to("Department for Education, Westminster")).to eq(%w[Westminster Lambeth Wandsworth]) | ||
expect(check_nearest_three_to("Manchester Airport")).to eq(["Manchester", "Cheshire East", "Trafford"]) | ||
expect(check_nearest_three_to("Belfast City Airport")).to eq(["Belfast", "Ards and North Down", "Lisburn and Castlereagh"]) | ||
expect(check_nearest_three_to("Edinburgh Airport")).to eq(["City of Edinburgh", "West Lothian", "Fife"]) | ||
expect(check_nearest_three_to("Cardiff Airport")).to eq(["Vale of Glamorgan", "Cardiff", "Rhondda Cynon Taf"]) | ||
|
||
# And one across the world to check it doesn't break | ||
expect(check_nearest_three_to("Sydney Opera House")).to eq(["Cornwall", "Isles of Scilly", "South Hams"]) | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider inlining instead of separate method.