Anomalie #1538
fermé
Manque d'initialisation sur $errors pour Contributions fait planter ->store() après appel du constructeur
100%
Description
Bonjour,
Je remonte ce qui est, je pense, une anomalie qui touche le plugin Paypal, mais qui je pense a sa source dans le Core.
Lorsque le plugin Paypal traite le /notify, il réalise les actions suivantes :
$contrib = new Contribution($this->zdb, $this->login, $args);
...
$store = $contrib->store();
Or, la fonction ->store() de Contributions commence par effectuer le test suivant :
if (count($this->errors) > 0) {
Et cela fait planter le programme (du moins sous PHP 7.2) :
stderr: Message: count(): Parameter must be an array or an object that implements Countable
Car à aucun moment ni dans l'initialisation de l'attribut ni dans le constructeur la valeur $this->errors est initialisée avec un tableau vide.
La propriété étant privée, il n'est pas possible de l'initialiser autrement qu'en passant par la méthode ->check(...) mais celle-ci s'attend à avoir un objet de formulaire, qu'il faut construire.
De mon côté je pense qu'il suffirait d'initialiser l'attribut $errors avec un tableau vide dans la déclaration de la classe pour régler le problème.
Mis à jour par Johan Cwiklinski il y a presque 4 ans
- Statut changé de Nouveau à In Progress
Il est possible de corriger le problème du côté du plugin aussi. Dans le coeur, sur les objets principaux (adhérents, dons/cotisations, ...) il y a une méthode check qui devrait normalement être exécutée dans tous les cas, même si ce n'est actuellement pas obligatoire dans les faits.
Je pense que le correctif suivant devrait corriger le problème dans le plugin (c'est du vite fait, je n'ai pas testé) :
diff --git a/lib/GalettePaypal/Controllers/PaypalController.php b/lib/GalettePaypal/Controllers/PaypalController.php
index 5b9662e..2be90a8 100644
--- a/lib/GalettePaypal/Controllers/PaypalController.php
+++ b/lib/GalettePaypal/Controllers/PaypalController.php
@@ -448,6 +448,18 @@ class PaypalController extends AbstractPluginController
}
if ($real_contrib) {
+ $store = false;
+ $valid = $contrib->check($post, [], []);
+ if ($valid !== true) {
+ Analog::log(
+ 'An error occurred while storing a new contribution from Paypal payment:' .
+ implode("\n ", $valid),
+ Analog::ERROR
+ );
+ $ph->setState(PaypalHistory::STATE_ERROR);
+ return $response->withStatus(500, 'Internal error');
+ }
+
$store = $contrib->store();
if ($store === true) {
//contribution has been stored :)
Mis à jour par Mathieu Pellegrin il y a presque 4 ans
Merci pour ta réponse!
J'ai dût être un peu plus précis pour que ça passe, $post est issu du $request et ne contenait pas les bonnes clés :
index 5b9662e..a7ced54 100644
--- a/lib/GalettePaypal/Controllers/PaypalController.php
+++ b/lib/GalettePaypal/Controllers/PaypalController.php
@@ -39,6 +39,7 @@ namespace GalettePaypal\Controllers;
use Analog\Analog;
use Galette\Controllers\AbstractPluginController;
use Galette\Entity\Contribution;
+use Galette\Entity\ContributionTypes;
use Galette\Entity\PaymentType;
use Galette\Filters\HistoryList;
use GalettePaypal\Paypal;
@@ -448,6 +449,24 @@ class PaypalController extends AbstractPluginController
}
if ($real_contrib) {
+
+ $fields = [
+ ContributionsTypes::PK => $post['item_number'],
+ Adherent::PK => $post['custom'],
+ 'type_paiement_cotis' => PaymentType::PAYPAL,
+ 'montant_cotis' => $post['mc_gross'],
+ ];
+ $valid = $contrib->check($fields, [], []);
+ if ($valid !== true) {
+ Analog::log(
+ 'An error occurred while storing a new contribution from Paypal payment:' .
+ implode("\n ", $valid),
+ Analog::ERROR
+ );
+ $ph->setState(PaypalHistory::STATE_ERROR);
+ return $response->withStatus(500, 'Internal error');
+ }
+
$store = $contrib->store();
if ($store === true) {
//contribution has been stored :)
Du coup je ne suis pas sûr que le tableau $args serve à quelque chose, car check() initialise pas mal des paramètres de la contribution, mais je l'ai laissé par sécurité.
À noter que je n'ai pas essayé non plus, j'ai backporté les modifications que j'ai faites pour mon plugin Stripe à partir de ta réponse initiale.
Mis à jour par Johan Cwiklinski il y a presque 4 ans
Mathieu Pellegrin a écrit (#note-2):
Du coup je ne suis pas sûr que le tableau $args serve à quelque chose, car check() initialise pas mal des paramètres de la contribution, mais je l'ai laissé par sécurité.
Oui bien vu, ça me semble tout à fait pertinent ;)
À noter que je n'ai pas essayé non plus, j'ai backporté les modifications que j'ai faites pour mon plugin Stripe à partir de ta réponse initiale.
^^ pas de problème ;)
Du coup, j'ai regardé d'un peu plus près, si on utilise check, il n'est plus besoin de vérifier manuellement les chevauchements ; du coup, le patch devient :
diff --git a/lib/GalettePaypal/Controllers/PaypalController.php b/lib/GalettePaypal/Controllers/PaypalController.php
index 5b9662e..892921f 100644
--- a/lib/GalettePaypal/Controllers/PaypalController.php
+++ b/lib/GalettePaypal/Controllers/PaypalController.php
@@ -425,29 +425,27 @@ class PaypalController extends AbstractPluginController
$args['ext'] = $this->preferences->pref_membership_ext;
}
$contrib = new Contribution($this->zdb, $this->login, $args);
- $contrib->amount = $post['mc_gross'];
+ $post = [
+ ContributionsTypes::PK => $post['item_number'],
+ Adherent::PK => $post['custom'],
+ 'type_paiement_cotis' => PaymentType::PAYPAL,
+ 'montant_cotis' => $post['mc_gross']
+ ];
//all goes well, we can proceed
- if ($real_contrib && $contrib->isCotis()) {
- // Check that membership fees does not overlap
- $overlap = $contrib->checkOverlap();
- if ($overlap !== true) {
- if ($overlap === false) {
- Analog::log(
- 'An error occurred checking overlapping fees :(',
- Analog::ERROR
- );
- } else {
- //method directly return error message
- Analog::log(
- 'Error while calculating overlapping fees from paypal payment: ' . $overlap,
- Analog::ERROR
- );
- }
+ if ($real_contrib) {
+ $store = false;
+ $valid = $contrib->check($post, [], []);
+ if ($valid !== true) {
+ Analog::log(
+ 'An error occurred while storing a new contribution from Paypal payment:' .
+ implode("\n ", $valid),
+ Analog::ERROR
+ );
+ $ph->setState(PaypalHistory::STATE_ERROR);
+ return $response->withStatus(500, 'Internal error');
}
- }
- if ($real_contrib) {
$store = $contrib->store();
if ($store === true) {
//contribution has been stored :)
J'ai ouvert une PR pour ce correctif : https://github.com/galette/plugin-paypal/pull/3
Mis à jour par Johan Cwiklinski il y a plus de 3 ans
- Statut changé de In Progress à Résolu
- % réalisé changé de 0 à 100
Appliqué par commit galette-plugin-paypa:0659fbf73b72c1a5f7eace5c98335d5277d90cc6.
Mis à jour par Johan Cwiklinski il y a plus de 3 ans
- Statut changé de Résolu à Fermé
Corrigé dans le plugin Paypal