Auto Renew Purchase bug?

Ok I fixed this myself after some php debugging.

In file Store\Model\ProductPurchase.php with method processExpiredProductPurchases()

Code:

if($isRemove || $purchase['recurring'] || !$this->autoRenewPurchased($purchase))
{
                $this->_removeProductChange($purchase);
                $purchaseRecordIds[] = $purchase['product_purchase_id'];
}

to

Code:

if($isRemove && !($purchase['recurring'] && !$this->autoRenewPurchased($purchase)))
{
               $this->_removeProductChange($purchase);
               $purchaseRecordIds[] = $purchase['product_purchase_id'];
}

The problem with the original code is that it will always remove that purchase since the OR statement will always true. Next it'll still remove this purchase because $purchase['recurring'] is true. Lastly $this->autoRenewPurchased($purchase) will update the end_date of the purchase's record, but it'll still get removed because of the previous variables, plus the product_purchase_id is the same since the purchase record is simply updated.

With the second code, the condition table output will be something like this

T && !(T && !(T)) = F, it's recurring and it auto renew purchase successfully, so don't delete
T && !(T && !(F)) = T, it's recurring but didn't auto renew purchase successfully (prob not enough money)
T && !(F && !(T)) = T, it's not recurring so it should delete purchase anyways, $this->autoRenewPurchased($purchase) will never return true anwyas
T && !(F && !(F)) = T, it's not recurring and $this->autoRenewPurchased($purchase) will always return false.

Now I searched through the entire Store php files and I haven't found any processExpiredProductPurchases($foo, false) so I believe my fix should be safe.

 

Actually I found more bugs. First with my code. It should be this.

In Store/Model/ProductPurchase.php, function processExpiredProductPurchases(array $purchases, $isRemove = true)

Code:

if($isRemove || !($purchase['recurring'] && $this->autoRenewPurchased($purchase)))
{
       $this->_removeProductChange($purchase);
       $purchaseRecordIds[] = $purchase['product_purchase_id'];
}

So whenever $isRemove is true, it'll always delete the subscriptions. Which is why I propose a change in the CronEntry

In Store/CronEntry/Store.php

Code:

public static function runProductPurchaseExpiredHandle()
{
        $productPurchaseModel = XenForo_Model::create('Brivium_Store_Model_ProductPurchase');
        $productPurchaseModel->processExpiredProductPurchases(
            //$productPurchaseModel->getExpiredProductPurchases(), true
               $productPurchaseModel->getExpiredProductPurchases(), false // don't delete subscription yet
        );
}

Since $isRemove is false, we now have to go through !($purchase['recurring'] && $this->autoRenewPurchased($purchase)). If !(T), that means the subscription successfully renewed, and deletion won't occur since $isRemove || !(T) are both False. If the second condition is true, remove the subscription since the renewal failed.

The final change I propose involves with all existing Store Products (or just permission related) addons.

In StoreProduct/AnyProducts/Model/ProductPurchase.php, function _processProductChange()

Code:

//return parent::_processProductChange($user, $product, $productTypeId);
return parent::_processProductChange($user, $product, $productTypeId, $existingPurchased);

Without passing $existingPurchase, the next execution of _processProductChange for other addons will start with NULL, which means the old changes are lost since it'll now take the user's existing permission and overwrite the xf_store_product_change existing settings.

Let me know what you think and if those changes work for your test. This definitely solved my renewal and permission problems.

 

Ok still think this bug isnt resolved in 1.2.3

In this statement

Code:

if($isRemove || ($purchase['recurring'] && !$this->autoRenewPurchased($purchase))){
                $this->_removeProductChange($purchase);
                $purchaseRecordIds[] = $purchase['product_purchase_id'];
            }

Since $isRemove is now set to "false" by default, this won't remove the expired product if the person who bought the product did not choose "Recurring Payments". Therefore $purchase['recurring'] is "false", which will cause:

($purchase['recurring'] && !$this->autoRenewPurchased($purchase))
.................F..................&&..............!(T or F) = F
to become false. I say my original fix in my previous post works

Code:

if($isRemove || !($purchase['recurring'] && $this->autoRenewPurchased($purchase))){
                $this->_removeProductChange($purchase);
                $purchaseRecordIds[] = $purchase['product_purchase_id'];
            }