From 8aea3c38edf623475ffc0fd3bc79ed3018ad4a5f Mon Sep 17 00:00:00 2001 From: jackiettran <41605212+jackiettran@users.noreply.github.com> Date: Fri, 9 Jan 2026 14:14:49 -0500 Subject: [PATCH] idempotency for stripe transfer, refund, charge --- backend/services/stripeService.js | 74 +++-- .../tests/unit/services/stripeService.test.js | 291 ++++++++++++------ 2 files changed, 253 insertions(+), 112 deletions(-) diff --git a/backend/services/stripeService.js b/backend/services/stripeService.js index ec14260..f0e9d21 100644 --- a/backend/services/stripeService.js +++ b/backend/services/stripeService.js @@ -104,12 +104,20 @@ class StripeService { metadata = {}, }) { try { - const transfer = await stripe.transfers.create({ - amount: Math.round(amount * 100), // Convert to cents - currency, - destination, - metadata, - }); + // Generate idempotency key from rental ID to prevent duplicate transfers + const idempotencyKey = metadata?.rentalId + ? `transfer_rental_${metadata.rentalId}` + : undefined; + + const transfer = await stripe.transfers.create( + { + amount: Math.round(amount * 100), // Convert to cents + currency, + destination, + metadata, + }, + idempotencyKey ? { idempotencyKey } : undefined + ); return transfer; } catch (error) { @@ -216,12 +224,20 @@ class StripeService { reason = "requested_by_customer", }) { try { - const refund = await stripe.refunds.create({ - payment_intent: paymentIntentId, - amount: Math.round(amount * 100), // Convert to cents - metadata, - reason, - }); + // Generate idempotency key - include amount to allow multiple partial refunds + const idempotencyKey = metadata?.rentalId + ? `refund_rental_${metadata.rentalId}_${Math.round(amount * 100)}` + : undefined; + + const refund = await stripe.refunds.create( + { + payment_intent: paymentIntentId, + amount: Math.round(amount * 100), // Convert to cents + metadata, + reason, + }, + idempotencyKey ? { idempotencyKey } : undefined + ); return refund; } catch (error) { @@ -252,20 +268,28 @@ class StripeService { metadata = {} ) { try { + // Generate idempotency key to prevent duplicate charges for same rental + const idempotencyKey = metadata?.rentalId + ? `charge_rental_${metadata.rentalId}` + : undefined; + // Create a payment intent with the stored payment method - const paymentIntent = await stripe.paymentIntents.create({ - amount: Math.round(amount * 100), // Convert to cents - currency: "usd", - payment_method: paymentMethodId, - customer: customerId, // Include customer ID - confirm: true, // Automatically confirm the payment - off_session: true, // Indicate this is an off-session payment - return_url: `${ - process.env.FRONTEND_URL || "http://localhost:3000" - }/complete-payment`, - metadata, - expand: ["latest_charge.payment_method_details"], // Expand to get payment method details - }); + const paymentIntent = await stripe.paymentIntents.create( + { + amount: Math.round(amount * 100), // Convert to cents + currency: "usd", + payment_method: paymentMethodId, + customer: customerId, // Include customer ID + confirm: true, // Automatically confirm the payment + off_session: true, // Indicate this is an off-session payment + return_url: `${ + process.env.FRONTEND_URL || "http://localhost:3000" + }/complete-payment`, + metadata, + expand: ["latest_charge.payment_method_details"], // Expand to get payment method details + }, + idempotencyKey ? { idempotencyKey } : undefined + ); // Check if additional authentication is required if (paymentIntent.status === "requires_action") { diff --git a/backend/tests/unit/services/stripeService.test.js b/backend/tests/unit/services/stripeService.test.js index 8a80e6e..a11236f 100644 --- a/backend/tests/unit/services/stripeService.test.js +++ b/backend/tests/unit/services/stripeService.test.js @@ -317,7 +317,7 @@ describe('StripeService', () => { }); describe('createTransfer', () => { - it('should create transfer with default currency', async () => { + it('should create transfer with default currency and idempotency key', async () => { const mockTransfer = { id: 'tr_123456789', amount: 5000, // $50.00 in cents @@ -340,19 +340,22 @@ describe('StripeService', () => { } }); - expect(mockStripeTransfersCreate).toHaveBeenCalledWith({ - amount: 5000, // Converted to cents - currency: 'usd', - destination: 'acct_123456789', - metadata: { - rentalId: '1', - ownerId: '2' - } - }); + expect(mockStripeTransfersCreate).toHaveBeenCalledWith( + { + amount: 5000, // Converted to cents + currency: 'usd', + destination: 'acct_123456789', + metadata: { + rentalId: '1', + ownerId: '2' + } + }, + { idempotencyKey: 'transfer_rental_1' } + ); expect(result).toEqual(mockTransfer); }); - it('should create transfer with custom currency', async () => { + it('should create transfer with custom currency and no idempotency key when no rentalId', async () => { const mockTransfer = { id: 'tr_123456789', amount: 5000, @@ -369,12 +372,15 @@ describe('StripeService', () => { destination: 'acct_123456789' }); - expect(mockStripeTransfersCreate).toHaveBeenCalledWith({ - amount: 5000, - currency: 'eur', - destination: 'acct_123456789', - metadata: {} - }); + expect(mockStripeTransfersCreate).toHaveBeenCalledWith( + { + amount: 5000, + currency: 'eur', + destination: 'acct_123456789', + metadata: {} + }, + undefined + ); expect(result).toEqual(mockTransfer); }); @@ -394,12 +400,15 @@ describe('StripeService', () => { destination: 'acct_123456789' }); - expect(mockStripeTransfersCreate).toHaveBeenCalledWith({ - amount: 12534, // Properly converted to cents - currency: 'usd', - destination: 'acct_123456789', - metadata: {} - }); + expect(mockStripeTransfersCreate).toHaveBeenCalledWith( + { + amount: 12534, // Properly converted to cents + currency: 'usd', + destination: 'acct_123456789', + metadata: {} + }, + undefined + ); }); it('should handle transfer creation errors', async () => { @@ -433,17 +442,20 @@ describe('StripeService', () => { destination: 'acct_123456789' }); - expect(mockStripeTransfersCreate).toHaveBeenCalledWith({ - amount: 1, - currency: 'usd', - destination: 'acct_123456789', - metadata: {} - }); + expect(mockStripeTransfersCreate).toHaveBeenCalledWith( + { + amount: 1, + currency: 'usd', + destination: 'acct_123456789', + metadata: {} + }, + undefined + ); }); }); describe('createRefund', () => { - it('should create refund with default parameters', async () => { + it('should create refund with default parameters and idempotency key', async () => { const mockRefund = { id: 're_123456789', amount: 5000, // $50.00 in cents @@ -465,18 +477,21 @@ describe('StripeService', () => { } }); - expect(mockStripeRefundsCreate).toHaveBeenCalledWith({ - payment_intent: 'pi_123456789', - amount: 5000, // Converted to cents - metadata: { - rentalId: '1' + expect(mockStripeRefundsCreate).toHaveBeenCalledWith( + { + payment_intent: 'pi_123456789', + amount: 5000, // Converted to cents + metadata: { + rentalId: '1' + }, + reason: 'requested_by_customer' }, - reason: 'requested_by_customer' - }); + { idempotencyKey: 'refund_rental_1_5000' } + ); expect(result).toEqual(mockRefund); }); - it('should create refund with custom reason', async () => { + it('should create refund with custom reason and no idempotency key when no rentalId', async () => { const mockRefund = { id: 're_123456789', amount: 10000, @@ -494,12 +509,15 @@ describe('StripeService', () => { reason: 'fraudulent' }); - expect(mockStripeRefundsCreate).toHaveBeenCalledWith({ - payment_intent: 'pi_123456789', - amount: 10000, - metadata: {}, - reason: 'fraudulent' - }); + expect(mockStripeRefundsCreate).toHaveBeenCalledWith( + { + payment_intent: 'pi_123456789', + amount: 10000, + metadata: {}, + reason: 'fraudulent' + }, + undefined + ); expect(result).toEqual(mockRefund); }); @@ -520,12 +538,15 @@ describe('StripeService', () => { amount: 125.34 }); - expect(mockStripeRefundsCreate).toHaveBeenCalledWith({ - payment_intent: 'pi_123456789', - amount: 12534, // Properly converted to cents - metadata: {}, - reason: 'requested_by_customer' - }); + expect(mockStripeRefundsCreate).toHaveBeenCalledWith( + { + payment_intent: 'pi_123456789', + amount: 12534, // Properly converted to cents + metadata: {}, + reason: 'requested_by_customer' + }, + undefined + ); }); it('should handle refund creation errors', async () => { @@ -612,7 +633,7 @@ describe('StripeService', () => { }); describe('chargePaymentMethod', () => { - it('should charge payment method successfully', async () => { + it('should charge payment method successfully with idempotency key', async () => { const mockPaymentIntent = { id: 'pi_123456789', status: 'succeeded', @@ -620,16 +641,14 @@ describe('StripeService', () => { amount: 5000, currency: 'usd', created: 1234567890, - charges: { - data: [{ - payment_method_details: { - type: 'card', - card: { - brand: 'visa', - last4: '4242' - } + latest_charge: { + payment_method_details: { + type: 'card', + card: { + brand: 'visa', + last4: '4242' } - }] + } } }; @@ -642,17 +661,20 @@ describe('StripeService', () => { { rentalId: '1' } ); - expect(mockStripePaymentIntentsCreate).toHaveBeenCalledWith({ - amount: 5000, // Converted to cents - currency: 'usd', - payment_method: 'pm_123456789', - customer: 'cus_123456789', - confirm: true, - off_session: true, - return_url: 'http://localhost:3000/payment-complete', - metadata: { rentalId: '1' }, - expand: ['charges.data.payment_method_details'] - }); + expect(mockStripePaymentIntentsCreate).toHaveBeenCalledWith( + { + amount: 5000, // Converted to cents + currency: 'usd', + payment_method: 'pm_123456789', + customer: 'cus_123456789', + confirm: true, + off_session: true, + return_url: 'http://localhost:3000/complete-payment', + metadata: { rentalId: '1' }, + expand: ['latest_charge.payment_method_details'] + }, + { idempotencyKey: 'charge_rental_1' } + ); expect(result).toEqual({ paymentIntentId: 'pi_123456789', @@ -684,7 +706,7 @@ describe('StripeService', () => { ); }); - it('should use default frontend URL when not set', async () => { + it('should use default frontend URL when not set and no idempotency key without rentalId', async () => { delete process.env.FRONTEND_URL; const mockPaymentIntent = { @@ -703,8 +725,9 @@ describe('StripeService', () => { expect(mockStripePaymentIntentsCreate).toHaveBeenCalledWith( expect.objectContaining({ - return_url: 'http://localhost:3000/payment-complete' - }) + return_url: 'http://localhost:3000/complete-payment' + }), + undefined ); }); @@ -726,7 +749,8 @@ describe('StripeService', () => { expect(mockStripePaymentIntentsCreate).toHaveBeenCalledWith( expect.objectContaining({ amount: 12534 // Properly converted to cents - }) + }), + undefined ); }); @@ -942,12 +966,15 @@ describe('StripeService', () => { destination: 'acct_123456789' }); - expect(mockStripeTransfersCreate).toHaveBeenCalledWith({ - amount: 99999999, - currency: 'usd', - destination: 'acct_123456789', - metadata: {} - }); + expect(mockStripeTransfersCreate).toHaveBeenCalledWith( + { + amount: 99999999, + currency: 'usd', + destination: 'acct_123456789', + metadata: {} + }, + undefined + ); }); it('should handle zero amounts', async () => { @@ -967,12 +994,15 @@ describe('StripeService', () => { amount: 0 }); - expect(mockStripeRefundsCreate).toHaveBeenCalledWith({ - payment_intent: 'pi_123456789', - amount: 0, - metadata: {}, - reason: 'requested_by_customer' - }); + expect(mockStripeRefundsCreate).toHaveBeenCalledWith( + { + payment_intent: 'pi_123456789', + amount: 0, + metadata: {}, + reason: 'requested_by_customer' + }, + undefined + ); }); it('should handle network timeout errors', async () => { @@ -1006,4 +1036,91 @@ describe('StripeService', () => { ); }); }); + + describe('Idempotency key generation', () => { + it('should generate different refund idempotency keys for different amounts on same rental', async () => { + mockStripeRefundsCreate.mockResolvedValue({ id: 're_123' }); + + // First refund - $30 + await StripeService.createRefund({ + paymentIntentId: 'pi_123', + amount: 30.00, + metadata: { rentalId: 'rental-uuid-123' } + }); + + // Second refund - $20 + await StripeService.createRefund({ + paymentIntentId: 'pi_123', + amount: 20.00, + metadata: { rentalId: 'rental-uuid-123' } + }); + + // Verify different idempotency keys were used + expect(mockStripeRefundsCreate).toHaveBeenNthCalledWith( + 1, + expect.any(Object), + { idempotencyKey: 'refund_rental_rental-uuid-123_3000' } + ); + expect(mockStripeRefundsCreate).toHaveBeenNthCalledWith( + 2, + expect.any(Object), + { idempotencyKey: 'refund_rental_rental-uuid-123_2000' } + ); + }); + + it('should generate consistent transfer idempotency key for same rental', async () => { + mockStripeTransfersCreate.mockResolvedValue({ id: 'tr_123' }); + + const rentalId = 'rental-uuid-456'; + + // Call twice with same rental + await StripeService.createTransfer({ + amount: 100.00, + destination: 'acct_123', + metadata: { rentalId } + }); + + await StripeService.createTransfer({ + amount: 100.00, + destination: 'acct_123', + metadata: { rentalId } + }); + + // Both should have the same idempotency key + expect(mockStripeTransfersCreate).toHaveBeenNthCalledWith( + 1, + expect.any(Object), + { idempotencyKey: 'transfer_rental_rental-uuid-456' } + ); + expect(mockStripeTransfersCreate).toHaveBeenNthCalledWith( + 2, + expect.any(Object), + { idempotencyKey: 'transfer_rental_rental-uuid-456' } + ); + }); + + it('should generate consistent charge idempotency key for same rental', async () => { + mockStripePaymentIntentsCreate.mockResolvedValue({ + id: 'pi_123', + status: 'succeeded', + client_secret: 'secret', + created: Date.now() / 1000, + latest_charge: null + }); + + const rentalId = 'rental-uuid-789'; + + await StripeService.chargePaymentMethod( + 'pm_123', + 100.00, + 'cus_123', + { rentalId } + ); + + expect(mockStripePaymentIntentsCreate).toHaveBeenCalledWith( + expect.any(Object), + { idempotencyKey: 'charge_rental_rental-uuid-789' } + ); + }); + }); }); \ No newline at end of file