tig-nl / gls-magento2

Integrate GLS shipping services into Magento 2. Add GLS shipping and parcelshops to your checkout. Create your labels with Track and Trace functionality from the Magento backend.
5 stars 8 forks source link

[BUG] GLS Api error when name is too long #29

Open SolsWebdesign opened 2 years ago

SolsWebdesign commented 2 years ago

To Reproduce Steps to reproduce the behavior:

  1. Buy a product in Magento store, enter a very long name or two long names (first name and second name together should exceed length 30)
  2. Go through checkout and choose GLS
  3. In the backend of your shop, create a shipment and push the button "GLS - Create Label"

Expected result A label

Actual result Errors:

Workaround On the frontend of the shop I have added validators to the names to make sure no one can enter a name longer then 30 characters. Unfortunatly, it appears that there are several Parcel shops with names exceeding 30 characters that the customer can choose. And those simply can't be checked.

Screenshots If applicable, add screenshots to help explain your problem

image image

it seems to also happen with phone numbers exceeding 15 characters.

Please complete the following information

tig-jeffreybranderhorst commented 2 years ago

Hi @SolsWebdesign,

The GLS API does not support names longer than 30 characters, so this is the API that gives the error. We cannot change that in our extension I'm afraid.

Have a great day, Jeffrey

SolsWebdesign commented 2 years ago

Hi Jeffrey, Should an extension not prevent errors like these? Or at least refuse addresses like this? It is a bit strange that the extension does allow for orders to be made with names and parcel shops and then completely refuses to create labels for these orders because it (the module itself) does not respect the limits of de GLS API? Should it not say, sorry this is not possible in the checkout? It just does not seem very customer friendly to me.

tig-jeffreybranderhorst commented 2 years ago

Hi @SolsWebdesign,

We understand you, we decided to not give error at 30+ characters because we don't want to block the checkout or block an order from finishing. This way the order can be placed and with manual changes in backend still be sent. That is the reason we decided to not give any error. But thank you for thinking along and the question!

Have a great day, Jeffrey

vandijkstef commented 2 years ago

While I really appreciate you're not blocking an order, the blockage is simply moved to our warehouse. It will most definitly be a lot harder with names as this is very flexible input, though having it cut-off on the shipment label (or have this as an option) isn't likely to hurt.

For phone numbers, I do think some easy cleaning can be done. Our client formatted as (00) 00 00 00 00 (obfuscated for obvious reasons) which is longer then 15 characters including white-space. I think removing characters '(', ')' and ' ' wont hurt, and I've created the following patch for this:

From d070daf1c2011e801e1855bc40845e419d9862c3 Mon Sep 17 00:00:00 2001
From: Stef van Dijk <stef@vandijkstef.nl>
Date: Thu, 10 Mar 2022 12:27:48 +0100
Subject: [PATCH] :bug: Cleanup phone numbers for GLS

---
 web/app/code/TIG/GLS/Plugin/Quote/Model/QuoteManagement.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/web/app/code/TIG/GLS/Plugin/Quote/Model/QuoteManagement.php b/web/app/code/TIG/GLS/Plugin/Quote/Model/QuoteManagement.php
index de3d6559f55..d14676028fc 100644
--- a/web/app/code/TIG/GLS/Plugin/Quote/Model/QuoteManagement.php
+++ b/web/app/code/TIG/GLS/Plugin/Quote/Model/QuoteManagement.php
@@ -109,7 +109,7 @@ class QuoteManagement
             'city'          => $shipping->getCity(),
             // If Shipping Address is same as Billing Address, Email is only saved in Billing.
             'email'         => $shipping->getEmail() ?: $billing->getEmail(),
-            'phone'         => $shipping->getTelephone() ?: '+00000000000',
+            'phone'         => str_replace([' ', '(', ')'], '', $shipping->getTelephone()) ?: '+00000000000',
             'addresseeType' => $shipping->getCompany() ? 'b' : 'p'
         ];
     }
-- 
2.34.1
tig-jeffreybranderhorst commented 2 years ago

Hi @vandijkstef,

Thank you for your message and Pull request. I placed the issue on our backlog with your code added in the comments. We will check and test the code and after that we will together with GLS make a plan to release it.

Have a great day, Jeffrey