Mollie

[mrPauwHaan]

replaces #68

Is it now correctly added @barredterra?

[barredterra]

Looks much better!

[0spinboson]

Hi all, so to clarify the scope of this PR, this is purely about being able to do payment initiation/completion, but no further integration with erpnext/payments? (So no payment processing, creating a bank transaction for a mollie virtual bank account, payment entry creation, possible reconciliation?) Or is PE creation / invoice reconciliation done through parsing the information passed along through the redirect, without setting up a webhook endpoint?

[mrPauwHaan]

@0spinboson Hi! To clarify the scope: You are correct that this PR focuses primarily on payment initiation and completion within Frappe web forms.

Specifically:
- Payment Initiation: It handles the redirect to Mollie and creates the Mollie Payment object.
- Verification: It verifies the status via the redirect URL (using the stored payment_id) to ensure the user has paid before they are redirected back to the web form's success page.
- Status Tracking: It can update a local payment_status field on the document.

What is not included (Out of Scope):

• It does not currently automate ERPNext-specific accounting flows like creating Payment Entries, Bank Transactions, or performing Invoice Reconciliation.
• It does not set up a Webhook endpoint; the verification is currently handled via the return/redirect logic.

The goal was to provide an essential 'checkout' building block for SMEs. Further integration with ERPNext’s accounting module would be a great next step, but this PR focuses on the gateway integration itself.

[0spinboson]

okay, cool. I have been LLMing away over the past months as well, working on a new module for use by associations, as part of that I also added a mollie integration (which does include those things too), and am currently working on adding a ING Checkout (Pay.nl) integration. The payment processing side of my implementation is a bit focused on verenigingen but has been written in a fairly service-oriented manner already. -> https://github.com/nlvegan/verenigingen/tree/develop/verenigingen/verenigingen_payments/mollie

Feel free to steal the webhook and mollie settings setup if you want, or we can collaborate on expansion -- I haven't really got a feel yet for the design of the payments gateways in the frappe payments app itself, and am a bit confused where the devs want to take it given that the refactor you were waiting for earlier was abandoned without it really being clear to me why (and whether there were other reasons than the ones stated).

[mrPauwHaan]

@0spinboson Thanks for sharing that link! Your implementation looks robust, especially the accounting side of things.

I totally share your confusion regarding the frappe_payments direction and the abandoned refactor. It’s a bit of a black box right now, which is part of why I kept the scope of this PR intentionally narrow.

To be transparent: For our specific internal use cases, the current implementation is sufficient, so I likely won't prioritize adding the full webhook/reconciliation logic myself in the near future. However, since you’ve already done the heavy lifting there, I would welcome you to build on top of this! If this gets merged, feel free to PR those features in, it would definitely be a great addition for the community.

[0spinboson]

okay, thanks for clarifying the intended scope. I asked my pet LLM to do a review and improve it, you can find those commits here -> develop...0spinboson:payments:fix/mollie-integration-improvements
To clarify: I also added a commit to rename the settings page to mollie checkout settings because it clashed with my own setup, hope that's okay. 🙃 That aside, any reason why you did not make the settings a singleton doctype?

[0spinboson]

its findings ->

PR #182 Review: Mollie Payment Gateway Integration

Reviewed: 2025-01-09 (Updated)
Branch: pr-182
Status: Needs Work (Improved Assessment)

Scope Clarification

This PR provides a checkout building block for SMEs using Frappe web forms:

• Payment Initiation: Redirects to Mollie and creates Mollie Payment object
• Verification: Verifies status via redirect URL (using stored payment_id)
• Status Tracking: Updates local payment_status field on the document

Intentionally Out of Scope:

• ERPNext accounting integration (Payment Entries, Bank Transactions, Reconciliation)
• Webhook endpoint (redirect-based verification is acceptable for this use case)

---

Issues Found

Critical Issues

1. Security: Insufficient Input Validation on Reference DocType (mollie_checkout.py:65-101)

   The make_payment endpoint accepts arbitrary reference_doctype and reference_docname:

   @frappe.whitelist(allow_guest=True)
   def make_payment(data, reference_doctype, reference_docname):
       # ...
       frappe.db.set_value(reference_doctype, reference_docname, 'payment_status', status)

   Why this matters (even though other gateways use allow_guest=True):

   • Stripe/Razorpay create an Integration Request log and call on_payment_authorized on the reference doc
   • Mollie writes directly to arbitrary doctypes via frappe.db.set_value()
   • Without a whitelist of allowed doctypes, a malicious actor could potentially manipulate unrelated documents

   Recommended fix:

   ALLOWED_PAYMENT_DOCTYPES = ("Web Form Submission", "Payment Request", ...)
   
   if reference_doctype not in ALLOWED_PAYMENT_DOCTYPES:
       frappe.throw(_("Invalid reference doctype"))

2. Global Mollie Client (Thread Safety) (mollie_settings.py:12-13)

   mollie_client = Client()
   mollie_error = Error()

   Why this matters:

   • Stripe imports the library inside methods: import stripe then stripe.api_key = ...
   • Mollie sets API key on a shared module-level client
   • In multi-worker environments, one request could use another's API key

   Recommended fix:

   def create_request(self, data):
       mollie_client = Client()  # Create per-request
       mollie_client.set_api_key(self.get_password(...))

3. Bare except: Clauses (mollie_checkout.py:82-83, 97-98)

   except:
       pass

   • Silently swallows ALL exceptions including KeyboardInterrupt and SystemExit
   • Makes debugging impossible

   Recommended fix:

   except frappe.DoesNotExistError:
       pass  # Field doesn't exist on this doctype, which is expected
   except Exception as e:
       frappe.log_error(f"Error checking payment_status: {e}")

