pronamic / wp-pronamic-pay

The Pronamic Pay plugin allows you to easily accept payments with payment methods like credit card, iDEAL, Bancontact and Sofort through a variety of payment providers on your WordPress website.
https://pronamicpay.com
35 stars 14 forks source link

Delete legacy payment subscription property `$payment->recurring_type` #227

Closed remcotolsma closed 3 years ago

remcotolsma commented 3 years ago

Can we make the $payment->recurring_type property more self explaining? We use it now for the following 3 values:

https://github.com/pronamic/wp-pay-core/blob/aff3656759b15994d278232bef2e378c1bca4b75/src/Core/Recurring.php#L24-L43

In the Gravity Forms extension we use these to determine which actions should be triggered: https://github.com/wp-pay-extensions/gravityforms/blob/0d0200864a24424949172346d2d38aa001802e62/src/Extension.php#L613-L653

Should we only trigger the Gravity Forms create_subscription payment action after the first subscript payment is successfully paid?

Could we implement a $subscription->get_first_payment() to easily retrieve the first subscription payment and to check if the payment in question is the first payment?

$subscription = $payment->get_subscription();

$payment_first = $subscription->get_first_payment();

if ( $payment === $payment_first ) {
    // FIRST
}

@rvdsteege I think you know best how we use recurring_type, what are your thoughts on this?

Another point to consider may also be that a RECURRING payment does not require interaction from the customer. In https://github.com/pronamic/wp-pronamic-pay/issues/188#issuecomment-907155800 I also suggested a $payment->is_interactive() function.

remcotolsma commented 3 years ago

In the Gravity Forms extension we use these to determine which actions should be triggered: https://github.com/wp-pay-extensions/gravityforms/blob/0d0200864a24424949172346d2d38aa001802e62/src/Extension.php#L613-L653

Simplified this in https://github.com/wp-pay-extensions/gravityforms/commit/7366741c94f52ce65b1a96cf6696e02062c3267e.

$subscription = $payment->get_subscription();

$payment_first = $subscription->get_first_payment();

if ( $payment === $payment_first ) {
    // FIRST
}

We have to keep in mind that 1 payment can theoretically be linked to multiple subscriptions.

remcotolsma commented 3 years ago

Much discussed at the office, we were able to get rid of the property.