aligent / magento-stockists-module

GNU General Public License v3.0
1 stars 1 forks source link

GoogleMapsAdapter::performGeocode always returns empty results #15

Closed brettlaishley closed 2 years ago

brettlaishley commented 2 years ago

https://github.com/aligent/magento-stockists-module/blob/main/Model/GoogleMapsAdapter.php#L113

        if ($response['status'] !== self::STATUS_CODE_200) {

Some painful investigation has determined that back in 1.2.4 (old bitbucket repo) this line of code is causing errors. In the newer versions of this module it appears to silently fail and leave the lat/lng as 0 or null.

From our investigations back in v1.2.3 $httpClient->getStatus() did return 200 however with the later changes $response['status'] returns "OK" and automatically dismisses all geocode results.

@aligent-lturner I suspect you'd like to run some tests yourself, but I think it should be as simple as changing STATUS_CODE_200 to STATUS_CODE_OK.

Documentation available here showing "OK" is the returned status: https://developers.google.com/maps/documentation/javascript/geocoding#GeocodingStatusCodes

brettlaishley commented 2 years ago

In addition to this problem @aligent-lturner, I believe if a non 200/OK response is encountered, a different error is returned. Follow the flow here: \Aligent\Stockists\Model\GoogleMapsAdapter::handleResponse

        if ($location) {
            $result->setData($response["status"], $location["lat"], $location["lng"]);
        }

If no location is found the setData() function is never called, so when $result->wasSuccessful() is called back in GeocodeStockist::execute(), $result->getStatus() returns a null instead of a string.

aligent-lturner commented 2 years ago

In addition to this problem @aligent-lturner, I believe if a non 200/OK response is encountered, a different error is returned. Follow the flow here: \Aligent\Stockists\Model\GoogleMapsAdapter::handleResponse

        if ($location) {
            $result->setData($response["status"], $location["lat"], $location["lng"]);
        }

If no location is found the setData() function is never called, so when $result->wasSuccessful() is called back in GeocodeStockist::execute(), $result->getStatus() returns a null instead of a string.

This is fixed in the same PR (#16 ) - $result->getStatus() will now always return a string.