search

pronamic / wp-pay-core

Core components for the WordPress payment processing library. This library is used in the WordPress plugin Pronamic Pay: https://www.pronamicpay.com/, but also allows other plugin developers to set up a payment plugin.
https://www.wp-pay.org/
GNU General Public License v3.0
27 stars 3 forks source link

Strange status updates after next payment date changes #132

Closed remcotolsma closed 1 year ago

remcotolsma commented 1 year ago

After the changes in https://github.com/pronamic/wp-pay-core/pull/129 and https://github.com/pronamic/wp-pronamic-pay-woocommerce/pull/40 @rvdsteege sometimes noticed strange subscription status updates:

Scherm_afbeelding 2023-06-14 om 09 51 21

Scherm_afbeelding 2023-06-14 om 09 50 10

The last status update from SubscriptionStatus::ACTIVE to SubscriptionStatus::ON_HOLD is probably not intended.

rvdsteege commented 1 year ago

Got it, apparently it's only an issue with Safari and caused by an additional hit from my machine with the following user agent:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/601.2.4 (KHTML, like Gecko) Version/9.0.1 Safari/601.2.4 facebookexternalhit/1.1 Facebot Twitterbot/1.0

The facebookexternalhit/1.1 Facebot Twitterbot/1.0 seems to be used for iMessage link previews:

However, I'm not sharing any URLs in Messages so there must be something else also causing this request. I'm not yet sure what exactly causes the request (tried disabling link prefetching, but without luck) but will file a bug report with Apple.

remcotolsma commented 1 year ago

But apart from that, is such an extra hit supposed to cause such a status update?

rvdsteege commented 1 year ago

No, that's why I left the issue open ;-)

Seems it could be fixed with temporarily storing some "updating" flag in post meta, or we can use wp_cache_set() but that will only work with a persistent object cache. I don't like either solution.

remcotolsma commented 1 year ago

If I understand correctly, there are 2 processes running almost at the same time. In process 1 the payment status is still PaymentStatus::OPEN (subscription status = SubscriptionStatus::ON_HOLD) so that the status does not change, but we do call $subscription->save().

https://github.com/pronamic/wp-pay-core/blob/b55dff3f073c56f5780522a78f6412823b519e7a/src/Subscriptions/SubscriptionsModule.php#L135-L150

In the meantime, the payment status was successful in process 2. In the $subscription->save() routine of process 1, the current status is requested again from post meta, but it has already been updated by process 2 in the meantime. As a result, Pronamic Pay sees it as a status change, which is not actually the case is.

https://github.com/pronamic/wp-pay-core/blob/b55dff3f073c56f5780522a78f6412823b519e7a/src/Subscriptions/SubscriptionsDataStoreCPT.php#L727-L747

Can this be solved by only calling $subscription->save() if $status_before !== $status_update? Maybe that way we are a little safer in terms of concurrency?