From 3d11a1e8ebb15a5a0cbc21dae25b0c854fd473c9 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 20 May 2021 15:50:37 +1200
Subject: [PATCH] [REF] Cleanup validation in PaypalProIPN single function

This fixes interaction with the contribution object to instead use the function
and switches the standardised error handling (throw an exception & let
it be caught).

The contribution object used to be passed to completeOrder but no longer
so there is no reason to set total_amount & invoice_id on it
(they will be used from input)
---
 CRM/Core/Payment/PayPalProIPN.php | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/CRM/Core/Payment/PayPalProIPN.php b/CRM/Core/Payment/PayPalProIPN.php
index 80892ef196..b21134f8dc 100644
--- a/CRM/Core/Payment/PayPalProIPN.php
+++ b/CRM/Core/Payment/PayPalProIPN.php
@@ -364,26 +364,16 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
 
     // make sure the invoice is valid and matches what we have in the contribution record
     if ((!$recur) || ($recur && $first)) {
-      if ($contribution->invoice_id != $input['invoice']) {
-        Civi::log()->debug('PayPalProIPN: Invoice values dont match between database and IPN request.');
-        echo "Failure: Invoice values dont match between database and IPN request<p>contribution is" . $contribution->invoice_id . " and input is " . $input['invoice'];
-        return;
+      if ($this->getContributionObject()->invoice_id !== $input['invoice']) {
+        throw new CRM_Core_Exception('PayPalProIPN: Invoice values dont match between database and IPN request.');
       }
     }
-    else {
-      $contribution->invoice_id = md5(uniqid(rand(), TRUE));
-    }
 
     if (!$recur) {
-      if ($contribution->total_amount != $input['amount']) {
-        Civi::log()->debug('PayPalProIPN: Amount values dont match between database and IPN request.');
-        echo "Failure: Amount values dont match between database and IPN request<p>";
-        return;
+      if ($this->getContributionObject()->total_amount != $input['amount']) {
+        throw new CRM_Core_Exception('PayPalProIPN: Amount values dont match between database and IPN request.');
       }
     }
-    else {
-      $contribution->total_amount = $input['amount'];
-    }
 
     $status = $input['paymentStatus'];
     if ($status === 'Denied' || $status === 'Failed' || $status === 'Voided') {
@@ -402,7 +392,7 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
       Contribution::update(FALSE)->setValues([
         'cancel_date' => 'now',
         'contribution_status_id:name' => 'Cancelled',
-      ])->addWhere('id', '=', $contribution->id)->execute();
+      ])->addWhere('id', '=', $this->getContributionID())->execute();
       Civi::log()->debug("Setting contribution status to Cancelled");
       return;
     }
@@ -419,7 +409,7 @@ class CRM_Core_Payment_PayPalProIPN extends CRM_Core_Payment_BaseIPN {
       return;
     }
 
-    CRM_Contribute_BAO_Contribution::completeOrder($input, $this->getContributionRecurID(), $contribution->id ?? NULL);
+    CRM_Contribute_BAO_Contribution::completeOrder($input, $this->getContributionRecurID(), $this->getContributionID());
   }
 
   /**
-- 
2.25.1