From 3afaf33db2f7dd94dd34575ea7f034319922c4df Mon Sep 17 00:00:00 2001 From: TLabutis Date: Thu, 3 Sep 2026 22:50:17 +0300 Subject: [PATCH] Guard pickup point lookup miss in checkout parcel shop selection When the city and street posted from checkout match no row in dpd_shop, ParcelShopRepository::getIdByCityAndStreet returns false. That false was passed straight through DPDShop::getShopByPudoId, whose getFirst() also returns false, so $pudo->longitude and $pudo->latitude resolved to null. getClosestPudoShops then interpolated pSQL(null) into the haversine query, producing sin(( * pi() / 180)) and a SQL syntax error. executeS returned false, and ShopFactory::createShop is type-hinted array, so the request died with a TypeError and a 500. The failure was silent rather than merely broken because all three error handlers in pudo-search.js called DPDdisplayMessage($container, ...) while $container was declared in only one of the three functions, and even there from $(this).closest(), which resolves to an empty set in a plain function call. The other two threw a ReferenceError. Each handler also parsed response.responseText unguarded, and a PHP fatal returns an HTML error page, so the parse threw first. The customer saw an empty map and no message at all. PudoService::getClosestParcelShops now returns an empty array when the pudo is not a loaded object or the query fails, which removes the fatal. saveParcelShop turns the empty result into a status:false payload with a translated message, matching the !$isSuccess guard directly above it. Each error handler now scopes its own container lookup at response time, matching what the success handlers already did, and guards the parse. Also reordered the GoogleApiService constructor so isSslEnabled is assigned before getGeolocationUrl reads it, and dropped the branch that appended a second "s" to an already-complete "https" scheme. These cancelled each other out, so shipped output was always a correct https:// and this change is behaviour-neutral. Reordering alone would have introduced httpss:// on every SSL-everywhere shop, hence both. Not addressed: why the city and street data diverged after the 3.3.1 upgrade. This is a defensive fix, not a confirmed root-cause fix. The rewritten locker fetch path is the prime suspect but confirming it needs merchant environment details that are not in the ticket. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GUhfYfmoGVzjN1xL7S2Am8 --- CHANGELOG.md | 4 ++++ controllers/front/Ajax.php | 21 +++++++++------- src/Service/GoogleApiService.php | 9 ++----- src/Service/PudoService.php | 9 +++++++ views/js/front/pudo-search.js | 41 +++++++++++++++++++++++--------- 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d64c9d10..46cde0b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -204,3 +204,7 @@ - Added timeframes to shipment request - Fixed phone area code, city and street select width rendering as 0px when switching carriers in checkout - Fixed DPDBaltics menu item disappearing from sidebar when navigating to module pages + +## [3.3.2] +- Fixed checkout pickup point selection returning a 500 error when the selected city and street do not match an imported pickup point +- Fixed pickup point error messages not being displayed in checkout when a request fails diff --git a/controllers/front/Ajax.php b/controllers/front/Ajax.php index 0a911b87..a6e62d58 100644 --- a/controllers/front/Ajax.php +++ b/controllers/front/Ajax.php @@ -334,15 +334,20 @@ private function saveParcelShop($countryCode, $city, $street) $pudoId = $pudoService->getPudoIdByCityAndAddress($city, $street); $parcelShops = $pudoService->getClosestParcelShops($pudoId); - $coordinates = []; - $selectedPudo = null; - if (isset($parcelShops[0])) { - $coordinates = [ - 'lat' => $parcelShops[0]->getLatitude(), - 'lng' => $parcelShops[0]->getLongitude(), - ]; - $selectedPudo = $parcelShops[0]; + + if (!isset($parcelShops[0])) { + $this->messages[] = $this->module->l('No pickup points found for the selected address.', self::FILENAME); + $this->ajaxDie(json_encode([ + 'template' => $this->getMessageTemplate('danger'), + 'status' => false + ])); } + + $selectedPudo = $parcelShops[0]; + $coordinates = [ + 'lat' => $selectedPudo->getLatitude(), + 'lng' => $selectedPudo->getLongitude(), + ]; $pudoServices = $pudoService->setPudoServiceTypes($parcelShops); $pudoServices = $pudoService->formatPudoServicesWorkHours($pudoServices); diff --git a/src/Service/GoogleApiService.php b/src/Service/GoogleApiService.php index 57fffa62..1307a90b 100644 --- a/src/Service/GoogleApiService.php +++ b/src/Service/GoogleApiService.php @@ -51,9 +51,9 @@ class GoogleApiService public function __construct(Language $language, Shop $shop) { $apiKey = Configuration::get(Config::GOOGLE_API_KEY); - $this->geolocationApi = $this->getGeolocationUrl($apiKey); $this->isSslEnabled = (Configuration::get('PS_SSL_ENABLED')) && Configuration::get('PS_SSL_ENABLED_EVERYWHERE'); + $this->geolocationApi = $this->getGeolocationUrl($apiKey); $this->language = $language; $this->shop = $shop; } @@ -191,13 +191,8 @@ public function getResultFromGoogleApiService($requestStringified) private function getGeolocationUrl($apiKey) { - $url = 'https'; - if ($this->isSslEnabled) { - $url .='s'; - } - $url .= '://maps.googleapis.com/maps/api/geocode/json?key='. + return 'https://maps.googleapis.com/maps/api/geocode/json?key='. $apiKey.'&sensor=false&address='; - return $url; } diff --git a/src/Service/PudoService.php b/src/Service/PudoService.php index 3f0c5641..81aca942 100644 --- a/src/Service/PudoService.php +++ b/src/Service/PudoService.php @@ -43,6 +43,7 @@ use Language; use Smarty; use Tools; +use Validate; if (!defined('_PS_VERSION_')) { exit; @@ -325,6 +326,10 @@ public function getClosestParcelShops($pudoId) /** @var DPDShop $pudo */ $pudo = DPDShop::getShopByPudoId($pudoId); + if (!Validate::isLoadedObject($pudo)) { + return []; + } + $parcelShops = $this->parcelShopRepository->getClosestPudoShops( $pudo->longitude, $pudo->latitude, @@ -332,6 +337,10 @@ public function getClosestParcelShops($pudoId) Config::PARCEL_SHOP_MAP_POINTS_LIMIT ); + if (!is_array($parcelShops)) { + return []; + } + return $this->shopFactory->createShop($parcelShops); } diff --git a/views/js/front/pudo-search.js b/views/js/front/pudo-search.js index 431fc6c9..e1eb8b4f 100644 --- a/views/js/front/pudo-search.js +++ b/views/js/front/pudo-search.js @@ -81,8 +81,6 @@ $( document ).ajaxComplete(function( event, request, settings ) { }); function updateStreetSelect(city) { - var $container = $(this).closest('.dpd-pudo-container'); - $.ajax(dpdHookAjaxUrl, { type: 'POST', data: { @@ -109,10 +107,17 @@ function updateStreetSelect(city) { } }, error: function (response) { - var responseText = JSON.parse(response.responseText); + var $parent = $('.dpd-pudo-container'); + var responseText = null; - if (responseText) { - DPDdisplayMessage($container, responseText.template); + try { + responseText = JSON.parse(response.responseText); + } catch (e) { + responseText = null; + } + + if (responseText && responseText.template) { + DPDdisplayMessage($parent, responseText.template); } } }); @@ -146,10 +151,17 @@ function saveSelectedStreet(city, street) { } }, error: function (response) { - var responseText = JSON.parse(response.responseText); + var $parent = $('.dpd-pudo-container'); + var responseText = null; - if (responseText) { - DPDdisplayMessage($container, responseText.template); + try { + responseText = JSON.parse(response.responseText); + } catch (e) { + responseText = null; + } + + if (responseText && responseText.template) { + DPDdisplayMessage($parent, responseText.template); } } }); @@ -178,10 +190,17 @@ function updateParcelBlock(city, street, idCarrier) { } }, error: function (response) { - var responseText = JSON.parse(response.responseText); + var $parent = $('.dpd-pudo-container'); + var responseText = null; + + try { + responseText = JSON.parse(response.responseText); + } catch (e) { + responseText = null; + } - if (responseText) { - DPDdisplayMessage($container, responseText.template); + if (responseText && responseText.template) { + DPDdisplayMessage($parent, responseText.template); } } });