[18.0][ADD] payment_amazon_pay: Add new module for Amazon Pay - #934
[18.0][ADD] payment_amazon_pay: Add new module for Amazon Pay#934pilarvargas-tecnativa wants to merge 2 commits into
Conversation
12e46c8 to
8027688
Compare
345982b to
21d0186
Compare
e9f870a to
d1042dd
Compare
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Code review
d1042dd to
24eb66b
Compare
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Just a minor comment on the technical side: please review my comment about displaying errors if something occurs. I imagine that if the credentials are incorrect, the API should return some errors which can be displayed to the user. What do you think about the user experience?
| "name": "Amazon Pay", | ||
| "version": "18.0.1.0.0", | ||
| "category": "Accounting/Payment Providers", | ||
| "summary": "Amazon Pay payment provider for Odoo 18", |
There was a problem hiding this comment.
I think it is not recommended to include the Odoo version here, especially if this module is migrated to v19+ in the future.
| "summary": "Amazon Pay payment provider for Odoo 18", | |
| "summary": "Amazon Pay payment provider for Odoo", |
| ): | ||
| self.ensure_one() | ||
| if not self.amazon_pay_private_key: | ||
| raise ValidationError(_("Amazon Pay private key is missing.")) |
There was a problem hiding this comment.
Check all occurrences, please.
| raise ValidationError(_("Amazon Pay private key is missing.")) | |
| raise ValidationError(self.env._("Amazon Pay private key is missing.")) |
There was a problem hiding this comment.
Done, updated all occurrences in both model files.
| payment_method = self.env["payment.method"].search( | ||
| [("code", "=", "amazon_pay")], limit=1 | ||
| ) | ||
| if payment_method: | ||
| self.payment_method_id = payment_method |
There was a problem hiding this comment.
Why do you need to fill the payment_method_id here?
The Odoo base should populate this field in https://github.com/odoo/odoo/blob/678f2a48f66ef0bfbf4fdbbaf65fdf11fd499f05/addons/payment/controllers/portal.py#L358.
This value should come from the option selected by the user. Imagine if the payment provider has more than one payment method (this may not be the case here), but theoretically, a payment provider can have more than one payment method.
There was a problem hiding this comment.
You're right, removed. The base already sets it when creating the transaction.
| try: | ||
| btn = tx.provider_id._amazon_pay_get_button_payload(tx) | ||
| except Exception: | ||
| _logger.exception( | ||
| "Amazon Pay: failed to build button payload for %s", reference | ||
| ) | ||
| return request.redirect("/payment/status") |
There was a problem hiding this comment.
I'm not sure if simply redirecting to /payment/status is OK. If I have an error, it is not displayed to the user, and they may think everything is fine when it is not.
In my case, I don't have the credentials to test this module functionally, but I entered dummy credentials, and when I pay on the website, no error is displayed; it is just redirected to /payment/status, and the user can end up waiting there without any message describing what really happened.
There was a problem hiding this comment.
Fixed. When building the button payload fails, _set_error is now called on the transaction before redirecting, so the status page reflects the actual error state.
54a26dd to
881eee3
Compare
Good point. I've updated the error handling so that API errors are now displayed to the user on the status page instead of being silently swallowed. |
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Code review LGTM
| except Exception: | ||
| _logger.exception( | ||
| "Amazon Pay: failed to build button payload for %s", reference | ||
| ) | ||
| tx._set_error("Amazon Pay: failed to prepare the payment button.") |
There was a problem hiding this comment.
I’m not sure if you intentionally didn’t add an exception message here to avoid displaying technical information to the user, but since you do it in the ValidationError, it could be added here as well. Anyway, just a comment.
| except Exception: | |
| _logger.exception( | |
| "Amazon Pay: failed to build button payload for %s", reference | |
| ) | |
| tx._set_error("Amazon Pay: failed to prepare the payment button.") | |
| except Exception as ex: | |
| _logger.exception( | |
| "Amazon Pay: failed to build button payload for %s", reference | |
| ) | |
| tx._set_error(f"Amazon Pay: failed to prepare the payment button. {str(ex)}") |
There was a problem hiding this comment.
Done. The exception message is now included in both the ValidationError and generic Exception handlers.
71b8aa3 to
ec7a6b3
Compare
| <template id="amazon_pay_checkout_page" name="Amazon Pay Checkout"> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <title>Amazon Pay</title> | ||
| <link | ||
| rel="stylesheet" | ||
| href="/payment_amazon_pay/static/src/css/amazon_pay_checkout.css" | ||
| /> | ||
| </head> | ||
| <body> | ||
| <div class="o_amazon_pay_box"> | ||
| <p>Click the button to complete your payment with Amazon Pay.</p> | ||
| <div id="o_amazon_pay_button" /> | ||
| </div> | ||
| <script id="o_amazon_pay_sdk" t-att-src="checkout_js_url" /> | ||
| <script type="application/json" id="amazon-pay-cfg"> | ||
| <t t-out="amazon_pay_cfg" /> | ||
| </script> | ||
| <script | ||
| src="/payment_amazon_pay/static/src/js/amazon_pay_checkout.js" | ||
| /> | ||
| </body> | ||
| </html> | ||
| </template> |
There was a problem hiding this comment.
This is really necessary
d640709 to
d8b5687
Compare
d8b5687 to
d160d98
Compare
69bb7e0 to
82c4cbe
Compare
82c4cbe to
70a979a
Compare
70a979a to
82a4bc3
Compare
…utton preload TT63477
juancarlosonate-tecnativa
left a comment
There was a problem hiding this comment.
Button flow needs review, it shouldn't preload in start() or mutate the shared paymentContext. It should be set up only when the option is selected instead of globally. See payment_paypal for the pattern:
https://github.com/odoo/odoo/blob/18.0/addons/payment_paypal/static/src/js/payment_form.js
@Tecnativa TT61807