diff --git a/stripe/controllers/FrmStrpLiteEventsController.php b/stripe/controllers/FrmStrpLiteEventsController.php index a1d85cd8e9..4a205c0825 100644 --- a/stripe/controllers/FrmStrpLiteEventsController.php +++ b/stripe/controllers/FrmStrpLiteEventsController.php @@ -84,7 +84,9 @@ private function set_payment_status() { FrmTransLiteAppHelper::add_note_to_payment( $payment_values, $note ); - $u = $frm_payment->update( $payment->id, $payment_values ); + // Read the status again right before the update, in case another request has already changed it. + $payment_status_still_does_not_match = $this->payment_status_still_does_not_match( $payment->id ); + $updated = $frm_payment->update( $payment->id, $payment_values ); echo json_encode( array( @@ -93,7 +95,7 @@ private function set_payment_status() { ) ); - if ( ! $is_partial_refund ) { + if ( ! $is_partial_refund && $payment_status_still_does_not_match && $updated ) { $run_triggers = true; } }//end if @@ -108,6 +110,24 @@ private function set_payment_status() { } } + /** + * Double check that the payment status has not changed. + * This is to avoid running actions twice by mistake, since a Stripe Link + * return URL and a webhook event can both process the same payment. + * + * @since x.x + * + * @param int $payment_id The id of the payment to check. + * + * @return bool + */ + private function payment_status_still_does_not_match( $payment_id ) { + $frm_payment = new FrmTransLitePayment(); + $payment = $frm_payment->get_one( $payment_id ); + + return $payment && $payment->status !== $this->status; + } + /** * Skip updating the payment object for the first recurring payment. * This is to prevent double notifications because the first recurring payment creates an invoice and that invoice triggers the payment events. @@ -310,7 +330,9 @@ private function maybe_cancel_subscription( $sub ) { }; add_filter( $hook, $filter, 99 ); - $cancelled = FrmStrpLiteApiHelper::cancel_subscription( $sub->sub_id ); + + // There is no logged in user when a webhook event is processed, so the customer check has to be skipped here. + $cancelled = FrmStrpLiteAppHelper::call_stripe_helper_class( 'cancel_subscription_without_customer_check', $sub->sub_id ); if ( $cancelled ) { FrmTransLiteSubscriptionsController::change_subscription_status( @@ -319,6 +341,8 @@ private function maybe_cancel_subscription( $sub ) { 'sub' => $sub, ) ); + } else { + FrmTransLiteLog::log_message( 'Stripe Webhook Message', 'Unable to cancel subscription ' . $sub->sub_id . ' after it reached its payment limit.' ); } remove_filter( $hook, $filter, 99 ); diff --git a/stripe/controllers/FrmStrpLiteLinkController.php b/stripe/controllers/FrmStrpLiteLinkController.php index 38d4d22027..e2042b1fbd 100644 --- a/stripe/controllers/FrmStrpLiteLinkController.php +++ b/stripe/controllers/FrmStrpLiteLinkController.php @@ -146,13 +146,36 @@ private static function handle_one_time_stripe_link_return_url( $intent_id, $cli self::maybe_update_intent( $intent, $action, $entry ); - $frm_payment->update( $payment->id, $new_payment_values ); - FrmTransLiteActionsController::trigger_payment_status_change( compact( 'status', 'payment' ) ); + // A webhook event may have already updated this payment, so check the status again before running triggers. + $needs_triggers = $status !== $payment->status && self::payment_status_still_needs_to_update( $payment->id, $status ); + $updated = $frm_payment->update( $payment->id, $new_payment_values ); + + if ( $needs_triggers && $updated ) { + FrmTransLiteActionsController::trigger_payment_status_change( compact( 'status', 'payment' ) ); + } $redirect_helper->handle_success( $entry, isset( $charge ) ? $charge->id : '' ); die(); } + /** + * Check that the payment status has not been updated by another request already. + * This is to avoid running the payment actions twice. + * + * @since x.x + * + * @param int $payment_id The id of the payment to check. + * @param string $status The status the payment is about to be updated to. + * + * @return bool + */ + private static function payment_status_still_needs_to_update( $payment_id, $status ) { + $frm_payment = new FrmTransLitePayment(); + $payment = $frm_payment->get_one( $payment_id ); + + return $payment && $payment->status !== $status; + } + /** * Try to add the description to a Stripe link payment after it was confirmed. * diff --git a/stripe/helpers/FrmStrpLiteConnectApiAdapter.php b/stripe/helpers/FrmStrpLiteConnectApiAdapter.php index dc484e5b72..83c88fb974 100644 --- a/stripe/helpers/FrmStrpLiteConnectApiAdapter.php +++ b/stripe/helpers/FrmStrpLiteConnectApiAdapter.php @@ -37,6 +37,21 @@ public static function cancel_subscription( $sub_id ) { return FrmStrpLiteConnectHelper::cancel_subscription( $sub_id, $customer_id ); } + /** + * Cancel a subscription without checking that it belongs to the current user. + * Use this when there is no logged in user, like when a webhook event is processed. + * The customer check in self::cancel_subscription would always fail there because the current user ID is 0. + * + * @since x.x + * + * @param string $sub_id + * + * @return bool + */ + public static function cancel_subscription_without_customer_check( $sub_id ) { + return FrmStrpLiteConnectHelper::cancel_subscription( $sub_id ); + } + /** * @param string $payment_id * diff --git a/stripe/models/FrmStrpLiteAuth.php b/stripe/models/FrmStrpLiteAuth.php index b36ed09b04..45a8f01102 100644 --- a/stripe/models/FrmStrpLiteAuth.php +++ b/stripe/models/FrmStrpLiteAuth.php @@ -767,7 +767,7 @@ private static function get_redirect_url( $atts ) { $success_url = $atts['form']->options['success_url']; } - $success_url = trim( $atts['form']->options['success_url'] ); + $success_url = trim( $success_url ); $success_url = apply_filters( 'frm_content', $success_url, $atts['form'], $atts['entry'] ); $success_url = do_shortcode( $success_url ); $atts['id'] = $atts['entry']->id;