emfcamp / Website

The Electromagnetic Field web site
http://www.emfcamp.org
GNU Affero General Public License v3.0
41 stars 83 forks source link

Wise webhook improvements #1324

Open russss opened 8 months ago

russss commented 8 months ago

There is a new balances#update Wise webhook which actually includes the payment reference, so we don't need to poll the Wise API to fetch the statement in the webhook.

We should run a limited reconciliation process in the webhook - create the bank transaction and commit it, then try and match it against a payment and mark it as paid.

We should also have a scheduled task which runs a full Wise reconciliation every 6 hours in the event that there were any missing webhooks.

russss commented 8 months ago

I don't think it's worth doing this for 2024 due to the problems testing it.

jayaddison commented 8 months ago

I think what we're missing here is the ability to test a stateful payment flow that involves incoming bank transfer payment (credit, I think, in Wise lingo) webhooks, particularly with custom bank references.

It's possible the Wise Sandbox will provide for this case in future, and if so that'd be a good feature to use.

Alternatively we could cover some of the basics by adding a relevant webhook unit test fixture.

Architecture-astronauting an alternative

What follows below is a braindump of how we could (note: possibly somewhat impractically) develop some end-to-end testing for payments workflows, including this one, in the interim -- and in a way that should not be tightly-coupled with either the application or the payment provider (with the idealistic goal that perhaps others could reuse and contribute to the same testing framework).

I have a vague plan to create an pywisetransfer-mockserver free/open-source project, and to allow that to handle a set of payment events and expectations - runnable within a pytest context. Similarly an additional test suite -- outside of the existing unit tests -- could be added to this repository, to confirm the resulting database state.

The tricky part here is to write the scripted test in such a way that it is easy to adapt (that is, not too tightly coupled to either the Wise API schema nor this repository's data model, because both of those will change over time). The script should be written centrally so that it's easy to read and edit, but at test-run-time, only the parts that are relevant to the Wise API should be communicated via pywisetransfer-mockserver, and only the parts relevant to the expected final state should be communicated to the application-under-test (from this repository). This implies that the script(s) should run within a third, co-ordinating process.

A test script might be something like the pseudocode below:

# bank_transfers.plan
# description: one order for two adult tickets placed and paid by bank transfer, and one order for an adult ticket placed but unpaid

VAR ticket_price 18000

CONTEXT BENEFICIARY emf_test
    OFFER adult_ticket QUANTITY 5 UNIT_PRICE {ticket_price}

    VAR paid_credit = AWAIT CREDIT TYPE bank_transfer REFERENCE {order_ref} AMOUNT {ticket_price * 2}
    WAIT_UNTIL [paid_credit]  # some other tests may require multiple, potentially parallel and/or unordered, events to occur

    VAR report = REPORT adult_ticket
    EXPECT len(report) == 2  # two orders placed
    EXPECT report[order_ref].status == 'paid'
    EXPECT sum(item.amount for item in report where item.status == 'paid') == 36000

CONTEXT ORIGINATOR paid_test
    VAR order_ref = ORDER adult_ticket QUANTITY 2  # synchronous
    VAR payment = AWAIT TRANSFER TYPE bank_transfer DESTINATION emf_test AMOUNT {ticket_price * 2} REFERENCE {order_ref}
    VAR receipt = AWAIT RECEIPT adult_ticket REFERENCE {order_ref}
    WAIT_UNTIL [payment, receipt]

    EXPECT payment.completed
    EXPECT receipt IS NOT NULL
    EXPECT receipt.charged == {ticket_price * 2}

CONTEXT ORIGINATOR unpaid_test
    ORDER adult_ticket QUANTITY 1

CONTEXT global  # after all actions and expectations are complete: sense-check the state within this test script (the co-ordinator process)
    EXPECT adult_ticket ISSUED 5
    EXPECT adult_ticket AVAILABLE 2
    ...

(this glosses over currencies, fees, refunds, and the report aspect in particular is somewhat merchant-specific. attributes for each verb should probably be nested in a structure of some kind rather than a series of same-line directives)

Although the pseudocode above looks like a custom file format, it may be simpler and more expressive to write it using a high-level language (like Python). Again, that implies three components: the application-under-test (this repository), the mocked-API-provider (pywisetransfer-mockserver) to approximate payment provider behaviour, and a third test-coordinator component.

A basic system to achieve this feels like a nontrivial amount of work, and a long-term system that can test all the intricacies of payment systems (settlement delays, refunds, disputes, evolving payment methods and networks, ...) could be a very large ongoing project.

In my opinion the aspects of implementing a system like this that require the most care would be the verbs that need to step into the system-under-test, and/or rely on application-specific details (for example, OFFER -- where items need to be created directly or indirectly in the application database, and REPORT -- where the test script and the application need to agree on common terminology).