Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #156
remcotolsma opened this issue Oct 4, 2021 · 2 comments
Closed
Tracked by #156

Comments

@remcotolsma
Copy link
Member

remcotolsma commented Oct 4, 2021

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

  • \Pronamic\WordPress\Pay\Core\Recurring::FIRST // Constant for the first payment.
  • \Pronamic\WordPress\Pay\Core\Recurring::RECURRING // Constant for recurring payments.
  • \Pronamic\WordPress\Pay\Core\Recurring::SUBSCRIPTION // Constant for subscription payments.

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 #188 (comment) I also suggested a $payment->is_interactive() function.

@remcotolsma
Copy link
Member Author

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 pronamic/wp-pronamic-pay-gravityforms@7366741.

$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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants