Add server-side Stripe checkout for recurring donations #137
Conversation
tobihagemann
changed the title
WalkthroughThe pull request refactors the Stripe payment processing infrastructure across configuration, logic, and UI layers. Changes include: removing static price plan configurations from development, staging, and production parameter files; refactoring the RecurringPayment class constructor to accept a status object instead of individual parameters; replacing the Stripe JS v3 checkout flow with a server-side POST request to a new subscription checkout endpoint; and restructuring the donation credit card template to use separate per-flow captcha state objects and payment status trackers instead of a single unified captcha instance. A conditional x-ref binding was added to the captcha partial to support multiple captcha instances with customizable references. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
assets/js/cardpayments.js (1)
173-180: Consider validatingresponse.urlbefore redirect.If the backend returns an unexpected response without a
urlproperty,window.location.href = response.urlwill redirect toundefined, resulting in a broken navigation.🔎 Proposed fix
}).then(response => { // Redirect to Stripe Checkout + if (!response.url) { + throw { responseJSON: { message: 'Invalid checkout response' } }; + } window.location.href = response.url; }).catch(error => {layouts/partials/donate-creditcard.html (2)
46-46: Consider adding captcha verification check to disabled condition.The button is disabled while
oneTimeCaptchaState == 'verifying', but doesn't verify that the captcha was successfully completed (oneTimePaymentStatus.captcha != null). A user could submit before completing the captcha.🔎 Proposed fix
- <button :disabled="oneTimePaymentStatus.inProgress || !oneTimePaymentStatus.validCardNum || !acceptTerms || oneTimeCaptchaState == 'verifying'" type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-creditcard-onetime-checkout"> + <button :disabled="oneTimePaymentStatus.inProgress || !oneTimePaymentStatus.validCardNum || !acceptTerms || !oneTimePaymentStatus.captcha || oneTimeCaptchaState == 'verifying'" type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-creditcard-onetime-checkout">
61-61: Consider adding captcha verification check to disabled condition.Similar to the one-time flow, the recurring submit button should verify that the captcha was completed before enabling submission.
🔎 Proposed fix
- <button type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-creditcard-recurring-checkout" :disabled="!acceptTerms || recurringPaymentStatus.inProgress || recurringCaptchaState == 'verifying'"> + <button type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-creditcard-recurring-checkout" :disabled="!acceptTerms || !recurringPaymentStatus.captcha || recurringPaymentStatus.inProgress || recurringCaptchaState == 'verifying'">
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
assets/js/cardpayments.jsassets/js/const.template.jsconfig/development/params.yamlconfig/production/params.yamlconfig/staging/params.yamllayouts/partials/captcha.htmllayouts/partials/donate-creditcard.html
💤 Files with no reviewable changes (4)
- config/development/params.yaml
- assets/js/const.template.js
- config/production/params.yaml
- config/staging/params.yaml
🧰 Additional context used
🧬 Code graph analysis (1)
assets/js/cardpayments.js (1)
assets/js/const.template.js (1)
API_BASE_URL(3-3)
🔇 Additional comments (5)
layouts/partials/captcha.html (1)
15-15: LGTM!The conditional x-ref binding correctly supports per-flow captcha instances while maintaining backward compatibility with the default "captcha" reference.
assets/js/cardpayments.js (2)
4-4: LGTM!The new subscription checkout URL constant follows the same pattern as
STRIPE_PREPARE_PAYMENT_URLand correctly builds fromAPI_BASE_URL.
138-147: LGTM!The constructor refactor aligns
RecurringPaymentwith theOneTimePaymentpattern, enabling consistent status tracking across both payment flows.layouts/partials/donate-creditcard.html (2)
1-1: LGTM!The x-data initialization correctly establishes separate status objects and captcha states for each payment flow, enabling independent tracking and UI updates.
51-52: LGTM!The per-flow captcha integration correctly uses separate challenge URLs, state variables, and refs, enabling independent captcha validation for one-time and recurring payment flows.
Also applies to: 65-66
| const successUrl = window.location.href.split('#')[0] + 'thanks'; | ||
| const cancelUrl = window.location.href; |
There was a problem hiding this comment.
URL construction may produce incorrect path without trailing slash.
If window.location.href is /donate (no trailing slash), the successUrl becomes /donatethanks instead of /donate/thanks. Consider normalizing the base URL:
🔎 Proposed fix
- const successUrl = window.location.href.split('#')[0] + 'thanks';
+ const baseUrl = window.location.href.split('#')[0];
+ const successUrl = baseUrl.endsWith('/') ? baseUrl + 'thanks' : baseUrl + '/thanks';🤖 Prompt for AI Agents
assets/js/cardpayments.js around lines 159-160: the current successUrl
concatenation can merge path segments when the current URL lacks a trailing
slash (e.g., "/donate" -> "/donatethanks"); strip any fragment from
window.location.href, normalize the resulting base so it ends with a single
trailing slash (add one if missing, avoid duplicating), then append "thanks" to
produce "/donate/thanks"; keep cancelUrl as the full current href.
overheadhunter
left a comment
overheadhunter
left a comment
There was a problem hiding this comment.
We might also want to upgrade from stripe.js v3 to "clover". See https://docs.stripe.com/sdks/stripejs-versioning.
Updates the recurring donation flow to use server-side Checkout Session creation instead of the deprecated client-side
redirectToCheckoutAPI. TheRecurringPaymentclass now calls the new/stripe/subscriptions/checkoutendpoint and adds CAPTCHA protection (which was previously missing for recurring donations). Also removes the legacySTRIPE_PLANSclient-side config since plan IDs are now managed server-side.