Skip to content

Feedback #1

@FredrikMPS

Description

@FredrikMPS

Here's some collected feedback from the mobile (s)t(r)eam.

  • There is no specification for how authentication is handled, I'm guessing because it hasn't been decided on, but still 😄
  • The amount property on Payment should be specified as an integer rather than a float to avoid rounding errors in the float representation in many languages. It would be in the lowest denomination of whatever currency has been specified, e.g. cents in case currencywas set to 'EUR'. This would automatically prevent someone from trying to charge a fraction of the smallest denomination as well, e.g. 3.437843 EUR.
  • The paymentTypeproperty of Payment is specified as an enum of type: object, which is not supported by the swagger specification. This leads to weirdness in the generated documentation where parts of the api spec is included as if it was part of the input data to the endpoint.
  • Having the /payments/ endpoint accept three distinct inputs doesn't feel all that RESTful, more like RPC. We suggest adding several endpoints /payments/creditcard/, /payments/token/ and /payments/encryptedcard/ each accepting just one format of input.
  • When it comes to responses of the /payments/ path, the 200 response is specified for a successful payment. Other that that only default is further specified, containing one more possible response, 402, in its description for some reason.
  • Any POST that creates a new resource that can be further operated on should return a 201 rather than a 200 and include a URI for that resource in the Location header of the response.
  • The definition of the Error model very closely resembles https://tools.ietf.org/html/draft-ietf-appsawg-http-problem-00 in semantics with mostly some differences in the naming of properties, so we could reuse that specification instead of rolling our own.
  • The balance path specifies a POST operation to retrieve the balance of your Bambora account, this should be a GET.
  • To make the API more RESTful we could try to work more with resources and vary the data on those, rather than returning messages as responses to operations.
    A payment might have a status property, to represent it's current state. This could then be set to 'declined' if a payment didn't go through or 'captured' after a successful capture.
  • A lot more validation could be added in the spec for the different properties, but this is fine for a draft.
  • What does it mean for the payment to be run "as a pre-authorization"? 😄

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions