fix: webhook handler missing payment_status check, null metadata guard, and sync error handling #358

New issue

Closed

opened 2026-04-06 16:38:03 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented

2026-04-06 16:38:03 +00:00

Owner

Type

bug

Lineage

Standalone fix for regressions from #351

Repo

forgejo_admin/basketball-api

User Story

As a platform operator, I need the Stripe webhook handler to correctly check payment_status before marking orders as paid, handle null metadata without crashing, and commit order status even if jersey sync fails, so that webhook processing is robust against edge cases.

What Broke

_handle_generic_order_completed in webhooks.py has three bugs introduced by #351:

No payment_status check — marks orders as paid regardless of the Stripe session's payment_status field
Null metadata crash — .get("order_id") on None metadata raises TypeError
Unhandled sync errors — if sync_player_jersey_from_order raises, order status never commits

Repro Steps

Send a webhook event with payment_status: "unpaid" — order incorrectly marked paid
Send a webhook event with metadata: null — handler crashes with TypeError
If sync_player_jersey_from_order raises, the order status commit is skipped

Expected Behavior

Orders only marked paid when payment_status == "paid"
Null metadata handled gracefully (no crash)
Order status committed even if jersey sync fails

Environment

basketball-api main branch, post-merge of #351

Context

The dict() conversion on Stripe objects in #351 introduced these edge cases.

File Targets

src/basketball_api/routes/webhooks.py — fix handler logic
tests/test_checkout.py — add 4 edge case tests

Acceptance Criteria

Orders are only marked paid when payment_status == "paid"
Null metadata does not crash the handler
sync_player_jersey_from_order errors are caught; order status still commits
All 4 new tests pass
All existing tests still pass

Test Expectations

test_webhook_skips_fulfillment_when_payment_status_not_paid
test_webhook_skips_fulfillment_when_payment_status_missing
test_webhook_handles_null_metadata
test_webhook_commits_order_status_even_if_sync_fails

Constraints

No changes to existing test behavior
Minimal changes to handler logic

Checklist

Fix payment_status check
Add None guard for metadata
Wrap sync in try/except
Add 4 tests
All tests pass
ruff format + check

Fixes regressions from #351

### Type bug ### Lineage Standalone fix for regressions from #351 ### Repo forgejo_admin/basketball-api ### User Story As a platform operator, I need the Stripe webhook handler to correctly check payment_status before marking orders as paid, handle null metadata without crashing, and commit order status even if jersey sync fails, so that webhook processing is robust against edge cases. ### What Broke `_handle_generic_order_completed` in `webhooks.py` has three bugs introduced by #351: 1. No `payment_status` check — marks orders as `paid` regardless of the Stripe session's `payment_status` field 2. Null metadata crash — `.get("order_id")` on `None` metadata raises `TypeError` 3. Unhandled sync errors — if `sync_player_jersey_from_order` raises, order status never commits ### Repro Steps 1. Send a webhook event with `payment_status: "unpaid"` — order incorrectly marked paid 2. Send a webhook event with `metadata: null` — handler crashes with TypeError 3. If `sync_player_jersey_from_order` raises, the order status commit is skipped ### Expected Behavior 1. Orders only marked `paid` when `payment_status == "paid"` 2. Null metadata handled gracefully (no crash) 3. Order status committed even if jersey sync fails ### Environment basketball-api main branch, post-merge of #351 ### Context The `dict()` conversion on Stripe objects in #351 introduced these edge cases. ### File Targets - `src/basketball_api/routes/webhooks.py` — fix handler logic - `tests/test_checkout.py` — add 4 edge case tests ### Acceptance Criteria - Orders are only marked `paid` when `payment_status == "paid"` - Null metadata does not crash the handler - `sync_player_jersey_from_order` errors are caught; order status still commits - All 4 new tests pass - All existing tests still pass ### Test Expectations - `test_webhook_skips_fulfillment_when_payment_status_not_paid` - `test_webhook_skips_fulfillment_when_payment_status_missing` - `test_webhook_handles_null_metadata` - `test_webhook_commits_order_status_even_if_sync_fails` ### Constraints - No changes to existing test behavior - Minimal changes to handler logic ### Checklist - [ ] Fix payment_status check - [ ] Add None guard for metadata - [ ] Wrap sync in try/except - [ ] Add 4 tests - [ ] All tests pass - [ ] ruff format + check ### Related Fixes regressions from #351