-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for Klarna payment method #39
Conversation
Warning Rate limit exceeded@joamag has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes add two new methods, Changes
Sequence Diagram(s)sequenceDiagram
participant Order
participant StripeAPI
participant PaymentData
Order->>PaymentData: Retrieve payment URLs and metadata
Order->>StripeAPI: Create payment intent with parameters
StripeAPI-->>Order: Return client secret
Order->>PaymentData: Update with payment secret URL
sequenceDiagram
participant Order
participant StripeAPI
participant PaymentData
Order->>PaymentData: Retrieve payment identifier
Order->>StripeAPI: List charges for identifier
StripeAPI-->>Order: Return charge details
Order->>Order: Validate charge status and capture state
Order-->>PaymentData: Return True if payment is successful
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/budy/models/order.py (2)
399-399
: 🛠️ Refactor suggestionUpdate default payment providers to include stripe_klarna
Since you've added Klarna as a payment method, you should update the default payment providers list to include "stripe_klarna".
return appier.conf( "BUDY_PAYMENT_PROVIDERS", - ["stripe", "easypay_v2", "paypal", "stripe_sca"], + ["stripe", "easypay_v2", "paypal", "stripe_sca", "stripe_klarna"], cast=list, )
427-428
: 🛠️ Refactor suggestionAdd Klarna payment method to the class methods
You need to add a method to expose Klarna as an available payment method.
@classmethod def _pmethods_stripe_sca(cls): return ("stripe_sca",) + + @classmethod + def _pmethods_stripe_klarna(cls): + return ("stripe_klarna",)
🧹 Nitpick comments (4)
src/budy/models/order.py (4)
1964-2004
: Add a docstring to explain the Klarna payment processThis method handles Klarna payment initiation through Stripe, but it lacks a docstring explaining its purpose and workflow. Consider adding a descriptive docstring similar to the one in
_pay_stripe_sca
to improve maintainability.def _pay_stripe_klarna(self, payment_data): + """ + Starts the payment process for the current order using the + Stripe Klarna payment method. This method creates a payment + intent with Klarna as the payment method type and generates + a payment secret URL that includes the intent's client secret + and return URL. + + The payment process is as follows: + 1. The customer is redirected to the Stripe Klarna payment page. + 2. The customer confirms the payment through Klarna's interface. + 3. The customer is redirected back to the return URL with the + payment result. + + :type payment_data: dict + :param payment_data: The payment data received that is going to be used + to control the payment process. + :rtype: String + :return: The payment (secret) URL for the Stripe Klarna payment page. + """
2128-2141
: Add a docstring to explain the Klarna payment completion processSimilar to the
_pay_stripe_klarna
method, this method lacks a docstring explaining its purpose and workflow. Consider adding a descriptive docstring to improve maintainability.def _end_pay_stripe_klarna(self, payment_data): + """ + Finalizes the payment process for the current order using the + Stripe Klarna payment method. This method is called after the + customer has completed the Klarna payment flow and been redirected + back to the application. + + The method verifies that the payment was successful by checking + the charge status and capture state. + + :type payment_data: dict + :param payment_data: The payment data containing the payment identifier + and other relevant information. + :rtype: bool + :return: True if the payment was successfully processed, False otherwise. + """
2132-2140
: Consider adding error handling for Klarna-specific error casesThe current implementation checks for basic charge status, but you might want to add error handling for Klarna-specific error cases or status codes to provide more descriptive error messages to users.
1974-1976
: Add more Klarna payment parameters for better user experienceConsider adding more Klarna-specific payment parameters such as
payment_category
andcustom_payment_methods
to optimize the checkout experience.payment_method_types=["klarna"], - payment_method_options=dict(klarna=dict(preferred_locale="en-GB")), + payment_method_options=dict( + klarna=dict( + preferred_locale="en-GB", + payment_category="pay_later", + custom_payment_methods=["pay_in_30_days", "instalments"] + ) + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/budy/models/order.py
(3 hunks)
🔇 Additional comments (2)
src/budy/models/order.py (2)
2082-2094
: Good docstring update with clear explanationThe docstring update clearly explains that the method is called upon URL redirection from Stripe rather than a webhook event, which aligns with how the payment flow actually works.
1975-1976
:❓ Verification inconclusive
Verify Klarna supported locales
The preferred locale is hardcoded to "en-GB". Consider making this configurable via application settings to support different markets where Klarna operates.
🏁 Script executed:
#!/bin/bash # Check Stripe API documentation for supported Klarna locales curl -s https://stripe.com/docs/api/payment_intents/create | grep -A 10 "klarna\[preferred_locale\]"Length of output: 102
Below is a revised review comment. Since the Stripe docs query returned no clear evidence on supported Klarna locales and we haven't verified anywhere in the codebase that this value is externally configurable, please manually double-check both the Stripe integration and any related configuration settings to ensure that hardcoding "en-GB" is appropriate for your supported markets.
Action: Confirm and Externalize Klarna Locale
- The current setting of
preferred_locale="en-GB"
insrc/budy/models/order.py
(lines 1975–1976) remains hardcoded.- Verify via Stripe’s Klarna documentation and/or your integration tests that "en-GB" is indeed the intended default.
- Consider updating the code to read the locale from a configuration source (e.g., application settings) so that it can adapt to other markets.
Please review the integration and configuration to determine if this change is required.
src/budy/models/order.py
Outdated
appier.quote(secret), | ||
appier.quote(return_url), | ||
) | ||
print(secret) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove debug print statement
The print statement is debugging code that shouldn't be included in production.
- print(secret)
📝 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.
print(secret) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
12-12
: Changelog Entry Clarity and TraceabilityThe new entry clearly documents the addition of Stripe Klarna payment support as introduced in this PR. For enhanced traceability, consider adding a reference to the associated issue (e.g. "Resolves #182") if that fits your changelog style.
Related to https://github.com/hivesolutions/ustore-neo/issues/182
This pull request introduces support for Klarna payments via Stripe in the
src/budy/models/order.py
file. The most important changes include the addition of new methods to handle Klarna payments and modifications to existing methods to accommodate the new payment method.Klarna Payment Integration:
_pay_stripe_klarna
method to handle the creation of Klarna payment intents and generate the payment secret URL._end_pay_stripe_klarna
method to process the Klarna payment completion, verifying the charge status and capture status.Documentation Updates:
_end_pay_stripe_sca
method to reflect changes in the payment data description, specifically mentioning URL redirection instead of Webhook events.Summary by CodeRabbit