feat: UPI payment_intent.succeeded webhook

[bseverino]

Summary

• EDUN-15452: Make Stripe payment_intent.succeeded the authoritative async path for CommerceTools order finalization when UPI learners never return to /payment-return.
• Extends commerce-coordinator Stripe webhook ingress for source_system=commercetools, writing CT Payment/Order/fulfillment state and Segment Order Completed directly (parity with customer-twou finalize from EDUN-15346).
• Aligns with Schema B.4 (Enable Payment Methods (UPI) - Schema & High-Level Technical Documentation): webhook finalizer lives in CC rather than Frontastic HTTP.
• Legacy edx/commerce_coordinator?v=1 success path and charge.refunded handling remain unchanged.
• Scheduled orphan recovery is out of scope. Paid-without-enrollment when the webhook never completes is a support edge case (manual refund / replay); revisit if volume warrants automation.
• Known launch caveat until EDUN-15347: dual-writer (browser finalize + webhook) may duplicate; accepted for this ship.

What Was Built

Component	Details
Stripe WebhookView	CT payment_intent.succeeded branch with signature verification + SingleInvocation on PI id; fires payment_succeeded_commercetools_signal. Event types compared to StripeEventType.*.value. If enqueue fails (send_robust receiver error), return 503 so Stripe retries and handle_exception clears the running flag (avoids ACK + 10-minute suppress). Refund SingleInvocation falls back to event.id when idempotency_key is null.
Shared finalize	Celery task → Charge txn (idempotent by interactionId) → Order from cart → pending fulfillment (from Initial by key) → Segment plan 18 (is_mobile=False, payment_method=upi) → PI metadata backfill (order_id, ct_payment_id) merged with existing keys. Existing-order path still heals PENDING_FULFILMENT + metadata.
CT client	get_cart_by_id (with handle_commercetools_error), create_charge_payment_transaction; get_order_by_payment_id raises ValueError when not found. Payment lookup by metadata id falls back to key only on ResourceNotFound.
Reliability	PI-scoped lock (5 min TTL, released in finally). Celery max_retries=5 with 300s countdown (same outage window as fulfillment_completed_update_ct_line_item_task); webhook ACKs Stripe only after successful enqueue, so Celery is the automated retry path for CT/Stripe errors. Structured quarantine logs for non-retryable failures.

Test Plan

• Unit tests for CT routing (success / fail / legacy / unknown / SingleInvocation short-circuit / enqueue failure → 503 and running flag cleared)
• Unit tests for shared finalize (happy path, Charge skip, already-existed fulfillment + metadata heal, CT outage propagate, ResourceNotFound vs transient, Segment props including payment_method=upi, lock contention)
• Charge client unit test
• Stage smoke: succeed CT Stripe PI without /payment-return → CT Order + Charge + pending fulfillment + PI metadata + Segment
• Confirm legacy PI success + refund webhooks unchanged in stage

Activation Checklist (ops)

• Stripe Dashboard: CC webhook endpoint subscribed to payment_intent.succeeded in stage then prod (owner / verified date). URL: https://commerce-coordinator.<env>.edx.org/stripe/webhook/

NFR / Registration

• N/A — existing edx commerce-coordinator Stripe webhook; no new EDU OpenAPI/AsyncAPI, EDU-Events, or QAaS registration in this change.

Links

• Jira: EDUN-15452 · Epic EDUN-15262
• Blocked by (parity): EDUN-15346 · Blocks: EDUN-15347
• Schema: Enable Payment Methods (UPI) - Schema & High-Level Technical Documentation

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
commerce_coordinator/apps/commercetools
clients.py					1012-1019, 1404, 1417-1426
signals.py					104-108
stripe_payment_finalize.py					93-94, 107-115, 135-139, 369-381, 417-418
tasks.py					563, 639-648, 666-667
commerce_coordinator/apps/commercetools/tests
test_clients.py
test_finalize_task.py
test_stripe_payment_finalize.py
commerce_coordinator/apps/stripe
exceptions.py
signals.py
views.py					82, 127
commerce_coordinator/apps/stripe/tests
test_views.py					121
Project Total

This report was generated by python-coverage-comment-action

[Copilot]

Pull request overview

This PR makes Stripe payment_intent.succeeded the authoritative async finalization path for CommerceTools-originated (UPI) checkouts when learners don’t return to /payment-return, and adds an orphan-recovery command to backfill missed finalizations.

Changes:

• Extends Stripe webhook routing to emit a new CT-specific “payment succeeded” signal for source_system=commercetools, dispatching a Celery finalize task.
• Introduces shared finalization logic to create/verify CT charge transaction, create CT order from cart, transition line items, emit Segment “Order Completed”, and backfill Stripe PI metadata.
• Adds a management command to discover and recover orphaned Stripe/CT payments via Stripe-primary + CT-secondary discovery, with quarantine logging.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
commerce_coordinator/settings/base.py	Wires new Stripe→CT finalize signal receiver.
commerce_coordinator/apps/stripe/views.py	Adds CT webhook routing for payment_intent.succeeded and refactors refund routing.
commerce_coordinator/apps/stripe/tests/test_views.py	Adds unit coverage for CT webhook routing and SingleInvocation behavior.
commerce_coordinator/apps/stripe/signals.py	Adds new payment_succeeded_commercetools_signal.
commerce_coordinator/apps/commercetools/signals.py	Adds receiver that dispatches the CT Stripe finalize Celery task.
commerce_coordinator/apps/commercetools/tasks.py	Adds finalize task + structured quarantine logging utilities.
commerce_coordinator/apps/commercetools/stripe_payment_finalize.py	Implements shared CT order finalization from Stripe PaymentIntents.
commerce_coordinator/apps/commercetools/clients.py	Adds CT cart fetch + charge transaction helper; changes not-found order lookup to ValueError.
commerce_coordinator/apps/commercetools/management/commands/recover_orphaned_stripe_commercetools_payments.py	Adds orphan discovery + finalize recovery command.
commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py	Adds unit tests for shared finalize flow and metadata healing.
commerce_coordinator/apps/commercetools/tests/test_recovery_command.py	Adds unit tests for recovery discovery, dry-run, finalize, and quarantine behavior.
commerce_coordinator/apps/commercetools/tests/test_finalize_task.py	Adds unit tests for finalize Celery task behavior and quarantine handling.
commerce_coordinator/apps/commercetools/tests/test_clients.py	Updates order-lookup exception type test + adds charge transaction test.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (2)

commerce_coordinator/apps/commercetools/stripe_payment_finalize.py:248

• Deriving from_state_id via order.line_items[0].state[0].state.id can raise IndexError if the order has no line items or the line-item state list is empty, and it also implicitly assumes all line items share the same current state. Since the desired transition is explicitly from the built-in Initial fulfillment state, consider looking up that state ID directly (via TwoUKeys.INITIAL_FULFILMENT_STATE) and using it as from_state_id instead of indexing into the order object.

        order_version=order.version,
        line_items=order.line_items,
        from_state_id=order.line_items[0].state[0].state.id,
        new_state_key=TwoUKeys.PENDING_FULFILMENT_STATE,
        use_state_id=True,

commerce_coordinator/apps/commercetools/management/commands/recover_orphaned_stripe_commercetools_payments.py:103

