mollie
BUG: chargebacks & refunds resources 'get' function call invalid endpoint
The chargebacks and refunds resource implement the 'get' function simply as if it's a normal one of the basic resources (e.g.. payments, orders, customers). For these basic resources simply the name (e.g. 'payments') can prefixed with the api version and postfixed with it's id, i.e. '/v2/payments/<payment_id>'. However chargebacks and refunds are related to a payment and cannot do the same trick, as it results in the following:
- current: '/v2/chargebacks/<chargeback_id>'
- results in 404 "Invalid resource endpoint"
- should be: '/v2/payments/<payment_id>/chargebacks/<chargeback_id>'
Side note: curious to know why, if chargeback id's would be unique then there is no need to add the payment id. Could be related to the fact that they are related to payments and i.e. '/v2/payments/<payment_id>/chargebacks/' is a valid endpoint and to keep it consistent also included the payment id with the individual chargeback endpoint. Chargeback id's are also only 6 characters long while payment id's are 10. Maybe they are only unique per payment.
The chargebacks and refunds resources work in a similar fashion as captures, i.e. they need to be prefixed with /payments/<payment_id>, which the captures resources already solved:
- captures: overwrites the get_resource_name which is used in ResourceBase.get to set the path for the resource's API.
That would also be a working solution for both the chargebacks and refunds resources.
Hi @Meess ,
Thank you for your issue report. The generic issue here is the way the resources are implemented: the generic get() behavior is implemented in the ResourceBase class, which means that all subclasses inherit the method, even when they don't support it.The same goes for create(), list(), delete() and update(). We can override those to let them return an error message for all calls, but that is quite a lame solution. This architecture is legacy behavior: it was already there in the python2 client from many years ago. I agree that it is a bit annoying to expose methods in the client that actually aren't working (especially when we don't have extensive documentation describing all supported functionality of the client, leaving people like yourself at exploring the client API directly).
The correct way of retrieving a chargeback is by using the client.payment_chargebacks.get() method (or via the client.profile_chargebacks or client.settlement_chargebacks objects), which uses the Chargeback class internally. The object at client.chargebacks is not really useful as it provides no direct functionality to the end user of the client, so I will research whether we can remove this without affecting existing behavior. The same goes for client.refunds. Another way of approaching this would be to refactor the Chargebacks behavior to be more like Captures, which is similar and a bit cleaner, although I'm afraid such a change will require updates from many existing users to their existing implementations.
Finally: as to why chargeback ids aren't unique (and if/why they need a payment id for uniqueness): this is a design decision that is part of the API, and is something that I cannot answer directly. I'll pass your question on to someone at the Mollie API team, and will hopefully return you an answer later.
@Meess I asked the API design team why there is no GET /v2/chargebacks/<chargeback_id>, their response was: "nobody ever seemed to need it" :)
The team is now aware, but don't expect a change here soon as of now.
Note: after some further work on the client, I'm now seeing that the behavior of Captures is actually a bug. The path client.captures.get(<capture_id>) doesn't work and that's correct, because capture retrieval in the API is only available when combined with either a Payment ID, or a Settlement ID. Overwriting the get_resource_name() method in the Captures class is wrong, because this override is only valid in the context of Payments, not for Settlements. The get_resource_name() override should actually go in the PaymentCaptures class.
My solution to solve this is to drop both the client.settlement_captures and client.payment_captures (it seems the last one was never added) objects completely, and condense everything in the client.captures object. You can then simply do:
payment_capture = client.captures.with_parent_id(<payment_id>).get(<capture_id>)
# or, in a different scenario
settlement_capture = client.captures.with_parent_id(<settlement_id>).get(<capture_id>)
In both ways, the correct settlement will be returned, while using the correct endpoint in the API.
The same construct would then be applied for Chargebacks (and all similar-structured classes): drop client.payment_chargebacks, client.profile_chargebacks and client.settlement_chargebacks objects, and condense everything in client.chargebacks.
I'm currently refactoring the code, working towards this new layout, adding deprecation warnings as I go.
News update: we discussed this with Mollie, and decided to do a major overhaul of the Client API interface, to fix this properly (a bit different than discussed in this issue). This will however involve breaking changes in the API, so it will be released as a major version update (v3). A release candidate will be available soon.
@Meess We have just released the first release candidate for v3 of the client, which addresses this issue. You can install it using pip install --upgrade mollie-api-python --pre. There are easy before/after code examples in the documentation at https://github.com/mollie/mollie-api-python/blob/master/v3-api-changes.md
Could you test it and provide us with some feedback?
The final 3.0.0 version has been released yesterday. I consider this as resolved, feel free to reopen.