There is a problem with carrier configuration when there is more than one carrier of the same carrierType. This issue may be similar to the #144 issue.
Reproduction steps
For example, my client has a multistore PrestaShop installation, each shop has its own MyParcel Carrier added to the carriers.
In the configuration of the MyParcel module, we have added these carriers to Carrier Settings.
So the Carrier Settings look like this:
ID
Name
68
MyParcel
61
MyParcel
71
MyParcel
49
PostNL
(all of them have the carrierType set to postnl)
Each of them has different options enabled, for example, the carrier with 61 id has an evening delivery option enabled (The rest of this issue will reference this carrier) - and this setting is not available in the cart.
Relevant log output
No response
Additional context
When we are in the checkout page, the browser sends an AJAX request to this endpoint:
module/myparcelnl/checkout?carrier_id=61
carrier_id is valid here - and this is the last moment where everything is working correctly.
The controller receives carrier_id and here is the first big question, why is it not passed to the DeliveryOptionsConfigProvider constructor? It would be easier to find the correct config because we already have the valid id of the carrier from the request but okay, let's move on.
In the DeliveryOptionsConfigProvider class there is a get method which builds the carrier settings, it calls these methods: generateCarrierSettings -> getPsCarrier -> getPrestashopCarrier (getPrestaShopCarrierId) - and the last one is bad in my opinion:
wthijmen
commented
2 years ago
Module version
1.5.4
PrestaShop version
1.7.7.1
PHP version
7.4
What went wrong?
There is a problem with carrier configuration when there is more than one carrier of the same
carrierType. This issue may be similar to the #144 issue.Reproduction steps
For example, my client has a multistore PrestaShop installation, each shop has its own MyParcel Carrier added to the carriers.
In the configuration of the MyParcel module, we have added these carriers to
Carrier Settings.So the Carrier Settings look like this:
(all of them have the
carrierTypeset to postnl)Each of them has different options enabled, for example, the carrier with 61 id has an evening delivery option enabled (The rest of this issue will reference this carrier) - and this setting is not available in the cart.
Relevant log output
No response
Additional context
When we are in the checkout page, the browser sends an AJAX request to this endpoint:
module/myparcelnl/checkout?carrier_id=61carrier_idis valid here - and this is the last moment where everything is working correctly.The controller receives
carrier_idand here is the first big question, why is it not passed to theDeliveryOptionsConfigProviderconstructor? It would be easier to find the correct config because we already have the valid id of the carrier from the request but okay, let's move on.In the
DeliveryOptionsConfigProviderclass there is agetmethod which builds the carrier settings, it calls these methods:generateCarrierSettings->getPsCarrier->getPrestashopCarrier(getPrestaShopCarrierId) - and the last one is bad in my opinion:https://github.com/myparcelnl/prestashop/blob/6f9adafd56ff185c92f71d138d0fd41fbcf3251f/src/Service/CarrierService.php#L59-L77
This method creates an SQL query similar to the following:
And the result of this query has 18 rows in my case because we can add multiple carriers but the code takes only the first one: https://github.com/myparcelnl/prestashop/blob/6f9adafd56ff185c92f71d138d0fd41fbcf3251f/src/Service/CarrierService.php#L72
So in the result, we have a wrong carrier id which means that the rest of the code obtains the wrong configuration.
I think the problem is clearly visible now. Id of the carrier shouldn't be recognized by only the type of carrier because it's not precise enough.