From 6f87a98f28580d44494d429c150d1069a282ce70 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 17 Jul 2024 20:18:22 +0800 Subject: [PATCH 1/4] WIP --- .../shop/plans/update-billing-details.twig | 3 +- .../shop/plans/update-billing-details.twig | 3 +- src/Plugin.php | 2 +- src/controllers/SubscriptionsController.php | 83 ++++++++++++++----- src/elements/Subscription.php | 6 ++ src/elements/db/SubscriptionQuery.php | 1 + src/migrations/Install.php | 1 + ..._044256_add_return_url_to_subscription.php | 32 +++++++ src/records/Subscription.php | 1 + 9 files changed, 110 insertions(+), 22 deletions(-) create mode 100644 src/migrations/m240717_044256_add_return_url_to_subscription.php diff --git a/example-templates/dist/shop/plans/update-billing-details.twig b/example-templates/dist/shop/plans/update-billing-details.twig index a641e7ec76..4591167ad6 100644 --- a/example-templates/dist/shop/plans/update-billing-details.twig +++ b/example-templates/dist/shop/plans/update-billing-details.twig @@ -4,7 +4,7 @@ {% set subscriptionUid = craft.app.request.getParam('subscription') %} {# @var subscription \craft\commerce\elements\Subscription #} {% set subscription = craft.subscriptions() - .anyStatus() + .status(null) .uid(subscriptionUid) .one() %} @@ -41,6 +41,7 @@
+ {{ redirectInput('shop/plans') }} {{ subscription.getBillingIssueResolveFormHtml()|raw }}
diff --git a/example-templates/src/shop/plans/update-billing-details.twig b/example-templates/src/shop/plans/update-billing-details.twig index 0536e46278..9ca5b5b6cb 100755 --- a/example-templates/src/shop/plans/update-billing-details.twig +++ b/example-templates/src/shop/plans/update-billing-details.twig @@ -4,7 +4,7 @@ {% set subscriptionUid = craft.app.request.getParam('subscription') %} {# @var subscription \craft\commerce\elements\Subscription #} {% set subscription = craft.subscriptions() - .anyStatus() + .status(null) .uid(subscriptionUid) .one() %} @@ -41,6 +41,7 @@
+ {{ redirectInput('[[folderName]]/plans') }} {{ subscription.getBillingIssueResolveFormHtml()|raw }}
diff --git a/src/Plugin.php b/src/Plugin.php index f07cf0c015..76d4430831 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -207,7 +207,7 @@ public static function editions(): array /** * @inheritDoc */ - public string $schemaVersion = '4.5.2'; + public string $schemaVersion = '4.5.3'; /** * @inheritdoc diff --git a/src/controllers/SubscriptionsController.php b/src/controllers/SubscriptionsController.php index bbacdc1bd9..bdbd3cd506 100644 --- a/src/controllers/SubscriptionsController.php +++ b/src/controllers/SubscriptionsController.php @@ -10,11 +10,13 @@ use Craft; use craft\base\Element; use craft\commerce\base\SubscriptionGateway; +use craft\commerce\db\Table; use craft\commerce\elements\Subscription; use craft\commerce\errors\SubscriptionException; use craft\commerce\helpers\PaymentForm; use craft\commerce\Plugin; use craft\commerce\Plugin as Commerce; +use craft\commerce\records\Subscription as SubscriptionRecord; use craft\commerce\stripe\gateways\PaymentIntents; use craft\commerce\web\assets\commercecp\CommerceCpAsset; use craft\helpers\App; @@ -164,6 +166,9 @@ public function actionSubscribe(): ?Response { $this->requireLogin(); $this->requirePostRequest(); + $user = Craft::$app->getUser()->getIdentity(); + + $returnUrl = $this->request->getValidatedBodyParam('redirect'); $plugin = Commerce::getInstance(); @@ -194,34 +199,35 @@ public function actionSubscribe(): ?Response } try { - // We provide backward compatibility for stripe plugin 3.0 $paymentFormData = $this->request->getBodyParam(PaymentForm::getPaymentFormParamName($gateway->handle)) ?? []; - $createPaymentSource = function($gateway, $paymentFormData) use ($plugin) { - $paymentForm = $gateway->getPaymentFormModel(); - $paymentForm->setAttributes($paymentFormData, false); + if (!empty($paymentFormData)) { + Craft::$app->getDeprecator()->log('SubscriptionController::create-newPaymentMethod', 'The subscription create action now requires that a customer’s default payment source is set up before subscribing, or pass the payment source information to the subscribe form.'); - if ($paymentForm->validate()) { - $plugin->getPaymentSources()->createPaymentSource(Craft::$app->getUser()->getId(), $gateway, $paymentForm); - } - }; - - $exists = class_exists(PaymentIntents::class); - /** @phpstan-ignore-next-line */ - if ($exists && $plan->getGateway() instanceof PaymentIntents) { - Craft::$app->getDeprecator()->log('SubscriptionController::create-newPaymentMethod', 'The subscription create action now requires that a customer’s default payment source is set up before subscribing with Stripe.'); - // Only be backward compatible with Stripe if they supply the payment method ID. - if (isset($paymentFormData['paymentMethodId'])) { + $createPaymentSource = function($gateway, $paymentFormData) use ($plugin) { + $paymentForm = $gateway->getPaymentFormModel(); + $paymentForm->setAttributes($paymentFormData, false); + + if ($paymentForm->validate()) { + $plugin->getPaymentSources()->createPaymentSource(Craft::$app->getUser()->getId(), $gateway, $paymentForm); + } + }; + + $exists = class_exists(PaymentIntents::class); + /** @phpstan-ignore-next-line */ + if ($exists && $plan->getGateway() instanceof PaymentIntents) { + if (isset($paymentFormData['paymentMethodId'])) { + $createPaymentSource($gateway, $paymentFormData); + } + } else { $createPaymentSource($gateway, $paymentFormData); } - } else { - $createPaymentSource($gateway, $paymentFormData); } $fieldsLocation = $this->request->getParam('fieldsLocation', 'fields'); $fieldValues = $this->request->getBodyParam($fieldsLocation, []); - $subscription = $plugin->getSubscriptions()->createSubscription(Craft::$app->getUser()->getIdentity(), $plan, $parameters, $fieldValues); + $subscription = $plugin->getSubscriptions()->createSubscription($user, $plan, $parameters, $fieldValues); } catch (\Exception $exception) { Craft::$app->getErrorHandler()->logException($exception); @@ -231,6 +237,14 @@ public function actionSubscribe(): ?Response $error = $exception->getMessage(); } + if ($returnUrl) { + $returnUrl = $this->getView()->renderObjectTemplate($returnUrl, $subscription); + $subscriptionRecord = SubscriptionRecord::findOne($subscription->id); + $subscriptionRecord->returnUrl = $returnUrl; + $subscriptionRecord->save(); + $subscription->returnUrl = $returnUrl; + } + if (!$error && $subscription && $subscription->isSuspended && !$subscription->hasStarted) { $url = Plugin::getInstance()->getSettings()->updateBillingDetailsUrl; @@ -249,7 +263,8 @@ public function actionSubscribe(): ?Response Craft::t('commerce', 'Subscription started.'), data: [ 'subscription' => $subscription ?? null, - ] + ], + redirect: $returnUrl ); } @@ -424,6 +439,35 @@ public function actionCancel(): ?Response ); } + public function actionCompleteSubscription(): ?Response + { + $subscriptionUid = $this->request->getRequiredQueryParam('subscription'); + $subscription = Subscription::find()->status(null)->uid($subscriptionUid)->one(); + + if (!$subscription) { + throw new NotFoundHttpException('Subscription not found'); + } + + $gateway = $subscription->getGateway(); + $transactionHash = $gateway->getTransactionHashFromWebhook(); + $useMutex = (bool)$transactionHash; + $transactionLockName = 'commerceTransaction:' . $transactionHash; + $mutex = Craft::$app->getMutex(); + + if ($useMutex && !$mutex->acquire($transactionLockName, 15)) { + throw new Exception('Unable to acquire a lock for transaction: ' . $transactionHash); + } + + $gateway->refreshPaymentHistory($subscription); + + if ($useMutex) { + $mutex->release($transactionLockName); + } + + return $this->asSuccess(redirect: $subscription->returnUrl); + } + + /** * @param Subscription $subscription * @throws ForbiddenHttpException @@ -450,3 +494,4 @@ private function _canUpdateSubscription(Subscription $subscription): bool return ($isOwner === true && $isFrontEnd === true); } } + diff --git a/src/elements/Subscription.php b/src/elements/Subscription.php index d6c39bdc9d..ab9b756042 100644 --- a/src/elements/Subscription.php +++ b/src/elements/Subscription.php @@ -137,6 +137,11 @@ class Subscription extends Element */ public ?DateTime $dateSuspended = null; + /** + * @var string|null The URL to return to after a subscription is created + */ + public ?string $returnUrl = null; + /** * @var SubscriptionGatewayInterface|null */ @@ -594,6 +599,7 @@ public function afterSave(bool $isNew): void $subscriptionRecord->hasStarted = $this->hasStarted; $subscriptionRecord->isSuspended = $this->isSuspended; $subscriptionRecord->dateSuspended = $this->dateSuspended; + $subscriptionRecord->returnUrl = $this->returnUrl; // We want to always have the same date as the element table, based on the logic for updating these in the element service i.e resaving $subscriptionRecord->dateUpdated = $this->dateUpdated; diff --git a/src/elements/db/SubscriptionQuery.php b/src/elements/db/SubscriptionQuery.php index 09cacb8259..8697a3535d 100644 --- a/src/elements/db/SubscriptionQuery.php +++ b/src/elements/db/SubscriptionQuery.php @@ -733,6 +733,7 @@ protected function beforePrepare(): bool 'commerce_subscriptions.subscriptionData', 'commerce_subscriptions.trialDays', 'commerce_subscriptions.userId', + 'commerce_subscriptions.returnUrl', ]); if (isset($this->userId)) { diff --git a/src/migrations/Install.php b/src/migrations/Install.php index 8dc6b9ad02..c312ed318b 100644 --- a/src/migrations/Install.php +++ b/src/migrations/Install.php @@ -710,6 +710,7 @@ public function createTables(): void 'isCanceled' => $this->boolean()->notNull()->defaultValue(false), 'dateCanceled' => $this->dateTime(), 'isExpired' => $this->boolean()->notNull()->defaultValue(false), + 'returnUrl' => $this->text(), 'dateExpired' => $this->dateTime(), 'dateCreated' => $this->dateTime()->notNull(), 'dateUpdated' => $this->dateTime()->notNull(), diff --git a/src/migrations/m240717_044256_add_return_url_to_subscription.php b/src/migrations/m240717_044256_add_return_url_to_subscription.php new file mode 100644 index 0000000000..141119118c --- /dev/null +++ b/src/migrations/m240717_044256_add_return_url_to_subscription.php @@ -0,0 +1,32 @@ +addColumn('{{%commerce_subscriptions}}', 'returnUrl', $this->text()->after('isExpired')); + + return true; + } + + /** + * @inheritdoc + */ + public function safeDown(): bool + { + echo "m240717_044256_add_return_url_to_subscription cannot be reverted.\n"; + return false; + } +} diff --git a/src/records/Subscription.php b/src/records/Subscription.php index 5d27e710cc..b89dc1f440 100644 --- a/src/records/Subscription.php +++ b/src/records/Subscription.php @@ -35,6 +35,7 @@ * @property int $trialDays * @property ActiveQueryInterface $user * @property int $userId + * @property string $returnUrl * @author Pixel & Tonic, Inc. * @since 2.0 */ From 971f666938685b2d1f044b4c9a62c7fc11e43490 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 17 Jul 2024 20:21:01 +0800 Subject: [PATCH 2/4] Cleanup --- src/controllers/SubscriptionsController.php | 2 -- .../m240717_044256_add_return_url_to_subscription.php | 1 - 2 files changed, 3 deletions(-) diff --git a/src/controllers/SubscriptionsController.php b/src/controllers/SubscriptionsController.php index bdbd3cd506..d020da650a 100644 --- a/src/controllers/SubscriptionsController.php +++ b/src/controllers/SubscriptionsController.php @@ -10,7 +10,6 @@ use Craft; use craft\base\Element; use craft\commerce\base\SubscriptionGateway; -use craft\commerce\db\Table; use craft\commerce\elements\Subscription; use craft\commerce\errors\SubscriptionException; use craft\commerce\helpers\PaymentForm; @@ -494,4 +493,3 @@ private function _canUpdateSubscription(Subscription $subscription): bool return ($isOwner === true && $isFrontEnd === true); } } - diff --git a/src/migrations/m240717_044256_add_return_url_to_subscription.php b/src/migrations/m240717_044256_add_return_url_to_subscription.php index 141119118c..42bcb8a594 100644 --- a/src/migrations/m240717_044256_add_return_url_to_subscription.php +++ b/src/migrations/m240717_044256_add_return_url_to_subscription.php @@ -2,7 +2,6 @@ namespace craft\commerce\migrations; -use Craft; use craft\db\Migration; /** From 886f8b69b6c0923f3392643ac70413a2f9015278 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 17 Jul 2024 20:32:23 +0800 Subject: [PATCH 3/4] Fix inheritance of interface --- src/base/SubscriptionGatewayInterface.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/base/SubscriptionGatewayInterface.php b/src/base/SubscriptionGatewayInterface.php index 4dfcfa6646..8a56852098 100644 --- a/src/base/SubscriptionGatewayInterface.php +++ b/src/base/SubscriptionGatewayInterface.php @@ -7,7 +7,6 @@ namespace craft\commerce\base; -use craft\base\SavableComponentInterface; use craft\commerce\elements\Subscription; use craft\commerce\errors\NotImplementedException; use craft\commerce\errors\SubscriptionException; @@ -23,7 +22,7 @@ * @author Pixel & Tonic, Inc. * @since 2.0 */ -interface SubscriptionGatewayInterface extends SavableComponentInterface +interface SubscriptionGatewayInterface extends GatewayInterface { /** * Cancels a subscription. From 4a1689f4806d9784a35f40f3cc4bad1377108245 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 17 Jul 2024 20:32:29 +0800 Subject: [PATCH 4/4] Release notes --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59d751bdfd..dabdcd1636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Release Notes for Craft Commerce +## Unreleased + +- Fixed PHP error that occurred when saving an estimated billing address. ([#3549](https://github.com/craftcms/commerce/pull/3549)) +- Fixed a bug where SCA payment sources prevented a subscription from starting. ([#3590](https://github.com/craftcms/commerce/pull/3590)) + ## 4.6.3.1 - 2024-06-20 - Fixed a PHP error that could occur on app initialization. ([#3546](https://github.com/craftcms/commerce/issues/3546))