Fixed User upgrade reports 'Payment received, upgraded/extended' even if no upgrade was applied

[Xon]

Affected version

2.0.10

case CallbackState::PAYMENT_RECEIVED:
   $upgradeService->setPurchaseRequestKey($state->requestKey);
   $activeUpgrade = $upgradeService->upgrade();

   $state->logType = 'payment';
   $state->logMessage = 'Payment received, upgraded/extended.';
   break;

The result of $activeUpgrade is never checked w.r.t to generating the logMessage. Same applies for PAYMENT_REINSTATED

 

[Mike]

While probably worth checking, given what I've mentioned in your other bug report (we don't check canPurchase in this method now), I don't think there's necessarily a particular reason for the user upgrade insertion to fail at this point, short perhaps of a duplicate key error (though I don't think our code actually catches that anyway).

 

[Mike]

Looking at this code as it stands now, I don't think there's a situation where the normal flow would lead to returning anything but the active upgrade. Any error that occurs from the upgrade insert is set to throw an exception and these were the only situations that could return false (if throwing was disabled). Any sort of DB exception would throw an exception regardless.

So I think this should be resolved as is in 2.0.12.

 

[Xon]

if (!$active->user_upgrade_record_id)
{
   if (!$upgrade->canPurchase() && !$this->ignoreUnpurchasable)
   {
      return false;
   }
}

If paypal submits a new user upgrade that an admin has disabled between the user being redirected to paypal and paypal pushing the notification, then logging will say it has applied despite it not actually applying. (canPurchase/can_purchase doesn't really matter in this case)

 

[Mike]

That was the code I was referring to in my first message here (and my reply here). The code you're quoting is already gone from this stage of the flow -- if the payment has gone through, we now apply the purchase regardless.