Jump to content

Cart::duplicate() : bug quand présence produits offerts

PtitKev

By PtitKev
in Rapports de bugs

 Share

Recommended Posts

PtitKev Newbie

PtitKev

Posted
Posted

Bonjour à tous,

Fraichement arrivé sur Prestashop dans un contexte professionnel je vous fait part de mon premier bug core.

Constaté sur la version 1.6.1.17, toujours présent en 1.6.1.23.

Sur la page "historique des commandes" je recommande un panier contenant un produit offert.

Ce produit est systématiquement ajouté au nouveau panier.

L'erreur se situe dans la classe Cart, méthode duplicate(). La requête SQL remplissant la variable $product_gift est fausse. 

SELECT cr.`gift_product`, cr.`gift_product_attribute`
FROM `'._DB_PREFIX_.'cart_rule` cr
LEFT JOIN `'._DB_PREFIX_.'order_cart_rule` ocr ON (ocr.`id_order` = '.(int)$this->id.')
WHERE ocr.`id_cart_rule` = cr.`id_cart_rule`

On peut noter que le LEFT JOIN et le WHERE sont incompatible mais l'erreur vient surtout de la condition de la jointure : "id_order" = "id_cart".

Sachant que dans un objet Cart on je fait jamais référence à une commande, la seule façon de trouver un "id_order" pour la table "order_cart_rule" est de passer par la table "orders".

J'ai donc reconstruit la requête de cette façon : 

SELECT cr.`gift_product`, cr.`gift_product_attribute`
FROM `'._DB_PREFIX_.'orders` o
INNER JOIN `'._DB_PREFIX_.'order_cart_rule` ocr ON o.`id_order` = ocr.`id_order`
INNER JOIN `'._DB_PREFIX_.'cart_rule` cr ON ocr.`id_cart_rule` = cr.`id_cart_rule`
WHERE o.`id_cart` = '.(int)$this->id

J'ai constaté que le champ orders.id_cart n'avait pas d'index unique donc cette solution n'est pas, dans l'absolue, la bonne mais vu qu'on valide l’existence d'une commande pour un panier donné je considère qu'elle fera l'affaire :)

Comme disent les anglais : "et voilà !"

Link to comment
Share on other sites
Eolia Grand Master

Eolia

Posted
Posted

Merci du partage, mais personnellement j'ai viré cette requête car c'est une aberration.

Un produit offert n'est jamais dispo seul, il y a forcément une condition et si cart_rule il y a, ce produit sera alors ajouté.

La règle qui offre le produit peut être unique ou terminée ou que sais-je...

Ce n'est pas à Prestashop de décider à notre place quand un produit est offert ou pas^^ 

Link to comment
Share on other sites
doekia Community Regular

doekia

Posted
Posted

Merci pour le partage. Pour ton premier post bravo.

Concernant l'index qu'il soit unique ou non ne fera pas de différence significative en vitesse et comme un commande peut dans certains cas être splitté en 2 (multi-shipping) il ne peut pas être unique.

Link to comment
Share on other sites
PtitKev Newbie

PtitKev

Posted
  • Author
Posted

Merci à vous pour vos retours :)

19 hours ago, Eolia said:

Merci du partage, mais personnellement j'ai viré cette requête car c'est une aberration.

Un produit offert n'est jamais dispo seul, il y a forcément une condition et si cart_rule il y a, ce produit sera alors ajouté.

La règle qui offre le produit peut être unique ou terminée ou que sais-je...

Ce n'est pas à Prestashop de décider à notre place quand un produit est offert ou pas^^ 

Le produit est offert via une règle panier.

Cette requête permet de détecter quels produits dans le panier commandé sont des produits offerts.

Vu qu'elle bug aucun produit n'est considéré comme offert ce qui a pour résultat d'ajouter le produit offert dans le panier de re-commande :)

Du coup dans le nouveau panier j'ai le produit commandé initialement + le produit offert à 0€ + une nouvelle fois le produit offert ajouté par la règle de panier.

En aucun cas le fait que le produit ne soit pas commandable bloque l'ajout au panier (peut être une autre façon de voir le problème).

