RES-1858 Switch to use mapbox for geocoding - #644
Conversation
|
SonarCloud Quality Gate failed.
|
|
SonarCloud Quality Gate failed.
|
The rest of the Mapbox geocoder plumbing (config/geocoder.php, the
geocoder-php/mapbox-provider + toin0u/geocoder-laravel packages, and
MAPBOX_TOKEN in .env/CI/Fly secrets) already landed on develop as part
of other work and is already used for reverse geocoding in
SetPlaceNetworkData. This just switches the remaining forward-geocode
path, App\Helpers\Geocoder::geocode(), from a raw Google Maps HTTP call
to the same app('geocoder') chain (Mapbox, falling back to GeoPlugin),
matching that existing pattern.
country_code now comes straight from the provider's ISO country code
rather than matching the returned country name against our own country
list, and the dead reverseGeocode() method (superseded by the
reverseQuery() call already in SetPlaceNetworkData) is removed.
Public interface, the Geocoder container binding, and the test mocks
(GeocoderMock, GeocoderFailMock) are unchanged, so no other files need
updating for this to work.
This branch is rebuilt on current develop (~1240 commits ahead of
where it last was) so CI can actually run; the frontend part of the
original PR (replacing vue-google-autocomplete with a Mapbox GL
widget) is intentionally not carried over - see PR comment.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
18af27c to
fcfbcc0
Compare
|
Rebuilt this branch on current While rebasing this, it turned out most of the Mapbox plumbing this PR wanted had already landed on So the only remaining piece of this PR's intent was the forward geocode path, Infra/build files (Dockerfile, docker-compose.yml, docker_run.sh, package.json/package-lock.json) are taken as-is from develop - the versions in this branch were three years stale and would only have caused conflicts. Not carried over: the third part of the original PR description - replacing Needs manual verification:
|
|
develop gained #904 (a fetch() seam so GeocoderTest could exercise the parsing without calling Google) while this branch was replacing the whole method with the geocoder package. Same method, incompatible approaches. Taking this branch's: with the provider chain resolved from the container there is nothing to seam - a test binds its own provider instead, which is what the code comment here already promised. That also drops googleKey(), and reverseGeocode(), which was defined and never called anywhere in the codebase. GeocoderTest is rewritten onto that mechanism: it builds real Geocoder\Model\Address objects and binds a provider returning them. Both container keys have to be replaced, not just one. geocoder-laravel binds the string 'geocoder' through to ProviderAndDumperAggregator and registers that as a singleton, so replacing only the string leaves the real Mapbox chain reachable - and a test that passes then is quietly talking to the network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AddressCollection::get(0) raises OutOfBounds on an empty collection, so an address Mapbox and GeoPlugin both fail to place crashed the request instead of returning false the way callers expect - and the way the Google implementation did, since it counted results before indexing. Caught by rewriting GeocoderTest onto a bound provider: the nothing-found case throws with the real code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
2 participants
(new PR after cherry picking some files for #640 )