Medium Issues

4. Truncated Error Logging (mollie_settings.py:127)

   frappe.log_error(frappe.get_traceback()[:140])

   • 140 characters is too short for useful debugging
   • Other gateways use full traceback: frappe.log_error(frappe.get_traceback())

5. Missing License Header

   • All other gateway files have copyright headers
   • Add: # Copyright (c) 2024, [Author] and contributors

6. HTML Attribute Without Quotes (mollie_checkout.html:18)

   <img src={{image}}>

   Should be:

   <img src="{{image}}">

   • While Jinja escapes values, unquoted attributes can cause parsing issues

7. Stray Character in JS (mollie_checkout.js:1)

   +$(document).ready(function() {

   • Remove the leading +

8. Missing Unit Tests

   • No test_mollie_settings.py file
   • Other gateways have test files; this should follow the pattern

Minor Issues

9. Inconsistent Indentation (mollie_settings.py:149-156)

   charge_data = {
               'amount': {  # Spaces instead of tabs

   • Mix of tabs and spaces; run through Black/isort

10. Unused Import (mollie_settings.py:5)

    • make_get_request is imported but only used for credential validation
    • Consider removing or documenting why it's kept

11. DocType JSON Schema

    • Uses verbose field definitions with deprecated flags
    • creation timestamp is 2017, suggesting it was copied from another gateway
    • Should be regenerated with current schema format

12. Hardcoded Field Names (mollie_checkout.py:69,79,96)

    • Assumes payment_id and payment_status fields exist on reference doctype
    • Should check if field exists before writing (which is partly why bare except exists)

---

Comparison with Stripe Implementation

Feature	Stripe	Mollie (PR)	Notes
Client instantiation	Per-request	Module-level	Mollie should follow Stripe pattern
DB writes	Via Integration Request	Direct to reference doc	Mollie should create request log first
License header	Yes	No	Add header
Unit tests	Yes	No	Add tests
Exception handling	except Exception	except: pass	Fix bare except
Minimum amount validation	Yes	No	Nice-to-have for future

---

What's Good

• Currency validation is thorough (supported_currencies list)
• Payment status flow logic is clear (Open → Pending → Completed/Cancelled)
• Redirect URL construction follows existing patterns
• on_payment_authorized hook is called for custom post-payment logic
• Integration Request logging is present in create_request()

---

Recommended Action Plan

Must Fix Before Merge:

1. Add doctype whitelist validation in make_payment()
2. Move Mollie client instantiation inside methods
3. Replace bare except: with specific exception handling
4. Add quotes around HTML attributes
5. Remove stray + from JS file

Should Fix:
6. Remove traceback truncation
7. Add license header
8. Fix indentation inconsistencies

Nice to Have:
9. Add unit tests
10. Update DocType JSON to current schema

---

Summary

The implementation provides the core checkout functionality needed for SME payment flows. The main concerns are:

1. Security: The direct db.set_value() to arbitrary doctypes needs validation
2. Thread Safety: The global Mollie client should be instantiated per-request
3. Debugging: Bare exceptions and truncated logs make troubleshooting difficult

[mahsem]

okay, cool. I have been LLMing away over the past months as well, working on a new module for use by associations, as part of that I also added a mollie integration (which does include those things too), and am currently working on adding a ING Checkout (Pay.nl) integration. The payment processing side of my implementation is a bit focused on verenigingen but has been written in a fairly service-oriented manner already. -> https://github.com/nlvegan/verenigingen/tree/develop/verenigingen/verenigingen_payments/mollie

Feel free to steal the webhook and mollie settings setup if you want, or we can collaborate on expansion -- I haven't really got a feel yet for the design of the payments gateways in the frappe payments app itself, and am a bit confused where the devs want to take it given that the refactor you were waiting for earlier was abandoned without it really being clear to me why (and whether there were other reasons than the ones stated).

@0spinboson could this implementation be integrated with your payments/gateway v2 PR

[0spinboson]

okay, cool. I have been LLMing away over the past months as well, working on a new module for use by associations, as part of that I also added a mollie integration (which does include those things too), and am currently working on adding a ING Checkout (Pay.nl) integration. The payment processing side of my implementation is a bit focused on verenigingen but has been written in a fairly service-oriented manner already. -> https://github.com/nlvegan/verenigingen/tree/develop/verenigingen/verenigingen_payments/mollie
Feel free to steal the webhook and mollie settings setup if you want, or we can collaborate on expansion -- I haven't really got a feel yet for the design of the payments gateways in the frappe payments app itself, and am a bit confused where the devs want to take it given that the refactor you were waiting for earlier was abandoned without it really being clear to me why (and whether there were other reasons than the ones stated).

@0spinboson could this implementation be integrated with your payments/gateway v2 PR

The coming month i'll be rather busy with preparing for our association's annual general meeting and some other stuff, so probably not before mid-march.

[Miesjelangelo]

Any update on this? I'm building a new webshop and would like to integrate Mollie

[0spinboson]

yes and no. the erpnext side PR was accepted, so there's progress. I am still waiting on the payments side PRs to be reviewed. I still need to do a follow-on PR for both sides to fully enable the handling of recurring payments, and then refactor the mollie implementation. Been working on some other stuff in the meantime, but could work on this some more in the coming weeks if you want to test it.

[Miesjelangelo]

Thanks for the quick update! Happy to hear there is progres. I'll check back in when I'm getting to that part of the webshop!