6366 authorized vs authorised and another detail
-
84 84 $current_payment_status = $payment->getStatus(); 85 85 $new_status = $status->getValue(); 86 86 87 /* WARNING : THIS PIECE OF CODE IS USING INAPPROPRIATE GetHumanStatus::STATUS_AUTHORIZED : 88 * 89 * This is (another) issue here with Payum. 90 * current_payment_status will never match STATUS_AUTHORIZED 91 * on the opposite, we have no chance to detect 'authorised' value here 92 * because Payzen vads_trans_status is written 'authorised' and not 'authorized' 93 * However this code seems to work and I don't want to break it as this is Payum dependent -
Master
Pas certain que ça fonctionnait car il y a beaucoup de payment_token existants en base de prod. Les lignes suivantes ont cet effet : si le status correspond, on supprime le token car le paiement a été accepté et on n'aura plus besoin de l'url de notification.
Etant donné que la notification d'un paiement récurrent se fait via une url particulière, et pas via l'url de notif avec le token, je pense qu'il n'y a pas de soucis à appliquer la même correction que ce que tu as fait plus haut.
-
-
-
84 84 $current_payment_status = $payment->getStatus(); 85 85 $new_status = $status->getValue(); 86 86 87 /* WARNING : THIS PIECE OF CODE IS USING INAPPROPRIATE GetHumanStatus::STATUS_AUTHORIZED : 88 * 89 * This is (another) issue here with Payum. 90 * current_payment_status will never match STATUS_AUTHORIZED 91 * on the opposite, we have no chance to detect 'authorised' value here -
Master
je n'ai pas compris cette phrase :
on the opposite, we have no chance to detect 'authorised' value here because Payzen vads_trans_status is written 'authorised' and not 'authorized'
peux-tu m'expliquer stp ?
-
Master
Je commence par fournir la traduction chatGPT pour éviter toute ambiguité due à l'anglais :
"Au contraire, nous n'avons aucune chance de détecter la valeur 'authorised' ici parce que le statut vads_trans de Payzen est écrit 'authorised' et non 'authorized'."
Cette erreur orthographique est la clé du problème. Payzen transmet le string "authorised" et nous on utilisait un if qui cherche à savoir si le string a la valeur "authorized", ce qui ne risque pas d'arriver.
-
Master
Je me rends compte que l'expression "au contraire" n'est pas très appropriée ici. Plus clair aurait été : "d'un côté on va jamais matcher STATUS_AUTHORIZED et de l'autre 'authorised' ne sera jamais detecté".
-
Master
ok je comprend mieux, c'est la tournure de la phrase que je ne comprenais pas
-
-
240 243 && array_key_exists('vads_identifier', $payment->getDetails()) 241 244 && $payment->getDetails()['vads_identifier'] == $vads_identifier 242 245 ) { 243 if ( 244 GetHumanStatus::STATUS_CAPTURED == $new_status 245 || GetHumanStatus::STATUS_AUTHORIZED == $new_status 246 ) { 246 // Do not use GetHumanStatus Payum constants as they do not match Payzen vads_trans_status 247 // e.g. GetHumanStatus::STATUS_AUTHORIZED does not match 'authorised' value sent by Payzen 248 if ('captured' == $new_status || 'authorised' == $new_status) { -
Master
j'ai mixé l'existant et ton fix par sécurité
-
-
-
Toggle commit list
-
243 243 && array_key_exists('vads_identifier', $payment->getDetails()) 244 244 && $payment->getDetails()['vads_identifier'] == $vads_identifier 245 245 ) { 246 // Do not use GetHumanStatus Payum constants as they do not match Payzen vads_trans_status 246 // Payum GetHumanStatus constants don't match Payzen vads_trans_status here 247 247 // e.g. GetHumanStatus::STATUS_AUTHORIZED does not match 'authorised' value sent by Payzen 248 if ('captured' == $new_status || 'authorised' == $new_status) { 248 if (in_array($new_status, ['captured', 'authorised', GetHumanStatus::STATUS_CAPTURED, GetHumanStatus::STATUS_AUTHORIZED])) { -
MasterEdited by Yvon Kerdoncuff
Je ne suis pas trop d'accord avec cette modification dans cette fonction car la fonction notifyRecurringPaymentAction a la particularité de fonctionner sans aucune dépendance à Payum ce qui est un avantage, de surcroit on voit les problèmes que l'on rencontre avec ce module. Elle montre que l'on peut tout à fait gérer les notifications Payzen simplement sans avoir à utiliser les mécanismes de Payum.
En plus d'utiliser une dépendance supplémentaire, ce code fait un truc bizarre : le tableau sur lequel in_array est appelé contient deux fois le même élément.
J'entends cependant l'intérêt de vérifier les deux orthographes, mais dans ce cas j'écrirais plus simplement :
in_array($new_status, ['captured', 'authorised', 'authorized'])avec un commentaire précisant que l'orthographe utilisée par Payzen est 'authorised' mais qu'on sait jamais si Payzen change un jour son fusil d'épaule...
-
Master
Finalement, je comprends l'intérêt potentiellement supérieur d'utiliser le même bout de code aux trois endroits dans le code.
-
Master
ok pour moi de faire la modification que tu proposes
-
-
289 289 * current_payment_status will never match STATUS_AUTHORIZED 290 290 * on the opposite, we have no chance to detect 'authorised' value here 291 291 * because Payzen vads_trans_status is written 'authorised' and not 'authorized' 292 * However this code seems to work and I don't want to break it as this is Payum dependent 293 * and uses mechanisms I don't understand. 294 292 * 295 293 * @see comment above notifyRecurringPaymentAction for more Payum weird stuff 296 294 */ 297 if (GetHumanStatus::STATUS_CAPTURED == $payment->getStatus() || GetHumanStatus::STATUS_AUTHORIZED == $payment->getStatus()) { -
90 90 * current_payment_status will never match STATUS_AUTHORIZED 91 91 * on the opposite, we have no chance to detect 'authorised' value here 92 92 * because Payzen vads_trans_status is written 'authorised' and not 'authorized' 93 * However this code seems to work and I don't want to break it as this is Payum dependent 94 * and uses mechanisms I don't understand. 95 93 * 96 94 * @see comment above notifyRecurringPaymentAction for more Payum weird stuff 97 95 */ 98 if ( -
merged
Toggle commit list