19 hours ago, doekia said:

Merci pour le partage. Pour ton premier post bravo.

Concernant l'index qu'il soit unique ou non ne fera pas de différence significative en vitesse et comme un commande peut dans certains cas être splitté en 2 (multi-shipping) il ne peut pas être unique.

Je ne parlais pas d'index pour la vitesse mais pour l'unicité du champ : la seule façon de sécuriser la donnée sont les contraintes dont l'index unique fait parti :)

Je n'ai jamais vu de cas de multi-shipping. Je pensais qu'une panier pouvait passer dans plusieurs commandes (pas forcement en multi-shipping).

Donc oui, au final si un panier a plusieurs commandes ça ne change rien au fonctionnement de la requête.

 

Sinon, pour la contribution sur le core ça se passe ici ou plus sur leur Git ?

Merci et bonne journée.

Link to comment
Share on other sites
Eolia Grand Master

Eolia

Posted
Posted
Il y a 2 heures, PtitKev a dit :

Vu qu'elle bug aucun produit n'est considéré comme offert ce qui a pour résultat d'ajouter le produit offert dans le panier de re-commande :)

Du coup dans le nouveau panier j'ai le produit commandé initialement + le produit offert à 0€ + une nouvelle fois le produit offert ajouté par la règle de panier.

Ben il faut modifier plus bas aussi 

            foreach ($product_gift as $gift) {
                if (isset($gift['gift_product']) && isset($gift['gift_product_attribute']) && (int)$gift['gift_product'] == (int)$product['id_product'] && (int)$gift['gift_product_attribute'] == (int)$product['id_product_attribute']) {
                    $product['quantity'] = (int)$product['quantity'] - 1;
                }
            }

Inutile de lancer la suite si le produit est de type gift 

            foreach ($product_gift as $gift) {
                if (isset($gift['gift_product']) && isset($gift['gift_product_attribute']) && (int)$gift['gift_product'] == (int)$product['id_product'] && (int)$gift['gift_product_attribute'] == (int)$product['id_product_attribute']) {
                    continue 2;
                }
            }

 

Link to comment
Share on other sites
Janett Collaborator

Janett

Posted
Posted (edited)
22 hours ago, PtitKev said:

L'erreur se situe dans la classe Cart, méthode duplicate(). La requête SQL remplissant la variable $product_gift est fausse. 


SELECT cr.`gift_product`, cr.`gift_product_attribute`
FROM `'._DB_PREFIX_.'cart_rule` cr
LEFT JOIN `'._DB_PREFIX_.'order_cart_rule` ocr ON (ocr.`id_order` = '.(int)$this->id.')
WHERE ocr.`id_cart_rule` = cr.`id_cart_rule`

On peut noter que le LEFT JOIN et le WHERE sont incompatible mais l'erreur vient surtout de la condition de la jointure : "id_order" = "id_cart".

Sachant que dans un objet Cart on je fait jamais référence à une commande, la seule façon de trouver un "id_order" pour la table "order_cart_rule" est de passer par la table "orders".

J'ai donc reconstruit la requête de cette façon : 


SELECT cr.`gift_product`, cr.`gift_product_attribute`
FROM `'._DB_PREFIX_.'orders` o
INNER JOIN `'._DB_PREFIX_.'order_cart_rule` ocr ON o.`id_order` = ocr.`id_order`
INNER JOIN `'._DB_PREFIX_.'cart_rule` cr ON ocr.`id_cart_rule` = cr.`id_cart_rule`
WHERE o.`id_cart` = '.(int)$this->id

J'ai constaté que le champ orders.id_cart n'avait pas d'index unique donc cette solution n'est pas, dans l'absolue, la bonne mais vu qu'on valide l’existence d'une commande pour un panier donné je considère qu'elle fera l'affaire :)

Comme disent les anglais : "et voilà !"

Dans la 1.7 cela a apparemment été corrigé : https://github.com/PrestaShop/PrestaShop/pull/8498

Edited by Janett (see edit history)
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share
Go to topic listing
×
×
  • Create New...