Add donation platform with Stripe integration and recurring support - #72
Open
ghanshyam2005singh wants to merge 4 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughAdds a complete donation flow with one-time and monthly payments, Stripe Payment Element confirmation, webhook processing, donation persistence, statistics, recent donations, validation, and success handling. ChangesDonation Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Donor
participant donate.html
participant DonationAPI
participant Stripe
participant DonationsDB
Donor->>donate.html: Select donation type and amount
donate.html->>DonationAPI: POST donation metadata
DonationAPI->>Stripe: Create PaymentIntent or Subscription
Stripe-->>DonationAPI: Return client secret and identifiers
DonationAPI->>DonationsDB: Insert pending donation
DonationAPI-->>donate.html: Return client secret
donate.html->>Stripe: Confirm Payment Element
Stripe->>DonationAPI: Send signed webhook
DonationAPI->>DonationsDB: Update or insert donation record
DonationAPI-->>Stripe: Return webhook response
Stripe-->>donate.html: Return payment status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@public/donate.html`:
- Around line 305-308: Gate the payment flow on Stripe being fully initialized
before creating any backend PaymentIntent/Subscription. In the donate page logic
around initStripe(), mountPaymentElement(), and the submit/payment handlers, add
a ready state or awaitable initialization so a fast submit cannot proceed until
stripeInstance and stripeElements exist. If Stripe is not ready, block
submission and surface a clear loading/error state rather than calling the
backend and failing later in mountPaymentElement().
- Around line 46-65: The donation type control in the HTML template is missing
proper radio-group accessibility and loses keyboard focus styling when
`onTypeChange()` swaps classes. Update the donation selector around the donation
type tabs to use `fieldset` and `legend` semantics, and make sure the
active/inactive class strings for `onTypeChange()` preserve `peer-focus-visible`
styles for the radio inputs. Keep the visible tab spans (`tab-one-time`,
`tab-monthly`) focusable via the hidden inputs so keyboard users can clearly see
focus state while navigating the group.
- Around line 105-121: Associate the amount and email validation messages with
their inputs so assistive tech can announce them. Update the custom-amount and
donor-email fields in donate.html to use aria-describedby pointing to
amount-error and email-error, set aria-invalid based on validation state, and
make the error paragraphs live regions with role="alert" or aria-live. Apply the
same accessibility pattern to the other validation fields mentioned in the form
(including the related section around the later inputs) so all submission errors
are exposed consistently.
- Around line 75-94: The preset donation amount buttons are only showing
selection via CSS, so assistive tech cannot tell which one is active. Update the
amount-selection logic for the amount-btn controls in donate.html to set and
clear aria-pressed alongside the class toggles whenever a preset is chosen, so
the current selection is announced consistently.
- Around line 300-324: The `showToast` render path is using `innerHTML` to
inject user-controlled message text, which can expose XSS risk even with manual
escaping. Update `showToast` (and the related recent-donation rendering
referenced by `renderDonation()`) to build DOM nodes with `createElement` and
assign text via `textContent` instead of concatenating HTML strings. Keep the
existing styling/icon logic, but ensure any donation names, messages, or backend
error strings are inserted only as plain text.
- Around line 382-389: The resetToForm() flow leaves selectedType and the
related tab/description state stale after form.reset(), so the UI can look reset
while submission still uses the monthly path. Update resetToForm() to also reset
selectedType back to the default one-time value and refresh the type-dependent
UI in the same place where selectedAmount and updateSummary() are reset,
ensuring backToForm() reflects the cleared monthly state.
In `@src/api/donations.py`:
- Around line 257-262: The _mark_donation helper interpolates the SQL identifier
field directly into the SELECT query, which triggers the S608 risk; constrain
field to an explicit allowlist of known Stripe donation column names before
building the statement, and reject anything else. Keep the query in
_mark_donation and its call sites unchanged except for validating the field
value before the env.DB.prepare call so only approved identifiers can be used.
- Around line 247-250: The logging in the email-sending flow leaks sensitive
data: the success message in the Mailgun send branch prints the donor’s address,
and the failure branch logs the raw provider response body. Update the logging
in the donation email handling code to use a masked or redacted recipient in the
thank-you message, and remove or sanitize the Mailgun response text before
logging it. Keep the changes localized to the email/send logic around the resp
handling so the logs remain useful without exposing PII or provider payloads.
- Around line 120-146: Add idempotency support to the mutating Stripe paths by
extending `_stripe()` to accept and forward an `Idempotency-Key` header when
provided. Update the create-call sites for `POST /payment_intents`,
`/customers`, `/prices`, and `/subscriptions` to generate and pass a stable
per-submission key so retries reuse the same Stripe request. Keep the change
localized to `_stripe` and the Stripe request helpers/callers in `donations.py`.
- Around line 151-166: The webhook verification in _verify_stripe_webhook only
checks the HMAC and currently accepts any timestamp, so add a bounded tolerance
check for the Stripe t= value before validating the signature. Parse the
timestamp in _verify_stripe_webhook, compare it against the current time with an
acceptable age window, and return False when the signed event is too old or too
far in the future. Keep the existing signature comparison logic for the v1
values after the timestamp freshness check.
- Around line 541-552: The subscription webhook handlers in
_on_subscription_created and _on_subscription_updated are overwriting
already-completed donations with lower states. Update the status transition
logic so donation status is monotonic: once _mark_donation has set a record to
completed, later customer.subscription.created or customer.subscription.updated
events should not regress it to pending or cancelled. Use the existing
_mark_donation flow and the subscription status mapping in
_on_subscription_updated to either preserve completed status or store
subscription lifecycle state separately from payment completion.
- Around line 497-516: The webhook handler in the donations flow is swallowing
exceptions and always returning success, which prevents Stripe retries on real
processing failures. Update the event dispatch logic in the webhook function to
let exceptions from handlers like _on_payment_succeeded,
_on_subscription_created, and _on_invoice_paid propagate (or return a 5xx
response) instead of catching everything and returning 200. Keep the normal 200
only for successfully processed events so transient failures can be retried.
In `@src/donations.py`:
- Around line 6-36: Use Decimal-based arithmetic in donations helpers to avoid
float rounding issues and to reject non-finite values early. Update
dollars_to_cents, cents_to_dollars, and validate_donation_payload to parse
amount_raw with Decimal, explicitly reject NaN/Infinity before conversion, and
keep cent calculations exact. While touching the signature in
validate_donation_payload, replace Tuple with the built-in tuple[...] annotation
for the return type.
In `@src/worker.py`:
- Around line 2720-2732: Add explicit dispatcher coverage for the donation route
checks in the request routing block that handles get_donation_config,
get_donation_stats, get_recent_donations, create_donation_intent,
create_subscription_intent, and handle_donation_webhook. Update
tests/test_dispatcher.py with a small route matrix that asserts each
/api/donations/* path and method maps to the correct handler, so manual string
typos or mismatched methods are caught before they become 404s or misroutes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9fcc1c49-3632-47c1-a077-09c3b30e7f41
⛔ Files ignored due to path filters (1)
migrations/0008_add_donations.sqlis excluded by!**/migrations/**
📒 Files selected for processing (6)
public/donate.htmlpublic/partials/navbar.htmlsrc/api/__init__.pysrc/api/donations.pysrc/donations.pysrc/worker.py
Contributor
|
@ghanshyam2005singh please fix the conflicts |
ghanshyam2005singh
force-pushed
the
feat/donations-stripe-integration
branch
from
July 31, 2026 16:52
e8d9e9f to
8802e6b
Compare
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@public/donate.html`:
- Around line 127-158: Add aria-hidden="true" to every decorative Font Awesome
icon in the donate.html template, including the icons identified by submit-icon
and confirm-icon and the additional icon instances around the referenced
locations. Follow the existing showToast icon pattern and leave icons that
convey meaningful information unchanged.
- Around line 663-666: Populate donor_name when creating donation records by
adding a name source from the donation form or authenticated user, and ensure
the value is included in direct donation inserts, webhook backfill, and Stripe
metadata/schema flows. Update the recent-donations query or record construction
to preserve the stored name so renderDonation() displays it for non-anonymous
donations while retaining Anonymous for explicitly anonymous donations.
- Around line 28-51: Remove aria-hidden="true" from the tab-one-time and
tab-monthly spans so their visible text contributes to the wrapping labels’
accessible names, while preserving the existing radio inputs and styling.
In `@src/api/donations.py`:
- Around line 121-148: Set a 10-second abort timeout on every outbound request
in src/api/donations.py: add the AbortSignal timeout entry to the fetch_opts
dictionary in _stripe (lines 121-148) and to the opts dictionary used by
_send_thank_you_email (lines 256-262). Ensure both js.fetch calls receive the
signal.
- Around line 156-181: Add pytest unit coverage for the pure helpers
_verify_stripe_webhook, _stripe_encode, and _verify_token, following the
project’s existing test style. Cover valid, expired and future timestamps,
malformed signature chunks, secret-rotation headers with a matching second v1
value, and _stripe_encode handling nested dictionaries, lists of dictionaries,
and booleans; include the key _verify_token paths as well.
- Around line 315-321: Update the donation statistics query near the
total_donated response to count distinct donor_email values instead of all
completed donation rows, while preserving the existing completed-status filter
and total_cents calculation. Keep the donor_count conversion and response shape
unchanged.
- Around line 435-441: Update the Stripe customer flow around the “Create or
retrieve Stripe Customer” block to look up an existing Customer by donor email
before calling POST /customers, using quote and urlencode from urllib.parse as
needed. Reuse the matching Customer when found and only create one when no match
exists; preserve the existing stripe_err handling and metadata behavior.
- Around line 603-623: Update _on_invoice_paid to read the subscription from
invoice.parent.subscription_details.subscription when invoice["subscription"] is
absent, and derive the payment intent from the newer invoice.payments structure
when invoice["payment_intent"] is unavailable. Preserve the existing
legacy-field handling and duplicate-check flow so either Stripe invoice shape
records recurring donations correctly.
- Around line 73-88: Update _verify_token to avoid duplicating
worker.verify_token: preferably reuse the canonical verifier, or retain the
local implementation only if it also validates the exp claim and rejects expired
tokens before returning decoded claims. Preserve the existing invalid-token
behavior of returning None.
- Around line 281-301: Update _mark_donation to return both the donation row and
a boolean indicating whether the status actually changed; treat current ==
status as no transition and avoid the redundant UPDATE. Update
_on_payment_succeeded to send the thank-you email only when that boolean is
true, and adjust the other callers to unpack or discard the transition flag
while preserving their existing behavior.
- Around line 609-653: Update _on_invoice_paid to let database lookup,
deduplication, and INSERT exceptions propagate to handle_donation_webhook so
transient failures return 500 and remain retryable; retain exception handling
only around _send_thank_you_email. Also add invoice-ID-based deduplication using
the dedicated invoice identifier column, including when invoice_pi is empty, and
persist that identifier in the INSERT.
- Around line 443-457: Update the monthly donation flow in the subscription
creation block to reuse the configured donated-product ID from the environment
and define the recurring amount with subscription-item price_data, rather than
calling _stripe("POST", "/prices") with product_data. Remove the standalone
price creation and use the resulting subscription item configuration when
creating the subscription, preserving the existing amount, currency, monthly
interval, and error handling behavior.
In `@src/donations.py`:
- Around line 15-25: Update the donation helpers’ signatures with concrete
annotations: add amount_dollars: float | str to dollars_to_cents, annotate the
key-based payload input for validate_donation_payload, and add appropriate
parameter types to build_donation_record and format_donation_for_display.
Preserve the existing return annotations and behavior while using types that
reflect each function’s actual payload and row access requirements.
- Around line 25-70: Add isolated business-logic tests for
validate_donation_payload covering missing, invalid, non-finite, below-minimum,
and above-maximum amounts; malformed field types; invalid donation types;
missing and malformed emails; overlong messages; and anonymous value
normalization. Assert both returned normalized payloads and exact validation
errors without involving external services or request handlers.
- Around line 48-69: Update the donation validation flow around donation_type,
email, message, and anonymous to reject unsupported JSON types before membership
checks, .strip(), or boolean conversion. Require donation_type, email, and
message values to be strings when provided, require anonymous to be an actual
boolean, and return the existing validation-error tuple format for invalid
values while preserving valid defaults and processing.
In `@src/worker.py`:
- Around line 1433-1435: Update the donation schema/index definitions near the
existing donations indexes to add a composite index covering status and
created_at, matching the filtering and ordering used by get_recent_donations.
Keep the existing individual indexes unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8b9daba6-148d-48ab-a4c2-8fccc43f2c43
⛔ Files ignored due to path filters (1)
migrations/0014_add_donations.sqlis excluded by!**/migrations/**
📒 Files selected for processing (6)
public/donate.htmlsrc/api/__init__.pysrc/api/donations.pysrc/donations.pysrc/worker.pytests/conftest.py
Comment on lines
+73
to
+88
| def _verify_token(raw: str, secret: str) -> Optional[dict]: | ||
| if not raw: | ||
| return None | ||
| try: | ||
| token = raw.removeprefix("Bearer ").strip() | ||
| dot = token.rfind(".") | ||
| if dot == -1: | ||
| return None | ||
| p, sig = token[:dot], token[dot + 1:] | ||
| exp = _hmac.new(secret.encode(), p.encode(), hashlib.sha256).hexdigest() | ||
| if not _hmac.compare_digest(sig, exp): | ||
| return None | ||
| padding = (4 - len(p) % 4) % 4 | ||
| return json.loads(base64.b64decode(p + "=" * padding).decode()) | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate existing JWT verification helpers to confirm duplication and exp handling.
set -euo pipefail
rg -nP --type=py -C6 'def\s+_?verify_token|def\s+_?verify_jwt|JWT_SECRET' -g '!src/api/donations.py'Repository: alphaonelabs/learn
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files '*.py' | sed -n '1,160p'
echo
echo "== donations token references =="
rg -n --type=py '_verify_token|verify_token|verify_jwt|JWT_SECRET|bearer|Bearer|exp' src || true
echo
echo "== donations relevant sections =="
wc -l src/api/donations.py
sed -n '1,120p' src/api/donations.py
sed -n '340,445p' src/api/donations.pyRepository: alphaonelabs/learn
Length of output: 17788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== worker verify_token =="
sed -n '390,445p' src/worker.py
echo
echo "== imported tokens tests =="
sed -n '1,220p' tests/test_tokens.py
echo
echo "== auth helpers involving tokens =="
rg -n --type=py -C3 'exp|JWT_SECRET|auth_user|verify_token|_verify_token|Authorization|Bearer' tests tests/src || true
echo
echo "== deterministic verifier behavior for absent/expired claims =="
python3 - <<'PY'
import base64, hashlib, hmac, json, time
payload = {"id": "old", "exp": int(time.time()) - 100}
# current verifier behavior: no checking payload and no validation that exp is numeric
print("payload", payload)
print("base64 token without signature", base64.b64encode(json.dumps(payload).encode()).decode())
def new(raw, secret):
if not raw:
return None
try:
token = raw.removeprefix("Bearer ").strip()
dot = token.rfind(".")
if dot == -1:
return None
p, sig = token[:dot], token[dot + 1:]
expsig = hmac.new(secret.encode(), p.encode(), hashlib.sha256).hexdigest()
if not hmac.compare_digest(sig, expsig):
return None
padding = (4 - len(p) % 4) % 4
claims = json.loads(base64.b64decode(p + "=" * padding).decode())
exp_claim = claims.get("exp")
if exp_claim is not None and time.time() > float(exp_claim):
return None
return claims
except Exception:
return None
print("signature token for expired payload", base64.b64encode(json.dumps(payload).encode()).decode() + "." + hmac.new(b"s".encode(), json.dumps(payload).encode(), hashlib.sha256).hexdigest())
print("decoded via verifier returns expired", new(base64.b64encode(json.dumps(payload).encode()).decode() + "." + hmac.new(b"s".encode(), json.dumps(payload).encode(), hashlib.sha256).hexdigest(), "s"))
payload2 = {"id": "none"}
pay2 = base64.b64encode(json.dumps(payload2).encode()).decode() + "." + hmac.new(b"s".encode(), json.dumps(payload2).encode(), hashlib.sha256).hexdigest()
print("decoded via verifier returns no-exp", new(pay2, "s"))
PYRepository: alphaonelabs/learn
Length of output: 29741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== canonical verify_token implementation =="
sed -n '396,430p' src/worker.py
echo
echo "== token creators =="
rg -n --type=py -C4 'def create_token|create_token\(' src tests
echo
echo "== deterministic verifier behavior for expired/at-exp claims =="
python3 - <<'PY'
import base64, hashlib, hmac, json, time
SECRET = b"test-seed"
PAYLOAD = {"id": "old", "exp": int(time.time()) - 100}
BASE64 = base64.b64encode(json.dumps(PAYLOAD).encode()).decode()
SIG = hmac.new(SECRET, json.dumps(PAYLOAD).encode(), hashlib.sha256).hexdigest()
TOKEN = BASE64 + "." + SIG
def verifier(raw):
if not raw:
return None
try:
token = raw.removeprefix("Bearer ").strip()
dot = token.rfind(".")
if dot == -1:
return None
p, sig = token[:dot], token[dot + 1:]
exp_signature = hmac.new(SECRET, p.encode(), hashlib.sha256).hexdigest()
if not hmac.compare_digest(sig, exp_signature):
return None
padding = (4 - len(p) % 4) % 4
return json.loads(base64.b64decode(p + "=" * padding).decode())
except Exception:
return None
payloads_without_exp = [{"id": "no-exp"}]
for payload in [PAYLOAD] + payloads_without_exp:
base = base64.b64encode(json.dumps(payload).encode()).decode()
s = hmac.new(SECRET, json.dumps(payload).encode(), hashlib.sha256).hexdigest()
token = base + "." + s
print("claims", payload)
print("accepted", verifier(token) is not None)
PYRepository: alphaonelabs/learn
Length of output: 13278
Remove the duplicate token verifier or align it with worker.verify_token.
_verify_token uses the same HMAC payload/signature checks as src/worker.py’s canonical verifier, so keeping another copy adds maintenance risk. If this helper stays local, add a check so it rejects tokens with an expired exp claim instead of accepting them and returning their claims.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 87-87: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/donations.py` around lines 73 - 88, Update _verify_token to avoid
duplicating worker.verify_token: preferably reuse the canonical verifier, or
retain the local implementation only if it also validates the exp claim and
rejects expired tokens before returning decoded claims. Preserve the existing
invalid-token behavior of returning None.
Source: Path instructions
Comment on lines
+609
to
+653
| try: | ||
| # Find the original subscription donation to copy its details | ||
| row = await env.DB.prepare( | ||
| "SELECT * FROM donations WHERE stripe_subscription_id = ? LIMIT 1" | ||
| ).bind(sub_id).first() | ||
| if not row: | ||
| print(f"[webhook] No donation found for subscription {sub_id}") | ||
| return | ||
|
|
||
| # Avoid duplicating the first payment already handled by payment_intent.succeeded | ||
| invoice_pi = invoice.get("payment_intent") or "" | ||
| if invoice_pi: | ||
| existing = await env.DB.prepare( | ||
| "SELECT id FROM donations WHERE stripe_payment_intent_id = ? LIMIT 1" | ||
| ).bind(invoice_pi).first() | ||
| if existing: | ||
| return | ||
|
|
||
| new_id = _new_id() | ||
| await env.DB.prepare( | ||
| "INSERT INTO donations" | ||
| " (id, user_id, amount, currency, donation_type, donor_email, message," | ||
| " anonymous, status, stripe_subscription_id, stripe_customer_id, stripe_payment_intent_id)" | ||
| " VALUES (?, ?, ?, ?, 'monthly', ?, ?, ?, 'completed', ?, ?, ?)" | ||
| ).bind( | ||
| new_id, | ||
| getattr(row, "user_id", None), | ||
| getattr(row, "amount", 0), | ||
| getattr(row, "currency", "usd"), | ||
| getattr(row, "donor_email", ""), | ||
| getattr(row, "message", ""), | ||
| getattr(row, "anonymous", 0), | ||
| sub_id, | ||
| getattr(row, "stripe_customer_id", None), | ||
| invoice_pi or None, | ||
| ).run() | ||
|
|
||
| print(f"[webhook] Recurring donation {new_id} recorded for subscription {sub_id}") | ||
| await _send_thank_you_email(env, { | ||
| "email": getattr(row, "donor_email", "") or "", | ||
| "amount": getattr(row, "amount", 0), | ||
| "type": "monthly", | ||
| }) | ||
| except Exception as exc: | ||
| print(f"[webhook] _on_invoice_paid error: {exc}") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
This try/except cancels the retry path that handle_donation_webhook provides.
handle_donation_webhook (Line 550) now returns 500 so Stripe retries after a transient failure. _on_invoice_paid catches every exception and returns normally, so a failed INSERT here still produces a 200. Stripe treats the event as delivered and never retries it. The recurring donation row is then lost permanently, and the donor receives no receipt for a charge that succeeded.
Let the DB errors propagate, as _mark_donation already does. Keep the guard only where a failure must not block the response — the thank-you email.
🐛 Proposed fix
sub_id = invoice.get("subscription")
if not sub_id:
return
- try:
- # Find the original subscription donation to copy its details
- row = await env.DB.prepare(
- "SELECT * FROM donations WHERE stripe_subscription_id = ? LIMIT 1"
- ).bind(sub_id).first()
- if not row:
- print(f"[webhook] No donation found for subscription {sub_id}")
- return
+ # Find the original subscription donation to copy its details
+ row = await env.DB.prepare(
+ "SELECT * FROM donations WHERE stripe_subscription_id = ? LIMIT 1"
+ ).bind(sub_id).first()
+ if not row:
+ print(f"[webhook] No donation found for subscription {sub_id}")
+ return
- # Avoid duplicating the first payment already handled by payment_intent.succeeded
- invoice_pi = invoice.get("payment_intent") or ""
- if invoice_pi:
- existing = await env.DB.prepare(
- "SELECT id FROM donations WHERE stripe_payment_intent_id = ? LIMIT 1"
- ).bind(invoice_pi).first()
- if existing:
- return
+ # Avoid duplicating the first payment already handled by payment_intent.succeeded
+ invoice_pi = invoice.get("payment_intent") or ""
+ if invoice_pi:
+ existing = await env.DB.prepare(
+ "SELECT id FROM donations WHERE stripe_payment_intent_id = ? LIMIT 1"
+ ).bind(invoice_pi).first()
+ if existing:
+ return
- new_id = _new_id()
- await env.DB.prepare(
+ new_id = _new_id()
+ await env.DB.prepare(
"INSERT INTO donations"
" (id, user_id, amount, currency, donation_type, donor_email, message,"
" anonymous, status, stripe_subscription_id, stripe_customer_id, stripe_payment_intent_id)"
" VALUES (?, ?, ?, ?, 'monthly', ?, ?, ?, 'completed', ?, ?, ?)"
- ).bind(
+ ).bind(
new_id,
getattr(row, "user_id", None),
getattr(row, "amount", 0),
getattr(row, "currency", "usd"),
getattr(row, "donor_email", ""),
getattr(row, "message", ""),
getattr(row, "anonymous", 0),
sub_id,
getattr(row, "stripe_customer_id", None),
invoice_pi or None,
- ).run()
+ ).run()
- print(f"[webhook] Recurring donation {new_id} recorded for subscription {sub_id}")
- await _send_thank_you_email(env, {
- "email": getattr(row, "donor_email", "") or "",
- "amount": getattr(row, "amount", 0),
- "type": "monthly",
- })
- except Exception as exc:
- print(f"[webhook] _on_invoice_paid error: {exc}")
+ print(f"[webhook] Recurring donation {new_id} recorded for subscription {sub_id}")
+ # _send_thank_you_email already swallows its own errors, so email failures
+ # cannot mask a successful insert.
+ await _send_thank_you_email(env, {
+ "email": getattr(row, "donor_email", "") or "",
+ "amount": getattr(row, "amount", 0),
+ "type": "monthly",
+ })Note also that this handler has no dedupe when invoice_pi is empty. If Stripe redelivers such an event, the INSERT runs twice for one charge. Storing the invoice ID in a dedicated column and checking it would close that path.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 652-652: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/donations.py` around lines 609 - 653, Update _on_invoice_paid to let
database lookup, deduplication, and INSERT exceptions propagate to
handle_donation_webhook so transient failures return 500 and remain retryable;
retain exception handling only around _send_thank_you_email. Also add
invoice-ID-based deduplication using the dedicated invoice identifier column,
including when invoice_pi is empty, and persist that identifier in the INSERT.
Comment on lines
+15
to
+25
| def dollars_to_cents(amount_dollars) -> int: | ||
| """Convert a dollar amount to cents using exact decimal arithmetic.""" | ||
| d = Decimal(str(amount_dollars)) | ||
| return int((d * 100).to_integral_value(rounding=ROUND_HALF_UP)) | ||
|
|
||
|
|
||
| def cents_to_dollars(amount_cents: int) -> float: | ||
| return float(Decimal(amount_cents) / 100) | ||
|
|
||
|
|
||
| def validate_donation_payload(body: dict) -> tuple[Optional[dict], Optional[str]]: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the declared Python target before selecting modern typing syntax.
fd -H -t f 'pyproject.toml' . -o -H -t f 'setup.py' . -o -H -t f '.python-version' . -o -H -t f 'tox.ini' . |
xargs -r rg -n -C 2 'requires-python|python_requires|target-version|python_version'Repository: alphaonelabs/learn
Length of output: 269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked config files:"
git ls-files | rg '(^|/)(pyproject\.toml|setup\.py|setup\.cfg|tox\.ini|\.python-version)$' || true
echo
echo "Python version/type hints settings:"
while IFS= read -r f; do
echo "--- $f ---"
rg -n -C 2 'requires-python|python_requires|target-version|python_version|python-version|mypy|pyupgrade' "$f" || true
done < <(git ls-files | rg '(^|/)(pyproject\.toml|setup\.py|setup\.cfg|tox\.ini|\.python-version)$' || true)
echo
echo "Candidate donation files:"
git ls-files | rg '(^|/)donations\.py$' || trueRepository: alphaonelabs/learn
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -u
echo "Repository files with likely Python config names:"
git ls-files 2>/dev/null | grep -E '(^|/)(pyproject\.toml|setup\.py|setup\.cfg|tox\.ini|\.python-version)$' || true
echo
echo "Search Python version/type-hint settings in config files:"
for f in $(git ls-files 2>/dev/null | grep -E '(^|/)(pyproject\.toml|setup\.py|setup\.cfg|tox\.ini|\.python-version)$' || true); do
echo "--- $f ---"
grep -n -C 2 -i -E 'requires-python|python_requires|target-version|python_version|python-version|mypy|pyupgrade' "$f" || true
done
echo
echo "Find donations.py files:"
git ls-files 2>/dev/null | grep -E '(^|/)donations\.py$' || true
echo
echo "Inspect src/donations.py if present:"
if [ -f src/donations.py ]; then
nl -ba src/donations.py | sed -n '1,130p'
fiRepository: alphaonelabs/learn
Length of output: 404
🏁 Script executed:
#!/bin/bash
set -u
echo "Find donations.py files:"
git ls-files 2>/dev/null | grep -E '(^|/)donations\.py$' || true
echo
echo "Inspect src/donations.py if present:"
if [ -f src/donations.py ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/donations.py | sed -n '1,140p'
fi
echo
echo "Imports and definitions summary:"
if [ -f src/donations.py ]; then
grep -nE '^(from|import |\t?[A-Za-z_][A-Za-z0-9_.]*\):|def |class ' src/donations.py | sed -n '1,200p' || true
fiRepository: alphaonelabs/learn
Length of output: 4649
Add concrete type hints for donation helper parameters.
Python files should use parameter and return type hints under the project guidelines. Add amount_dollars: float | str on dollars_to_cents, key-based input types for validate_donation_payload, and parameter types for build_donation_record and format_donation_for_display; this makes invalid inputs, payload shape, and row access failures easier to catch before tests run.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 25-25: Too many return statements (9 > 6)
(PLR0911)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/donations.py` around lines 15 - 25, Update the donation helpers’
signatures with concrete annotations: add amount_dollars: float | str to
dollars_to_cents, annotate the key-based payload input for
validate_donation_payload, and add appropriate parameter types to
build_donation_record and format_donation_for_display. Preserve the existing
return annotations and behavior while using types that reflect each function’s
actual payload and row access requirements.
Source: Path instructions
Comment on lines
+25
to
+70
| def validate_donation_payload(body: dict) -> tuple[Optional[dict], Optional[str]]: | ||
| """Validate and normalise a donation payload. | ||
|
|
||
| Returns (normalised_payload, error_message). One of the two will be None. | ||
| """ | ||
| amount_raw = body.get("amount") | ||
| if amount_raw is None: | ||
| return None, "amount is required" | ||
|
|
||
| try: | ||
| cleaned = str(amount_raw).replace(",", "").strip() | ||
| amount_d = Decimal(cleaned) | ||
| if not amount_d.is_finite(): | ||
| raise ValueError("non-finite amount") | ||
| except (InvalidOperation, ValueError): | ||
| return None, "amount must be a valid number" | ||
|
|
||
| amount_cents = dollars_to_cents(amount_d) | ||
| if amount_cents < MIN_AMOUNT_CENTS: | ||
| return None, "Minimum donation is $1.00" | ||
| if amount_cents > MAX_AMOUNT_CENTS: | ||
| return None, "Maximum donation is $10,000.00" | ||
|
|
||
| donation_type = body.get("donation_type", "one-time") | ||
| if donation_type not in VALID_DONATION_TYPES: | ||
| return None, f"donation_type must be one of: {', '.join(VALID_DONATION_TYPES)}" | ||
|
|
||
| email = (body.get("email") or "").strip() | ||
| if not email: | ||
| return None, "email is required" | ||
| if not re.match(r"^[^@\s]+@[^@\s]+\.[^@\s]+$", email): | ||
| return None, "Invalid email address" | ||
|
|
||
| message = (body.get("message") or "").strip() | ||
| if len(message) > 500: | ||
| return None, "Message must be 500 characters or less" | ||
|
|
||
| return { | ||
| "amount_cents": amount_cents, | ||
| "amount_dollars": cents_to_dollars(amount_cents), | ||
| "currency": "usd", | ||
| "donation_type": donation_type, | ||
| "email": email, | ||
| "message": message, | ||
| "anonymous": bool(body.get("anonymous", False)), | ||
| }, None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate donation tests and their covered helper functions.
fd -H -t f . tests src 2>/dev/null |
xargs -r rg -n -C 3 'validate_donation_payload|dollars_to_cents|build_donation_record|format_donation_for_display'Repository: alphaonelabs/learn
Length of output: 4464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files containing 'test' or fixture-like paths:"
git ls-files | rg '(^tests|test_|_test\.py|conftest|pytest|tox|setup\.cfg|pyproject|requirements.*test|requirements-test)' || true
echo
echo "All files with donation tests or validate_donation_payload references:"
git ls-files | rg 'tests|test|verify|pytest|tox' | xargs -r rg -n -C 4 'validate_donation_payload|dollars_to_cents|build_donation_record|format_donation_for_display|MIN_AMOUNT_CENTS|MAX_AMOUNT_CENTS|100|1000000|donation_type|anonymous|email|message' || true
echo
echo "Focused test-related searches:"
rg -n -C 3 'donation|validate_donation_payload|dollars_to_cents|anon|anonymous|min|max|email|message' tests src tests 2>/dev/null || trueRepository: alphaonelabs/learn
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files mentioning donations/API routes:"
rg -n --max-count 8 'api/donations|api_donations|donations|create_donation|create_subscription|handle_donation|validate_donation_payload' src tests | head -n 120 || true
echo
echo "Donation module outline/headers:"
if [ -f src/donations.py ]; then
sed -n '1,120p' src/donations.py | cat -n
fi
echo
echo "Precise donation test references, limited:"
rg -n --max-count 50 'validate_donation_payload|dollars_to_cents|build_donation_record|format_donation_for_display|donation_type|create_donation|create_subscription|handle_donation|Minimum donation|Maximum donation' tests src --glob '!**/worker.py' --glob '!src/worker.py' || trueRepository: alphaonelabs/learn
Length of output: 9017
Add isolated tests for donation payload validation.
validate_donation_payload() has branches for amount limits, invalid numeric input, malformed field types, email validation, message length, and anonymous normalization. Add tests that cover these paths for the business logic layer.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 25-25: Too many return statements (9 > 6)
(PLR0911)
[warning] 38-38: Abstract raise to an inner function
(TRY301)
[warning] 38-38: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/donations.py` around lines 25 - 70, Add isolated business-logic tests for
validate_donation_payload covering missing, invalid, non-finite, below-minimum,
and above-maximum amounts; malformed field types; invalid donation types;
missing and malformed emails; overlong messages; and anonymous value
normalization. Assert both returned normalized payloads and exact validation
errors without involving external services or request handlers.
Source: Path instructions
Comment on lines
+48
to
+69
| donation_type = body.get("donation_type", "one-time") | ||
| if donation_type not in VALID_DONATION_TYPES: | ||
| return None, f"donation_type must be one of: {', '.join(VALID_DONATION_TYPES)}" | ||
|
|
||
| email = (body.get("email") or "").strip() | ||
| if not email: | ||
| return None, "email is required" | ||
| if not re.match(r"^[^@\s]+@[^@\s]+\.[^@\s]+$", email): | ||
| return None, "Invalid email address" | ||
|
|
||
| message = (body.get("message") or "").strip() | ||
| if len(message) > 500: | ||
| return None, "Message must be 500 characters or less" | ||
|
|
||
| return { | ||
| "amount_cents": amount_cents, | ||
| "amount_dollars": cents_to_dollars(amount_cents), | ||
| "currency": "usd", | ||
| "donation_type": donation_type, | ||
| "email": email, | ||
| "message": message, | ||
| "anonymous": bool(body.get("anonymous", False)), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject unsupported JSON value types before processing them.
At Line 49, a list or dictionary donation_type raises TypeError during set membership. At Lines 52 and 58, non-string email or message values raise AttributeError on .strip(). Line 69 also converts "false" to True.
Validate these fields as strings and validate anonymous as a boolean. Return a validation error instead of allowing a malformed request to produce a server error or an incorrect anonymity setting.
Proposed fix
donation_type = body.get("donation_type", "one-time")
- if donation_type not in VALID_DONATION_TYPES:
+ if not isinstance(donation_type, str) or donation_type not in VALID_DONATION_TYPES:
return None, f"donation_type must be one of: {', '.join(VALID_DONATION_TYPES)}"
- email = (body.get("email") or "").strip()
+ email_raw = body.get("email")
+ if not isinstance(email_raw, str):
+ return None, "email is required"
+ email = email_raw.strip()
if not email:
return None, "email is required"
- message = (body.get("message") or "").strip()
+ message_raw = body.get("message", "")
+ if not isinstance(message_raw, str):
+ return None, "message must be a string"
+ message = message_raw.strip()
if len(message) > 500:
return None, "Message must be 500 characters or less"
+ anonymous = body.get("anonymous", False)
+ if not isinstance(anonymous, bool):
+ return None, "anonymous must be a boolean"
+
return {
- "anonymous": bool(body.get("anonymous", False)),
+ "anonymous": anonymous,
}, NoneAs per path instructions, “Ensure proper error handling and logging.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| donation_type = body.get("donation_type", "one-time") | |
| if donation_type not in VALID_DONATION_TYPES: | |
| return None, f"donation_type must be one of: {', '.join(VALID_DONATION_TYPES)}" | |
| email = (body.get("email") or "").strip() | |
| if not email: | |
| return None, "email is required" | |
| if not re.match(r"^[^@\s]+@[^@\s]+\.[^@\s]+$", email): | |
| return None, "Invalid email address" | |
| message = (body.get("message") or "").strip() | |
| if len(message) > 500: | |
| return None, "Message must be 500 characters or less" | |
| return { | |
| "amount_cents": amount_cents, | |
| "amount_dollars": cents_to_dollars(amount_cents), | |
| "currency": "usd", | |
| "donation_type": donation_type, | |
| "email": email, | |
| "message": message, | |
| "anonymous": bool(body.get("anonymous", False)), | |
| donation_type = body.get("donation_type", "one-time") | |
| if not isinstance(donation_type, str) or donation_type not in VALID_DONATION_TYPES: | |
| return None, f"donation_type must be one of: {', '.join(VALID_DONATION_TYPES)}" | |
| email_raw = body.get("email") | |
| if not isinstance(email_raw, str): | |
| return None, "email is required" | |
| email = email_raw.strip() | |
| if not email: | |
| return None, "email is required" | |
| if not re.match(r"^[^@\s]+@[^@\s]+\.[^@\s]+$", email): | |
| return None, "Invalid email address" | |
| message_raw = body.get("message", "") | |
| if not isinstance(message_raw, str): | |
| return None, "message must be a string" | |
| message = message_raw.strip() | |
| if len(message) > 500: | |
| return None, "Message must be 500 characters or less" | |
| anonymous = body.get("anonymous", False) | |
| if not isinstance(anonymous, bool): | |
| return None, "anonymous must be a boolean" | |
| return { | |
| "amount_cents": amount_cents, | |
| "amount_dollars": cents_to_dollars(amount_cents), | |
| "currency": "usd", | |
| "donation_type": donation_type, | |
| "email": email, | |
| "message": message, | |
| "anonymous": anonymous, | |
| }, None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/donations.py` around lines 48 - 69, Update the donation validation flow
around donation_type, email, message, and anonymous to reject unsupported JSON
types before membership checks, .strip(), or boolean conversion. Require
donation_type, email, and message values to be strings when provided, require
anonymous to be an actual boolean, and return the existing validation-error
tuple format for invalid values while preserving valid defaults and processing.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a complete donation system for the platform, including one-time and recurring donations, Stripe payment integration, donation statistics, recent donations feed, and webhook processing.
What is Added
Frontend
Donate Page (
public/donate.html)Backend
Donation API
Added:
GET /api/donations/statsGET /api/donations/recentPOST /api/donations/one-timePOST /api/donations/monthlyGET /api/donations/configPOST /api/donations/webhookArchitecture Improvements
Created dedicated donation modules instead of expanding
worker.py:src/donations.pyBusiness logic layer:
src/api/donations.pyAPI layer:
Database
Added:
migrations/0008_add_donations.sqlNew donations table with support for:
Stripe Integration
Implemented:
Handled events:
Email Notifications
Added Mailgun-powered thank-you emails for successful donations.
Environment Variables Required
Screenshot
Screencast.from.2026-06-28.14-35-52.mp4
Summary
This change provides a complete donation flow and supports recurring contributions. Stripe and Mailgun environment variables are required.