Retry purchase to confirm the payment intent again #24
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Pay
participant Adapter
participant StripeAPI
Client->>Pay: retryPurchase(paymentId, paymentMethodId, additionalParams)
Pay->>Adapter: retryPurchase(paymentId, paymentMethodId, additionalParams)
Adapter->>StripeAPI: POST /payment_intents/{paymentId}/confirm
StripeAPI-->>Adapter: Response (payment intent status)
Adapter-->>Pay: Result array
Pay-->>Client: Result array
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/Pay/Adapter/StripeTest.php (1)
295-314: LGTM! Good basic test coverage.The test method properly verifies the retry purchase functionality with appropriate assertions. The dependency on
testPurchaseensures a valid payment intent exists for retrying. The comment about status variability is a good acknowledgment of Stripe's behavior.Consider adding additional test cases for edge scenarios:
- Retrying with a different payment method
- Testing with additional parameters
- Error handling for invalid payment IDs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Pay/Adapter.php(1 hunks)src/Pay/Adapter/Stripe.php(1 hunks)src/Pay/Pay.php(1 hunks)tests/Pay/Adapter/StripeTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/Pay/Adapter.php (2)
src/Pay/Adapter/Stripe.php (1)
retryPurchase(58-72)src/Pay/Pay.php (1)
retryPurchase(97-100)
src/Pay/Pay.php (2)
src/Pay/Adapter.php (1)
retryPurchase(91-91)src/Pay/Adapter/Stripe.php (1)
retryPurchase(58-72)
tests/Pay/Adapter/StripeTest.php (3)
src/Pay/Adapter.php (1)
retryPurchase(91-91)src/Pay/Adapter/Stripe.php (1)
retryPurchase(58-72)src/Pay/Pay.php (1)
retryPurchase(97-100)
src/Pay/Adapter/Stripe.php (2)
src/Pay/Adapter.php (1)
retryPurchase(91-91)src/Pay/Pay.php (1)
retryPurchase(97-100)
🔇 Additional comments (3)
src/Pay/Adapter.php (1)
83-91: LGTM! Well-designed abstract method.The abstract method declaration is well-structured with clear documentation, appropriate parameter types, and consistent naming. The optional parameters with sensible defaults provide good flexibility for implementations.
src/Pay/Pay.php (1)
89-100: LGTM! Proper facade delegation.The method correctly implements the facade pattern by delegating to the adapter. The signature matches the abstract declaration and follows the established pattern used by other methods in this class.
src/Pay/Adapter/Stripe.php (1)
50-72: LGTM! Correct Stripe API implementation.The implementation properly follows Stripe's payment intent confirmation API:
- Uses the correct endpoint
/payment_intents/{paymentId}/confirm- Conditionally includes payment method only when provided
- Properly merges additional parameters
- Follows the established pattern of using the
executemethodThe empty check for
$paymentMethodIdcorrectly handles both null and empty string cases.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/Pay/Adapter/StripeTest.php (2)
307-312: Inconsistent card number format.The failing payment method uses a string for the card number (
'4000000000000341') while other test methods use integers. This inconsistency could lead to confusion and potential issues.Apply this diff to maintain consistency:
- $failingPm = $this->stripe->createPaymentMethod($customerId, 'card', [ - 'number' => '4000000000000341', - 'exp_month' => 8, - 'exp_year' => 2030, - 'cvc' => 123, - ]); + $failingPm = $this->stripe->createPaymentMethod($customerId, 'card', [ + 'number' => 4000000000000341, + 'exp_month' => 8, + 'exp_year' => 2030, + 'cvc' => 123, + ]);
334-339: Maintain card number format consistency.Similar to the failing payment method, the succeeding payment method should use an integer for the card number to maintain consistency.
Apply this diff:
- $succeedingPm = $this->stripe->createPaymentMethod($customerId, 'card', [ - 'number' => '4242424242424242', // Stripe test card: always succeeds - 'exp_month' => 8, - 'exp_year' => 2030, - 'cvc' => 123, - ]); + $succeedingPm = $this->stripe->createPaymentMethod($customerId, 'card', [ + 'number' => 4242424242424242, // Stripe test card: always succeeds + 'exp_month' => 8, + 'exp_year' => 2030, + 'cvc' => 123, + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Pay/Adapter/StripeTest.php(1 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
tests/Pay/Adapter/StripeTest.php
319-319: Avoid unused local variables such as '$purchase'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (2)
tests/Pay/Adapter/StripeTest.php (2)
295-356: Overall test implementation looks solid.The test correctly validates the retry purchase functionality by:
- Creating a payment method that will fail
- Attempting a purchase and catching the expected exception
- Creating a succeeding payment method
- Retrying the purchase with the new payment method
- Asserting the payment intent ID remains the same and status is succeeded
This comprehensive test covers the core retry functionality well.
351-355: No downstream tests depend on testRetryPurchase
I searched for any@depends testRetryPurchaseand found none—overwritingpaymentMethodId/paymentIdintestRetryPurchaseis scoped to that test and does not affect other tests. No changes needed.
stnguyen90
left a comment
stnguyen90
left a comment
There was a problem hiding this comment.
Code rabbit has a good suggestion
Summary by CodeRabbit