• meta = self._pi_metadata(pi_id) performs an extra Stripe API call for every orphan, even though finalize_ct_order_from_stripe_pi will immediately retrieve the PaymentIntent again. This doubles Stripe traffic for the recovery job and increases runtime/rate-limit risk. Consider fetching PI metadata lazily only inside the quarantine/exception paths (where you need it for logging), and skipping the metadata fetch on the happy path.

        for pi_id in orphans:
            meta = self._pi_metadata(pi_id)
            try:
                result = finalize_ct_order_from_stripe_pi(
                    pi_id, source="recovery", client=self.ct_api_client,

[rv-mromero]

Request changes

Architecture matches EDUN-15452: Stripe-signed payment_intent.succeeded for source_system=commercetools, shared CT finalize, recovery command, legacy/refund paths left intact. Copilot's earlier comments (metadata merge, ResourceNotFound-only fallback, max_retries on the decorator, refund event.id) are addressed. CI is green.

The gap is partial success: if the order is created and a later step fails, retry/recovery will not finish fulfillment.

Blocker: existing-order path skips fulfillment

finalize_ct_order_from_stripe_pi creates the order, then transitions line items to PENDING_FULFILMENT, then emits Segment, then backfills PI metadata. If anything after create_order_from_cart fails (CT blip, empty line_items/state, worker kill), the next run hits get_order_by_payment_id and returns already_existed=True without retrying the transition.

Same-cart double-create is naturally blocked by CommerceTools (one order per cart), so this is not the EDUN-15347 duplicate-order case. It is paid + order exists + line items still Initial, so enrollment may never start. Recovery will keep treating it as "already existed" and skip.

Fix: on the existing-order path, still transition line items that are not already pending (use TwoUKeys.INITIAL_FULFILMENT_STATE as from_state, not line_items[0].state[0]). Keep Segment best-effort if you want parity, but the state heal is the one that unblocks learners.

Medium

• Celery retries vs SingleInvocation TTL. New task: max_retries=5, countdown=3 (~15s). Fulfillment tasks in this file wait 5 minutes between retries. Webhook SingleInvocation cache TTL is 10 minutes, so Stripe's 5-minute retry is dropped; the next real webhook retry is typically ~1 hour. Recovery is the real backstop — do not treat merge as launch-ready until the cron in the PR checklist is actually scheduled.
• Recovery Search fallback can scan the whole Stripe account. Confirm the Search query in stage (docs show double quotes). If Search rejects the query, _list_filter_stripe_orphans pages every PI since --since until it finds --limit orphans. Cap pages/PIs examined.
• Recovery quarantines transients immediately. Webhook Celery retries CT/Stripe errors; the management command logs [quarantine] on the first CommercetoolsError. Next cron will retry, but a noisy New Relic alert on that contract will page during blips. Either retry a few times in-command, or don't quarantine retryable errors.
• No lock around finalize. Webhook + recovery can both enter create-order. CT will reject the second cart→order, which is fine, but concurrent Charge adds can still duplicate interactionIds. A short acquire_task_lock on PI id (same pattern as fulfillment_completed_update_ct_line_item_task) would serialize this ticket's own writers. EDUN-15347 still owns browser+webhook analytics duplication.

Low / nits

• get_cart_by_id has no handle_commercetools_error wrapper (unlike create_order_from_cart).
• Extra Stripe PaymentIntent.retrieve per orphan in recovery before finalize (Copilot); fetch metadata only on quarantine.
• Coverage: new finalize ~74%, recovery command ~76%; missing task retry-exhaustion and empty line-item paths.
• Drive-by: deleted the SONIC-898 IP-allowlist TODO on WebhookView. Unrelated; keep it or replace with a ticket.

What looks solid

• Signature check on raw body; CT vs legacy source_system split; unknown sources no-op 200.
• Charge txn skipped when interaction_id already matches; CT outage on order lookup is not treated as not-found.
• PI metadata backfill merges existing keys (source_system, ct_cart_id).
• Refund SingleInvocation falls back to event.id when idempotency_key is null.
• Segment payload: plan 18, is_mobile=False, payment_method from CT (upi in tests), totals from USD cart.
• Tests cover routing, Charge skip, metadata heal, ResourceNotFound vs transient, recovery dry-run / fallback / CT-secondary.

Launch still depends on ops, even after the code fix: Stripe Dashboard subscribed to payment_intent.succeeded, recovery cron, quarantine alert. Stage smokes in the PR body are still unchecked.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

commerce_coordinator/apps/commercetools/management/commands/recover_orphaned_stripe_commercetools_payments.py:31

• This command imports _log_quarantine from commercetools.tasks even though the leading underscore marks it as private/internal. Since this is now part of the recovery command’s stable behavior, consider making it a public helper (e.g., log_quarantine) or moving it to a shared module to avoid accidental breaking changes/renames in tasks.py.

from commerce_coordinator.apps.commercetools.stripe_payment_finalize import (
    FinalizeError,
    FinalizeInProgressError,
    finalize_ct_order_from_stripe_pi
)
from commerce_coordinator.apps.commercetools.tasks import _log_quarantine

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

commerce_coordinator/apps/commercetools/tasks.py:595

• The PR description says it adds a recovery management command recover_orphaned_stripe_commercetools_payments (plus unit tests), but that command doesn’t appear to exist anywhere in this change set (repo-wide search returns no matches). If orphan recovery is part of EDUN-15452’s acceptance criteria, the command (and its tests) likely still needs to be added here; otherwise please update the PR description/test plan to reflect what’s actually included.

    """
    Celery task wrapping the shared finalize path for a Stripe
    PaymentIntent that originated from a CommerceTools cart.

    Bounded retries on transient CT/Stripe errors; non-retryable failures
    are quarantined via structured log.
    """

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

commerce_coordinator/apps/stripe/views.py:84

• Same Enum-vs-string mismatch here: event.type needs to be compared to StripeEventType.PAYMENT_SUCCESS.value or the CT finalize path will never run.

        if event.type != StripeEventType.PAYMENT_SUCCESS:

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[colinbrash]

without the recovery command is the celery timeout the right length?  not sure if we wanted to adjust it to be closer to the fulfillment CT update task (longer in case of a CT outage). approving either way 👍

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

commerce_coordinator/apps/stripe/exceptions.py:27

• The default_detail for this 503 is a bit generic; this exception is specifically raised when the CommerceTools finalize path fails to enqueue from the webhook. Making the message explicit will help ops triage (webhook dispatch vs Stripe API failures).

class StripeWebhookDispatchAPIError(APIException):
    status_code = 503
    default_detail = 'Failed to enqueue Stripe webhook handler.'
    default_code = 'stripe_webhook_dispatch_error'

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.