From 1c3c803ee1e4d02cafa6904c74f133ababe232d7 Mon Sep 17 00:00:00 2001 From: Ben Sturmfels Date: Fri, 15 Nov 2024 19:24:49 +1100 Subject: [PATCH] supporters: Handle Stripe sustainer renewals and ACH delayed payments --- conservancy/settings/base.py | 7 +- conservancy/supporters/admin.py | 47 +++- conservancy/supporters/mail.py | 2 +- .../management/commands/export_stripe.py | 13 +- ...move_sustainerorder_payment_id_and_more.py | 75 +++++ conservancy/supporters/models.py | 16 +- conservancy/supporters/views.py | 259 +++++++++++++++--- 7 files changed, 364 insertions(+), 55 deletions(-) create mode 100644 conservancy/supporters/migrations/0006_remove_sustainerorder_payment_id_and_more.py diff --git a/conservancy/settings/base.py b/conservancy/settings/base.py index 09169a0c..1fb0d289 100644 --- a/conservancy/settings/base.py +++ b/conservancy/settings/base.py @@ -63,7 +63,12 @@ LOGGING = { 'handlers': ['console'], 'level': 'DEBUG', 'propagate': False, - } + }, + 'conservancy.supporters': { + 'handlers': ['console'], + 'level': 'DEBUG', + 'propagate': False, + }, }, 'root': { 'handlers': ['console'], diff --git a/conservancy/supporters/admin.py b/conservancy/supporters/admin.py index 1492fcca..4a42d6c0 100644 --- a/conservancy/supporters/admin.py +++ b/conservancy/supporters/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin -from .models import Supporter, SustainerOrder +from .models import Supporter, SustainerOrder, SustainerPayment @admin.register(Supporter) @@ -8,13 +8,38 @@ class SupporterAdmin(admin.ModelAdmin): list_display = ('display_name', 'display_until_date') +class SustainerPaymentInline(admin.TabularInline): + model = SustainerPayment + fields = [ + 'paid_time', + 'amount', + 'stripe_payment_intent_ref', + 'stripe_invoice_ref', + ] + can_delete = False + readonly_fields = [ + 'paid_time', + 'amount', + 'stripe_payment_intent_ref', + 'stripe_invoice_ref', + ] + + def has_add_permission(self, request, obj=None): + return False + + def has_change_permission(self, request, obj=None): + return False + + + @admin.register(SustainerOrder) class SustainerOrderAdmin(admin.ModelAdmin): fields = [ 'created_time', 'paid_time', 'payment_method', - 'payment_id', + 'stripe_customer_ref', + 'stripe_subscription_ref', 'recurring', 'name', 'email', @@ -28,7 +53,21 @@ class SustainerOrderAdmin(admin.ModelAdmin): 'zip_code', 'country', ] - - readonly_fields = ['created_time', 'paid_time', 'payment_method', 'payment_id', 'recurring'] + inlines = [SustainerPaymentInline] + readonly_fields = ['created_time', 'paid_time', 'payment_method', 'stripe_customer_ref', 'stripe_subscription_ref', 'recurring'] list_display = ['created_time', 'name', 'email', 'amount', 'recurring', 'paid_time'] list_filter = ['paid_time'] + + +@admin.register(SustainerPayment) +class SustainerPaymentAdmin(admin.ModelAdmin): + fields = [ + 'order', + 'paid_time', + 'amount', + 'stripe_invoice_ref', + 'stripe_payment_intent_ref', + ] + readonly_fields = ['order', 'paid_time', 'amount', 'stripe_invoice_ref', 'stripe_payment_intent_ref'] + list_display = ['order', 'paid_time', 'amount'] + list_filter = ['paid_time'] diff --git a/conservancy/supporters/mail.py b/conservancy/supporters/mail.py index 1fdf2eed..56e1f767 100644 --- a/conservancy/supporters/mail.py +++ b/conservancy/supporters/mail.py @@ -3,7 +3,7 @@ from django.template.loader import render_to_string def make_stripe_email(order) -> EmailMessage: - subject = 'Thanks for your sustainer payment!' + subject = 'Thanks for being a sustainer!' email_body = render_to_string( 'supporters/mail/sustainer_thanks.txt', {'order': order}, diff --git a/conservancy/supporters/management/commands/export_stripe.py b/conservancy/supporters/management/commands/export_stripe.py index 8a8fa22f..e6c0f606 100644 --- a/conservancy/supporters/management/commands/export_stripe.py +++ b/conservancy/supporters/management/commands/export_stripe.py @@ -2,25 +2,26 @@ import csv import sys from django.core.management.base import BaseCommand -from ...models import SustainerOrder +from ...models import SustainerPayment class Command(BaseCommand): help = "Closes the specified poll for voting" def handle(self, *args, **options): - orders = SustainerOrder.objects.filter(paid_time__isnull=False).order_by('paid_time') + payments = SustainerPayment.objects.select_related('order').order_by('paid_time') columns = ['order_time', 'payment_time', 'name', 'email', 'amount', 'transaction_id', 'public_ack', 'shirt_size', 'join_list', 'street', 'city', 'state', 'zip_code', 'country'] writer = csv.writer(sys.stdout) writer.writerow(columns) - for order in orders: + for payment in payments: + order = payment.order writer.writerow([ order.created_time, - order.paid_time, + payment.paid_time, order.name, order.email, - order.amount, - order.payment_id, + payment.amount, + payment.stripe_payment_intent_ref, order.acknowledge_publicly, repr(order.tshirt_size if order.tshirt_size else ''), order.add_to_mailing_list, diff --git a/conservancy/supporters/migrations/0006_remove_sustainerorder_payment_id_and_more.py b/conservancy/supporters/migrations/0006_remove_sustainerorder_payment_id_and_more.py new file mode 100644 index 00000000..194b233c --- /dev/null +++ b/conservancy/supporters/migrations/0006_remove_sustainerorder_payment_id_and_more.py @@ -0,0 +1,75 @@ +# Generated by Django 5.1.2 on 2024-11-15 02:56 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('supporters', '0005_alter_sustainerorder_amount_and_more'), + ] + + operations = [ + migrations.RemoveField( + model_name='sustainerorder', + name='payment_id', + ), + migrations.AddField( + model_name='sustainerorder', + name='stripe_checkout_session_data', + field=models.JSONField(null=True), + ), + migrations.AddField( + model_name='sustainerorder', + name='stripe_customer_ref', + field=models.CharField(max_length=50, null=True), + ), + migrations.AddField( + model_name='sustainerorder', + name='stripe_initial_payment_intent_ref', + field=models.CharField(max_length=50, null=True, unique=True), + ), + migrations.AddField( + model_name='sustainerorder', + name='stripe_subscription_ref', + field=models.CharField(max_length=50, null=True, unique=True), + ), + migrations.AlterField( + model_name='sustainerorder', + name='amount', + field=models.DecimalField(decimal_places=2, max_digits=7), + ), + migrations.CreateModel( + name='SustainerPayment', + fields=[ + ( + 'id', + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID', + ), + ), + ('paid_time', models.DateTimeField(auto_now_add=True)), + ( + 'stripe_invoice_ref', + models.CharField(max_length=50, null=True, unique=True), + ), + ('amount', models.DecimalField(decimal_places=2, max_digits=7)), + ( + 'stripe_payment_intent_ref', + models.CharField(max_length=50, null=True, unique=True), + ), + ('stripe_invoice_data', models.JSONField(null=True)), + ( + 'order', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to='supporters.sustainerorder', + ), + ), + ], + ), + ] diff --git a/conservancy/supporters/models.py b/conservancy/supporters/models.py index ebacac13..7ade5b37 100644 --- a/conservancy/supporters/models.py +++ b/conservancy/supporters/models.py @@ -64,14 +64,17 @@ class SustainerOrder(models.Model): ] created_time = models.DateTimeField(auto_now_add=True) + stripe_customer_ref = models.CharField(max_length=50, null=True) + stripe_subscription_ref = models.CharField(max_length=50, null=True, unique=True) + stripe_initial_payment_intent_ref = models.CharField(max_length=50, null=True, unique=True) + stripe_checkout_session_data = models.JSONField(null=True) name = models.CharField(max_length=255) email = models.EmailField() - amount = models.PositiveIntegerField() + amount = models.DecimalField(max_digits=7, decimal_places=2) recurring = models.CharField( max_length=10, choices=RENEW_CHOICES, blank=True, default='' ) payment_method = models.CharField(max_length=10, default='Stripe') - payment_id = models.CharField(max_length=255, blank=True) paid_time = models.DateTimeField(null=True, blank=True) acknowledge_publicly = models.BooleanField(default=True) add_to_mailing_list = models.BooleanField(default=True) @@ -89,3 +92,12 @@ class SustainerOrder(models.Model): def paid(self): return self.paid_time is not None + + +class SustainerPayment(models.Model): + order = models.ForeignKey(SustainerOrder, on_delete=models.CASCADE) + paid_time = models.DateTimeField(auto_now_add=True) + stripe_invoice_ref = models.CharField(max_length=50, null=True, unique=True) + amount = models.DecimalField(max_digits=7, decimal_places=2) + stripe_payment_intent_ref = models.CharField(max_length=50, null=True, unique=True) + stripe_invoice_data = models.JSONField(null=True) diff --git a/conservancy/supporters/views.py b/conservancy/supporters/views.py index aab7b6ac..6178cd05 100644 --- a/conservancy/supporters/views.py +++ b/conservancy/supporters/views.py @@ -1,7 +1,9 @@ -from datetime import datetime +import datetime +import decimal import logging from django.conf import settings +from django.db import transaction from django.http import HttpResponse from django.shortcuts import render, redirect from django.utils import timezone @@ -9,7 +11,7 @@ import stripe from .. import ParameterValidator from . import forms, mail -from .models import Supporter, SustainerOrder +from .models import Supporter, SustainerOrder, SustainerPayment logger = logging.getLogger(__name__) @@ -35,7 +37,7 @@ def sponsors(request): Performs object queries necessary to render the sponsors page. """ - supporters = Supporter.objects.all().filter(display_until_date__gte=datetime.now()) + supporters = Supporter.objects.all().filter(display_until_date__gte=datetime.datetime.now()) supporters_count = len(supporters) anonymous_count = len(supporters.filter(display_name='Anonymous')) supporters = supporters.exclude(display_name='Anonymous').order_by('ledger_entity_id') @@ -82,6 +84,96 @@ def sustainers_paypal(request): return render(request, 'supporters/sustainers_paypal.html') +# Sustainers via Stripe +# ===================== +# +# Background and problem +# ---------------------- +# +# Conservancy accepts both one-off and monthly/annual recurring sustainer +# payments. Currently we used PayPal for this to avoid the compliance work and cost +# associated with PCI compliance. The relevant sustainer details and are sent to PayPal +# as custom fields, the donor pays via the PayPal hosted payment form, receives a +# receipt from PayPal and then later Bradley runs a batch script that takes PayPal data +# and sends a custom thanks email. (Where does the data come from? Is it a dashboard +# export or an API call?) +# +# The problem here is firstly that PayPal are difficult and somewhat risky to deal with +# in a business sense - they have been known to shut you down on a whim. Secondly we're +# heavily tied to PayPal - we're using them as a sustainer database to capture things +# like T-shirt size and address before these are imported into Bradley's +# supporter.db. To be less tied to PayPal, we would need to capture these details in our +# own database and only pass the necessary minimum details to the payment provider to +# take the payment (ie. email and payment amount). +# +# We also use PayPal to manage billing for recurring monthly/annual subscriptions, but +# that's less of an issue because that's more difficult to do reliably ourselves. +# +# We would like to integrate Stripe as a payment provider, possibly eventually replacing +# PayPal entirely for new sustainers. We have to be careful though. While Stripe were +# once focused on just accepting credit card payments, they've now moved into the +# billing and "financial automation" market so we could easily tie ourselves to Stripe +# if we're not careful. +# +# The first thing we need to do is keep our own database of sustainer orders. When a +# sustainer signs up, we record all their information and unpaid order status there and +# pass only the necessary info across to Stripe. +# +# The second thing is to produce a CSV of payments to be processed by Bradley's +# fulfilment scripts that creates Beancount bookkeeping entries, updates the +# acknowledgements on the sponsors page and determines who to send a T-shirts to (not +# immediate for monthly donors). +# +# +# Approach to integrating with Stripe +# ----------------------------------- +# +# The simplest approach to integrate Stripe seems to be to use their hosted checkout +# page. It's a currently recommended approaches as of 2024 (ie. not legacy), +# requires relatively little code, can handle complicated bank verification processes +# like 3-D Secure, allows us to switch on/of additional payment methods such as +# ACH/direct debit and avoids running proprietary JavaScript within an sfconservancy.org +# page. The tradeoff is the slightly visually jarring transition to stripe.com and +# back. With relatively little efforte we instead use an embedded Stripe form or Stripe +# widgets in our own form; this would just require marginally more code and would run +# proprietary JS within the sfconservancy.org page +# (https://docs.stripe.com/payments/accept-a-payment). From a sustainer's perspective +# it's proprietary JS either way, but it feels conceptually cleaner to isolate +# it. Nonetheless this is a slipperly SAAS slope so we need to take care. +# +# To use Stripe hosted checkout, we first accept the sustainer sign-up and populate our +# database with an unpaid order. We then create/register a Stripe checkout session via +# the API with with the donor's email, amount and renewal period if relevant and forward +# the donor across to the session's unique stripe.com URL. +# +# The donor pays on stripe.com and is then redirected to a "success_url" along with an +# ID parameter we can use to look up the payment details to determine their payment +# status. Stripe also allow you to register to accept webhook HTTP requests +# corresponding to various events in their system. One of those is +# "checkout.session.completed", which corresponds to the redirect to "success_url". The +# Stripe fulfillment docs (https://docs.stripe.com/checkout/fulfillment) advise handling +# both the "success_url" redirect and the "checkout.session.completed" webhook in case +# the redirect fails. If this was a credit card payment, we know then an there that it +# was successful. If it's an ACH/direct debit payment, it will take a few days to be +# processed and confirmed paid "checkout.session.async_payment_succeeded". +# +# We record this initial payment success against our order in the sustainer database. +# +# If auto-renewing, Stripe will transparently set up what they call a "Subscription" and +# will automatically bill the donor again next month or year +# (https://docs.stripe.com/billing/subscriptions/overview). This is a GOOD THING, +# because it avoids us having to worry about missing billing people, or worse, +# double-billing. I've been there before. Stripe +# offer an additional self-service subscription management portal for donors to use, but +# by default they don't communicate with donors directly and leave you to manage +# subscriptions manually from within the Stripe dashboard. +# +# To find out about subscription renewals, we can either batch query the Stripe API or +# we can register for webhook events. If using webhooks, there are plenty of subtle +# pitfalls such as event ordering, duplication and race conditions that could lead to us +# messing up fulfillment. The other challenge with webhooks events is that you need to +# link them back to the sustainer order they relate to in our database. + def sustainers_stripe(request): if request.method == 'POST': form = forms.SustainerForm(request.POST) @@ -108,61 +200,123 @@ if stripe.api_key == '': logger.warning('Missing STRIPE_API_KEY') -def fulfill_checkout(session_id): - print("Fulfilling Checkout Session", session_id) +def fulfill_signup(session): + session_id = session["id"] + logger.debug(f'Fulfilling checkout session {session_id}') - # TODO: Make this function safe to run multiple times, - # even concurrently, with the same session ID + # TODO: Clean up orders that have been unpaid for, say, 14 days. + # TODO: Consider emailing ACH/direct-debit donors immediately to say pending. - # TODO: Make sure fulfillment hasn't already been - # peformed for this Checkout Session - - # Retrieve the Checkout Session from the API with line_items expanded - checkout_session = stripe.checkout.Session.retrieve( + # Retrieve the Checkout Session from the API with line_items expanded so we can get + # the payment intent ID for subscriptions. + session = stripe.checkout.Session.retrieve( session_id, - expand=['line_items', 'invoice'], + expand=['invoice'], ) + # This ensure's we're looking at a sustainer checkout, not some other + # unrelated Stripe checkout. + sustainerorder_id = session['client_reference_id'] + # Check the Checkout Session's payment_status property # to determine if fulfillment should be peformed - if checkout_session.payment_status != 'unpaid': - # TODO: Perform fulfillment of the line items - - # TODO: Record/save fulfillment status for this - # Checkout Session - logger.info(f'Session ID {session_id} PAID!') + if session.payment_status != 'unpaid': + logger.debug(f'Actioning paid session {session_id}') try: - order = SustainerOrder.objects.get( - id=checkout_session['client_reference_id'], paid_time=None - ) - order.paid_time = timezone.now() - if checkout_session['payment_intent']: - # Payments get a payment intent directly - order.payment_id = checkout_session['payment_intent'] - else: - # Subscriptions go get a payment intent generated on the invoice - order.payment_id = checkout_session['invoice']['payment_intent'] - order.save() - logger.info(f'Marked sustainer order {order.id} (order.email) as paid') + with transaction.atomic(): + # Lock this order to prevent a race condition from multiple webhooks. + order = SustainerOrder.objects.filter(id=sustainerorder_id, paid_time=None).select_for_update().get() + order.stripe_customer_ref = session['customer'] + order.stripe_subscription_ref = session['subscription'] + order.stripe_checkout_session_data = session + order.stripe_initial_payment_intent_ref = ( + # One-off sustainer + session['payment_intent'] + # Subscription sustainer + or session['invoice']['payment_intent'] + ) + order.paid_time = timezone.now() + order.save() + logger.info(f'Marked sustainer order {order.id} ({order.email}) as paid') + payment = SustainerPayment.objects.create( + order=order, + stripe_invoice_ref=session['invoice']['id'] if session['invoice'] else None, + amount=decimal.Decimal(session['amount_total']) / 100, + stripe_payment_intent_ref=order.stripe_initial_payment_intent_ref, + stripe_invoice_data=session['invoice'], + ) + logger.info(f'Created sustainer payment {payment.id}') email = mail.make_stripe_email(order) email.send() except SustainerOrder.DoesNotExist: - logger.info('No action') + logger.info(f'No such unpaid SustainerOrder {sustainerorder_id} - no action') + else: + logger.debug(f'Unpaid session {session_id} - no action') + + +def fulfill_invoice_payment(invoice): + """Handle (possible) renewal payment. + + Annoyingly, this handler runs both for initial subscription and renewal payments. A + better option would be if there was an event that ran renewal payments ONLY. I + looked at "customer.subscription.updated", but couldn't seem to tell whether the + update was for a new successful renewal payment as opposed eg. someone changed the + subscription amount in the Stripe dashboard. + + Scenarios: + + 1. This could be an initial subscription payment or a renewal payment. Only + action if payment intent ID doesn't match an initial sign-up payment in our + database. + + 2. This could also be an initial payment for a subsciption not yet in the + database (events came in out of order) or a payment for a non-sustainer + subscription. That's fine - we just ignore those cases. + """ + invoice_id = invoice.id + try: + with transaction.atomic(): + # An alternative to comparing the payment intent reference would be to only + # consider orders paid > 28 days ago. Renewals should never happen before then. + order = SustainerOrder.objects.exclude(stripe_initial_payment_intent_ref=invoice['payment_intent']).get( + stripe_subscription_ref=invoice.subscription, paid_time__isnull=False, + ) + payment = SustainerPayment.objects.create( + order=order, + stripe_invoice_ref=invoice.id, + amount=decimal.Decimal(invoice.total) / 100, + stripe_payment_intent_ref=invoice['payment_intent'], + stripe_invoice_data=invoice, + ) + logger.info(f'Created sustainer payment {payment.id} for invoice {invoice_id}') + except SustainerOrder.DoesNotExist: + logger.info(f'No such subscription to renew {invoice.subscription} for invoice {invoice_id}') + def success(request): - fulfill_checkout(request.GET['session_id']) + """Handle Stripe redirect after successful checkout.""" + # We don't run the fulfillment here since it's unnecessarily complicated to run it + # both here and from webhooks. return render(request, 'supporters/stripe_success.html', {}) def webhook(request): + """Handle a request to our webhook endpoint. + + Modelled on https://docs.stripe.com/checkout/fulfillment. + + To test these, either use a service like Pagekite to set up a public link to your + development environment and configure webhooks for that, or use the Stripe CLI tool + to forward the events to your development environment. + """ payload = request.body sig_header = request.META['HTTP_STRIPE_SIGNATURE'] event = None - # From webhook dashboard + # From the "event destinations" page in Stripe's "developer tools" area. endpoint_secret = settings.STRIPE_ENDPOINT_SECRET - if endpoint_secret == '': + if not endpoint_secret: logger.warning('Missing STRIPE_ENDPOINT_SECRET') try: @@ -174,10 +328,33 @@ def webhook(request): # Invalid signature return HttpResponse(status=400) - if ( - event['type'] == 'checkout.session.completed' - or event['type'] == 'checkout.session.async_payment_succeeded' - ): - fulfill_checkout(event['data']['object']['id']) - + # Register for these webhook events the "event destinations" page. Must be + # individually enabled. + if event['type'] == 'checkout.session.completed': + # Successful Stripe checkout. For credit cards, this usually indicates that the + # payment was successful. For ACH/direct-debit the payment will not yet have + # been processed and may take a few days. + session = event['data']['object'] + logger.debug(f'CHECKOUT.SESSION.COMPLETED webhook for session {session["id"]}') + fulfill_signup(session) + elif event['type'] == 'checkout.session.async_payment_succeeded': + # Runs for successful ACH/direct debit payments. + session = event['data']['object'] + logger.debug(f'CHECKOUT.SESSION.ASYNC_PAYMENT_SUCCEEDED webhook for session {session["id"]}') + fulfill_signup(session) + elif event['type'] == 'invoice.payment_succeeded': + # Successful initial subscription or renewal payment (only care about renewals). + # + # It not clear that this is the *best* webhook or approach to use + # handle subscription renewals, but it works. + # + # You can simulate subscription renewals via the Stripe developers site: + # https://docs.stripe.com/billing/testing/test-clocks/simulate-subscriptions + # + # I found I had to advance time by 1 month first to create the invoice, then 1 + # day for it to be billed. You can watch all the events via the "stripe listen" + # CLI command. + invoice = event['data']['object'] + logger.debug(f'INVOICE.PAYMENT_SUCCEEDED webhook for invoice {invoice["id"]}') + fulfill_invoice_payment(invoice) return HttpResponse(status=200)