From c3407e4e827790203f3db21133e6806e2a5b1188 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 12 Oct 2020 09:36:18 +1300
Subject: [PATCH] dev/financial#152 simplify interaction with membership,
 deprecate function

ids['membership'] is only calculated in the Authorize.net IPN flow for the purpose of determining
the autoRenew membership parameter for recurringNotify. This is currently passed to sendRecurringStartOrEndNotification
but that does not add any simplification & cannot be used elsewhere - so deprecating.

This is tested in testIPNPaymentMembershipRecurSuccessNoLeakage
---
 CRM/Contribute/BAO/ContributionRecur.php        |  1 +
 CRM/Core/Payment/AuthorizeNetIPN.php            | 17 ++++++++---------
 .../CRM/Core/Payment/AuthorizeNetIPNTest.php    |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php
index b7e12eb2c0..c44cd2f3bc 100644
--- a/CRM/Contribute/BAO/ContributionRecur.php
+++ b/CRM/Contribute/BAO/ContributionRecur.php
@@ -528,6 +528,7 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
    * @param bool $isFirstOrLastRecurringPayment
    */
   public static function sendRecurringStartOrEndNotification($ids, $recur, $isFirstOrLastRecurringPayment) {
+    CRM_Core_Error::deprecatedFunctionWarning('use CRM_Contribute_BAO_ContributionPage::recurringNotify');
     if ($isFirstOrLastRecurringPayment) {
       $autoRenewMembership = FALSE;
       if ($recur->id &&
diff --git a/CRM/Core/Payment/AuthorizeNetIPN.php b/CRM/Core/Payment/AuthorizeNetIPN.php
index d72dc82fa2..4aa8cdddbd 100644
--- a/CRM/Core/Payment/AuthorizeNetIPN.php
+++ b/CRM/Core/Payment/AuthorizeNetIPN.php
@@ -139,7 +139,6 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
             'related_contact' => $ids['related_contact'] ?? NULL,
             'participant' => !empty($objects['participant']) ? $objects['participant']->id : NULL,
             'contributionRecur' => !empty($objects['contributionRecur']) ? $objects['contributionRecur']->id : NULL,
-            'membership' => $ids['membership'] ?? NULL,
             'contact' => $ids['contact'] ?? NULL,
             'contributionPage' => $ids['contributionPage'] ?? NULL,
           ], $objects, $first);
@@ -165,6 +164,7 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
    */
   public function recur($input, $ids, $objects, $first) {
     $recur = &$objects['contributionRecur'];
+    $contribution = &$objects['contribution'];
 
     // do a subscription check
     if ($recur->processor_id != $input['subscription_id']) {
@@ -225,15 +225,15 @@ class CRM_Core_Payment_AuthorizeNetIPN extends CRM_Core_Payment_BaseIPN {
 
     CRM_Contribute_BAO_Contribution::completeOrder($input, $ids, $objects['contribution']);
 
-    // Only Authorize.net does this so it is on the a.net class. If there is a need for other processors
-    // to do this we should make it available via the api, e.g as a parameter, changing the nuance
-    // from isSentReceipt to an array of which receipts to send.
-    // Note that there is site-by-site opinions on which notifications are good to send.
     if ($isFirstOrLastRecurringPayment) {
-      CRM_Contribute_BAO_ContributionRecur::sendRecurringStartOrEndNotification($ids, $recur,
-        $isFirstOrLastRecurringPayment);
+      //send recurring Notification email for user
+      CRM_Contribute_BAO_ContributionPage::recurringNotify(TRUE,
+        $ids['contact'],
+        $ids['contributionPage'],
+        $recur,
+        (bool) $this->getMembershipID($contribution->id, $recur->id)
+      );
     }
-
   }
 
   /**
@@ -314,7 +314,6 @@ INNER JOIN civicrm_contribution co ON co.contribution_recur_id = cr.id
       $log->error('payment_notification', ['message' => $message, 'ids' => $ids, 'input' => $input]);
       throw new CRM_Core_Exception($message);
     }
-    $ids['membership'] = $this->getMembershipID($ids['contribution'], $ids['contributionRecur']);
   }
 
   /**
diff --git a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
index 12ee0cdc56..2b04f7a484 100644
--- a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
+++ b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
@@ -257,7 +257,7 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
   /**
    * Test IPN response mails don't leak.
    *
-   * @throws \CRM_Core_Exception
+   * @throws \CRM_Core_Exception|\CiviCRM_API3_Exception
    */
   public function testIPNPaymentMembershipRecurSuccessNoLeakage() {
     $mut = new CiviMailUtils($this, TRUE);
-- 
2.25.1