Main#235
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
Confidence Score: 2/5The new Chapa checkout path can fail at import time, and successful payments can remain incomplete in Frappe.
payments/templates/pages/chapa_checkout.py; payments/payment_gateways/doctype/chapa_settings/chapa_settings.py; payments/payment_gateways/doctype/chapa_settings/chapa.py
|
| Filename | Overview |
|---|---|
| payments/templates/pages/chapa_checkout.py | Adds a guest checkout endpoint, but the function has an import/runtime blocker. |
| payments/payment_gateways/doctype/chapa_settings/chapa_settings.py | Adds the main Chapa settings controller, with broken controller registration, checkout return, and payment reconciliation paths. |
| payments/payment_gateways/doctype/chapa_settings/chapa.py | Adds the gateway adapter, but it delegates create_request to a missing settings method. |
| payments/payment_gateways/chapa_integration.py | Adds a separate Chapa initialization helper with a different transaction reference shape. |
| pyproject.toml | Lowers the supported Python and Frappe version requirements. |
Prompt To Fix All With AI
Fix the following 5 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 5
payments/templates/pages/chapa_checkout.py:30
**Checkout Endpoint Cannot Load**
This line mixes space indentation into a tab-indented function and uses `self` in a standalone guest endpoint. Importing or calling `chapa_checkout` fails before any Chapa session can be created.
### Issue 2 of 5
payments/payment_gateways/doctype/chapa_settings/chapa_settings.py:25
**Gateway Controller Points Nowhere**
`gateway_controller` is stored as `ChapaSettings`, but the field resolves as a Dynamic Link to a real `Chapa Settings` document name. Any flow that loads the gateway controller will request `frappe.get_doc("Chapa Settings", "ChapaSettings")` and fail unless an admin happened to create a settings document with that exact name.
### Issue 3 of 5
payments/payment_gateways/doctype/chapa_settings/chapa_settings.py:124
**Checkout URL Becomes Null**
Payments callers expect `get_payment_url()` to return a checkout URL string, but this branch sets `frappe.local.response` and returns `None`. The shared checkout and webform paths can return an empty or null response instead of redirecting the buyer to Chapa.
### Issue 4 of 5
payments/payment_gateways/doctype/chapa_settings/chapa_settings.py:133
**Verified Payments Stay Unrecorded**
The guest callback verifies `tx_ref` with Chapa, then only returns the API payload. A successful Chapa payment is never matched to the local payment request or marked paid, so buyers can pay while Frappe still shows the transaction as incomplete.
### Issue 5 of 5
payments/payment_gateways/doctype/chapa_settings/chapa.py:15
**Missing Request Creator Method**
This adapter exposes `create_request()` but delegates to `ChapaSettings.create_request`, which is not defined in the new settings controller. Any caller using the standard gateway request path will get `AttributeError` instead of an integration request.
Reviews (1): Last reviewed commit: "l" | Re-trigger Greptile
| secret_key = settings.secret_key | ||
|
|
||
| # 🔑 unique transaction reference | ||
| tx_ref = f"{self.data.reference_docname}-{uuid.uuid4().hex[:10]}" |
There was a problem hiding this comment.
This line mixes space indentation into a tab-indented function and uses self in a standalone guest endpoint. Importing or calling chapa_checkout fails before any Chapa session can be created.
Prompt To Fix With AI
This is a comment left during a code review.
Path: payments/templates/pages/chapa_checkout.py
Line: 30
Comment:
**Checkout Endpoint Cannot Load**
This line mixes space indentation into a tab-indented function and uses `self` in a standalone guest endpoint. Importing or calling `chapa_checkout` fails before any Chapa session can be created.
How can I resolve this? If you propose a fix, please make it concise.| def on_update(self): | ||
| create_payment_gateway( | ||
| "Chapa", | ||
| settings="Chapa Settings", |
There was a problem hiding this comment.
Gateway Controller Points Nowhere
gateway_controller is stored as ChapaSettings, but the field resolves as a Dynamic Link to a real Chapa Settings document name. Any flow that loads the gateway controller will request frappe.get_doc("Chapa Settings", "ChapaSettings") and fail unless an admin happened to create a settings document with that exact name.
Prompt To Fix With AI
This is a comment left during a code review.
Path: payments/payment_gateways/doctype/chapa_settings/chapa_settings.py
Line: 25
Comment:
**Gateway Controller Points Nowhere**
`gateway_controller` is stored as `ChapaSettings`, but the field resolves as a Dynamic Link to a real `Chapa Settings` document name. Any flow that loads the gateway controller will request `frappe.get_doc("Chapa Settings", "ChapaSettings")` and fail unless an admin happened to create a settings document with that exact name.
How can I resolve this? If you propose a fix, please make it concise.| frappe.local.response["type"] = "redirect" | ||
| frappe.local.response["location"] = result["data"]["checkout_url"] | ||
|
|
||
| return |
There was a problem hiding this comment.
Payments callers expect get_payment_url() to return a checkout URL string, but this branch sets frappe.local.response and returns None. The shared checkout and webform paths can return an empty or null response instead of redirecting the buyer to Chapa.
Prompt To Fix With AI
This is a comment left during a code review.
Path: payments/payment_gateways/doctype/chapa_settings/chapa_settings.py
Line: 124
Comment:
**Checkout URL Becomes Null**
Payments callers expect `get_payment_url()` to return a checkout URL string, but this branch sets `frappe.local.response` and returns `None`. The shared checkout and webform paths can return an empty or null response instead of redirecting the buyer to Chapa.
How can I resolve this? If you propose a fix, please make it concise.| # ----------------------------- | ||
| # VERIFY PAYMENT (WEBHOOK) | ||
| # ----------------------------- | ||
| @frappe.whitelist(allow_guest=True) |
There was a problem hiding this comment.
Verified Payments Stay Unrecorded
The guest callback verifies tx_ref with Chapa, then only returns the API payload. A successful Chapa payment is never matched to the local payment request or marked paid, so buyers can pay while Frappe still shows the transaction as incomplete.
Prompt To Fix With AI
This is a comment left during a code review.
Path: payments/payment_gateways/doctype/chapa_settings/chapa_settings.py
Line: 133
Comment:
**Verified Payments Stay Unrecorded**
The guest callback verifies `tx_ref` with Chapa, then only returns the API payload. A successful Chapa payment is never matched to the local payment request or marked paid, so buyers can pay while Frappe still shows the transaction as incomplete.
How can I resolve this? If you propose a fix, please make it concise.| return self.settings.get_payment_url(**kwargs) | ||
|
|
||
| def create_request(self, data): | ||
| return self.settings.create_request(data) |
There was a problem hiding this comment.
Missing Request Creator Method
This adapter exposes create_request() but delegates to ChapaSettings.create_request, which is not defined in the new settings controller. Any caller using the standard gateway request path will get AttributeError instead of an integration request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: payments/payment_gateways/doctype/chapa_settings/chapa.py
Line: 15
Comment:
**Missing Request Creator Method**
This adapter exposes `create_request()` but delegates to `ChapaSettings.create_request`, which is not defined in the new settings controller. Any caller using the standard gateway request path will get `AttributeError` instead of an integration request.
How can I resolve this? If you propose a fix, please make it concise.
No description provided.