Stripe: Immediately cancel if processing a charge.failed request

strangerstudios / pmpro-cancel-on-next-payment-date

Change membership cancellation in Paid Memberships Pro to set expiration for next payment date instead of cancelling immediately.

https://www.paidmembershipspro.com/add-ons/cancel-on-next-payment-date

8 stars 8 forks source link

Stripe: Immediately cancel if processing a charge.failed request #40

Closed mircobabini closed 1 year ago

mircobabini commented 2 years ago

All Submissions:

[x] Have you followed the Contributing guidelines?
[x] Does your code follow the WordPress' coding standards?
[x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

We do want Stripe to immediately cancel on a charge.failed request, which means that it's impossible to renew the membership.

Other information:

[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[x] Have you successfully run tests with your changes locally?

mircobabini commented 1 year ago

You would say "we don't have bug reports about this, why should we add this condition here?"

Consistency, I would say. We want to immediately cancel on charge failures or any similar behaviour.

This is what's going on when processing a charge.failure webhook from Stripe, without this hack: [07-Aug-2022 04:49:38 UTC] 0@Yu9EW7jZxz5zc_lxW2BC4gAAAEE: CONPD: Update the expiration date to 2022-08-07 05:48:25 by pmproconpd_pmpro_change_level(wp:plugins/pmpro-cancel-on-next-payment-date/pmpro-cancel-on-next-payment-date.php:173)

CONPD is setting the expire date in less than an hour. And sending the auto-renewal cancellation email. We can just force immediate cancellation, the same way we do for PayPal.

ideadude commented 1 year ago

I think we will merge this in, but also comment that we attempt to fix the issue in core PMPro here (https://github.com/strangerstudios/paid-memberships-pro/blob/dev/classes/gateways/class.pmprogateway_stripe.php#L1222-L1255) by returning the current_period_start value instead of current_period_end when the Stripe customer is "delinquent".

But it's okay to be redundant to make sure that we really cancel in case.

ideadude commented 1 year ago

Going to merge in, but then add a ! empty in there and move the global declaration to the top of the function. Thanks!