Skip to content

fix(geocoding): support configured google maps locale in geocoding re… - #279

Merged
roncodes merged 2 commits into
fleetbase:dev-v0.6.59from
spanchenko:fix/google-maps-locale
Aug 2, 2026
Merged

roncodes merged 2 commits into
fleetbase:dev-v0.6.59from
spanchenko:fix/google-maps-locale

Conversation

@spanchenko

Copy link
Copy Markdown
Contributor

Description

Currently, Google Maps geocoding and reverse-geocoding requests in FleetOps always return address details and street names in English. This happens because Geocoding.php hardcodes 'en' as the locale for StatefulGeocoder and passes null as the region to the GoogleMaps provider, ignoring the system-configured services.google_maps.locale setting.

This PR introduces dynamic locale support by retrieving the configured locale (config('services.google_maps.locale', env('GOOGLE_MAPS_LOCALE', 'en'))) and passing it to both the GoogleMaps provider (for region bias) and StatefulGeocoder` (for localized language response).

Changes

  • Added Geocoding::getLocale() helper method to retrieve the configured locale.
  • Updated geocode(), reverseFromQuery(), and reverseFromCoordinates() in Geocoding.php to use getLocale() instead of hardcoded 'en' and null region parameters.

How to Test

  1. Configure Google Maps in System Settings (Settings -> Services -> Google Maps) with API key and set google_maps_locale to a non-English locale (e.g. ru, es, fr).
  2. Search for or save a new place via reverse geocoding or geocoder lookup.
  3. Verify that returned street names and address components are rendered in the configured locale instead of English.

@roncodes
roncodes changed the base branch from main to dev-v0.6.59 August 2, 2026 06:07
Resolves the conflict in Geocoding.php. The release branch centralised
geocoder construction into makeGeocoder() so a test double can be injected
through the fleetops.geocoder container binding, which removed the three
inline construction sites this PR edited. The locale logic now lives in
makeGeocoder(), so it applies to every caller rather than being repeated.

Two adjustments to the original change:

Locale and region are read from separate config keys. GoogleMaps' second
constructor argument is region biasing (a ccTLD such as `us`), not language —
language is the StatefulGeocoder argument. Passing one value to both worked
for locales that happen to also be ccTLDs (ru, es, fr) but the `en` default is
not a valid region, and it replaced the null passed previously. getLocale()
keeps this PR's key and drives language; getRegion() reads
services.google_maps.region and drives region bias.

Both getters use `?:` rather than a config() default. This key is stored in
the settings table, so it exists-but-empty on installs that never filled it
in, and config()'s default only applies when a key is absent entirely — a
stored empty value would otherwise return null from a `: string` method and
raise a TypeError on every geocode call.

Also drops a duplicated docblock left above getLocale(), and adds coverage
for both getters.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roncodes

roncodes commented Aug 2, 2026

Copy link
Copy Markdown
Member

Resolved the conflict with dev-v0.6.59 and pushed to this branch — it's mergeable again.

Why it conflicted: the release branch centralised geocoder construction into Geocoding::makeGeocoder() so a test double can be injected via the fleetops.geocoder container binding. That removed the three inline construction sites this PR edited. The locale logic now lives in makeGeocoder(), so it applies to every caller (and any future one) instead of being repeated three times.

Two adjustments worth flagging, since they change the original design:

1. Locale and region are now separate config keys. GoogleMaps::__construct(ClientInterface $client, ?string $region = null, ?string $apiKey = null) — the second argument is region biasing (a ccTLD like us), not language. Language is the StatefulGeocoder argument.

Passing one value to both works for locales that happen to also be ccTLDs (ru, es, fr), but the en default is not a valid region, and it would have replaced the null passed previously on every geocode call. So:

  • getLocale() — keeps your key (services.google_maps.locale / GOOGLE_MAPS_LOCALE, default en) and drives language, so your documented workflow still does what you described.
  • getRegion() — new key (services.google_maps.region / GOOGLE_MAPS_REGION, default us) drives region bias, matching what server/config/geocoder.php already does.

2. Both getters use ?: instead of a config() default. This key is stored in the settings table, so on installs that never filled it in it exists-but-empty — and config($key, $default) only falls back when a key is absent, not when it's null. A stored empty value would have returned null from a : string method and raised a TypeError on every geocode call. The added test covers exactly that case.

Also removed a duplicated docblock that ended up above getLocale(), and added coverage for both getters.

One thing to be aware of: region bias on this path changes from null to us when unset. That makes it consistent with geocoder.php and core-api (both already default to us), but it does bias ambiguous queries toward the US — set GOOGLE_MAPS_REGION to override.

Separately, and out of scope here: core-api's SettingController defaults the admin Locale field to us, so installs may have a ccTLD stored in what is now the language key. Worth validating or migrating that value at some point.

@roncodes
roncodes merged commit 7bd3130 into fleetbase:dev-v0.6.59 Aug 2, 2026
@spanchenko
spanchenko deleted the fix/google-maps-locale branch August 3, 2026 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants