backdrop-contrib / uc_recurring

GNU General Public License v2.0
0 stars 1 forks source link

Recuring fees for deleted orders need to be cleaned up #6

Open alanmels opened 2 years ago

alanmels commented 2 years ago

After deleting all orders from my testing system, I've found entries for recurring fees on /admin/store/orders/recurring were still there. I guess they also should be automatically cleared if the corresponding initial orders were deleted from the system. As I understand these recurring fees are useless after their initial orders get deleted, aren't they?

alanmels commented 2 years ago

Yep, my guess was right: as soon as cron is triggered after the initial orders were deleted it gives:

    Notice: Trying to get property 'order_id' of non-object in uc_recurring_create_renewal_order() (line 472 of /var/www/html/docroot/modules/uc_recurring/uc_recurring.module).
    Notice: Trying to get property 'uid' of non-object in uc_recurring_create_renewal_order() (line 475 of /var/www/html/docroot/modules/uc_recurring/uc_recurring.module).

I've tried to introduce the check condition like so:

 // Return early if for any reason $order is empty.
  if (empty($order)) {
    return;
  }

However, it returned bunch of new errors:

  Notice: Trying to get property 'order_id' of non-object in uc_recurring_test_gateway_renew() (line 85 of /var/www/html/docroot/modules/uc_recurring/includes/uc_recurring.test_gateway.inc).
    Notice: Trying to get property 'order_total' of non-object in uc_recurring_test_gateway_renew() (line 85 of /var/www/html/docroot/modules/uc_recurring/includes/uc_recurring.test_gateway.inc).
    Notice: Trying to get property 'payment_details' of non-object in test_gateway_charge() (line 53 of /var/www/html/docroot/modules/ubercart/payment/uc_credit/tests/test_gateway.module).
    Notice: Trying to access array offset on value of type null in test_gateway_charge() (line 53 of /var/www/html/docroot/modules/ubercart/payment/uc_credit/tests/test_gateway.module).
    Notice: Trying to get property 'payment_details' of non-object in test_gateway_charge() (line 54 of /var/www/html/docroot/modules/ubercart/payment/uc_credit/tests/test_gateway.module).
    Notice: Trying to access array offset on value of type null in test_gateway_charge() (line 54 of /var/www/html/docroot/modules/ubercart/payment/uc_credit/tests/test_gateway.module).
    Notice: Trying to get property 'payment_details' of non-object in test_gateway_charge() (line 64 of /var/www/html/docroot/modules/ubercart/payment/uc_credit/tests/test_gateway.module).
    Notice: Trying to access array offset on value of type null in test_gateway_charge() (line 64 of /var/www/html/docroot/modules/ubercart/payment/uc_credit/tests/test_gateway.module).

So the best way to automatically clear recurring orders if their base orders are deleted.

alanmels commented 2 years ago

Interestingly, there is no way to get rid of such orphaned recurring fees via UI on /admin/store/orders/recurring. You can only cancel them. I believe that's another problem, because during development admins could create ton's of test recurring orders and the only way to clean that page is to go to database.

alanmels commented 2 years ago

On the other hand, I've just found the following code in the .module file:

/**
 * Implements hook_uc_order().
 */
function uc_recurring_uc_order($op, $order, $arg2) {
  switch ($op) {
    case 'delete':
      $fees = uc_recurring_get_fees($order);
      foreach ($fees as $fee) {
        uc_recurring_fee_cancel($fee->rfid, $fee);
        uc_recurring_fee_user_delete($fee->rfid);
      }
      break;
  }
}

but I don't know why it didn't work. Need more testing.

alanmels commented 2 years ago

Regardless of the above hook implementation, the proposed patch is good for keeping the recurring fees table clean in case somehow fees get orphaned as it happened in my test case. However, the first PR was not good, the second one is:

https://github.com/backdrop-contrib/uc_recurring/pull/7/files#diff-30b8be38b407790c6f4287ee49a290bf1b5a277d0a61e4140360d81d81d161f2