-
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?
Conversation
4d290b5
to
cedac5a
Compare
GeoLocalAuthority.delete_all | ||
end | ||
|
||
def self.read_file(geojson_path) |
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.
Would move this method below load_geojsons
method as its helper.
end | ||
|
||
def self.empty_database | ||
GeoLocalAuthority.delete_all |
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.
|
||
features = read_file(geojson_path) | ||
|
||
features.each do |feature| |
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.
Would it make sense to wrap it into transaction?
|
||
SRID = 27_700 | ||
|
||
def self.nearest_three_to(string) |
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.
Maybe self.nearest_to(string, amount: 3)
instead hardcoding argument in method name?
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 comment
The 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 comment
The 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
Context
Ticket: https://dfedigital.atlassian.net/browse/CPDNPQ-1078
This is not to be merged into NPQ, this code will be used in another repository that will be set up later. That repo will use this code for searching local authorities.
Changes proposed in this pull request
This spike uses activerecord-postgis-adapter, geocoder, and rgeo to translate free text user location search into a list of the nearest local authorities.
The GeoJSON file available from the GOV Open Geography Portal is loaded into the DB by
Services::GeojsonLoader
using the PostGIS adapter provided byactiverecord-postgis-adapter
.Geocoder is used to convert entered user searches into lon/lat coordinates.
Rgeo is then used to transform those lon/lat coordinates into EPSG:27700 British National Grid projection coordinates. These coordinates once transformed line up to the projection stored in the database which is also the 27700 projection.
The transformed coordinates are then used in a distance query using <-> to find the nearest local authority geometries.
Note that the database adaptor had to be swapped over from postgres to postgis to enable this functionality.