[new rule] create rule for formatting prices

BirgitBader commented 1 year ago

(i) This issue has been manually transferred from a former internal repository, as a private repository issue cannot be transferred to a public repository.

Context

We currently have no rule defining the format of a money amount in an API.

As money amounts are very common in our APIs (checkout, order-positions, transactional-communication), we would like to introduce a rule that ensures that money amounts are always formatted the same.

Currently, a money amount is formatted as eurocents (integer) in the APIs checkout, order-positions, and transactional-communication.

BirgitBader commented 1 year ago

Comment history

Feb 22, 2023 by @cfranzen

Why not stick to the ISO standard for formatting currencies? (https://de.wikipedia.org/wiki/ISO_4217) Can we rely on euros as currency? I think Otto at least has some business in the UK and Switzerland.

Feb 22, 2023 by @thake

Thanks for your input @cfranzen! I think the checkout API was the first API that displayed money amounts. @jensfischer1515, can you give some background on why eurocents has been chosen?

BirgitBader commented 1 year ago

Feb 23, 2023 by @thake

I've compiled an overview of some possible formats and their pro's and con's:

BirgitBader commented 1 year ago

Feb 23, 2023 by @jensfischer1515

Historical background after some software archaeological research:

Checkout API price modelling was copied from legacy Order API (REST for ordering OTTO products only) in 2018.
Order API initially modelled price as EuroCents using Long in 2017.
This was done to mimic already established price formats e.g. in B2C/Core or product data interfaces.
Another technical reason was to prevent rounding problems.
Although also modelling a currency field was initially thought of, it was not implemented as such. Reasons forgotten, probably to reduce complexity for EUR-only business model.
There is no trace to be found that gross vs. net prices has ever been considered, probably also due to simpler business model requirements.

BirgitBader commented 1 year ago

Feb 28, 2023 by @thake

OrderEvent of kraken uses:


{
  "price": {"amount": 10.34, "currency": "EUR"}
}

BirgitBader commented 1 year ago

Feb 28, 2023 by @thake

Proposal

My proposed format after a discussion with @jensfischer1515, @JanKlasser3000, @BastianSperrhacke-Otto, and @sergey-kolodiev

Example

{
  "price": {"amount":  "1.45", "currency": "EUR" /* optional, defaults to EUR */}
}

Schema

title: Money
description: Common format for money amounts.
type: object
required:
  - amount
properties:
  amount:
    type: string
    description: The money amount without currency. "." is used as a decimal separator.
    pattern: ^\d+(\.\d+)?$
  currency:
    type: string
    description: The alphabetic currency code for the amount as defined in ISO 4217.
    enum:
      - "EUR"
    default: "EUR"
    pattern: "^[A-Z]{3}$"

Reasoning

The object format has been used in preference to eurocents because of the following reasons:

Although currently not needed, a clear separation of currency and money amount is possible.
Easier to use in OpenAPI and AsyncAPI definitions because one does not need to repeat the description of the money amount every time it is used in a property.

string has been chosen as a type for amount because number is deserialized to an IEEE 754 float representation (see https://floating-point-gui.de/). The representation as float can lead to rounding and calculation errors. This is especially true for currencies with many decimal places (e.g., bitcoin). Example in javascript:

let a = 2.2
let b = 2.1
console.log(a+b) //logs 4.300000000000001

currency is by default "EUR" and thus can be absent for most of OTTOs APIs. This reduces the payload for the current use cases.

Remarks

All participants of today's meeting were fully aware that introducing a new currency will always be a breaking change. APIs and their clients need to make adaptions to handle a new currency.

BirgitBader commented 1 year ago

Mar 2, 2023 by @cfranzen

I think that is a good proposal, although I dislike the type of amount being string. Most languages / frameworks would handle type "number" correctly (e.g. Spring, Micronaut) but some popular does not (e.g. Node, Go). So using string seems to be the best option.

BirgitBader commented 1 year ago

Mar 2, 2023 by @thake

Regarding the type of amount: @JanKlasser3000 did some research and found that other APIs on the market also use string (e.g., Paypal and Amazon). Others use cents encoded as integers (e.g., Klarna, Ayden). Using number to represent money amounts seems to be not very popular. At least, I've found no API using this format.

BirgitBader commented 1 year ago

Mar 2, 2023 by @jensfischer1515

https://developer.epages.com/beyond-docs/#show_cart_details uses Number for amount (and I was involved designing it)

AntjeGruenewald4789 commented 1 year ago

Many APIs on OTTO Market currently use number as type of amount. Receipts-V3 API, Products-V3 API, Orders-V4 API,...

BastianSperrhacke-Otto commented 1 year ago

@BirgitBader @AntjeGruenewald4789 "Number" with which precision?

AntjeGruenewald4789 commented 1 year ago

in all APIs teams work with two digits, like

The description is of different quality, but all teams use a MonteataryAmount object In Productcts-V3 you find the following description

In Receipts-V3

in Orders-V4

If you ask me, amounts in combination with currencies were this precision is not good enough, I would not expect in midterm plans for OTTO market. I would prefer to get a number for sure. But it's only my opion.

cfranzen commented 1 year ago

@AntjeGruenewald4789 as mentioned above: using number type for the amount definitely is a good option from a pure API perspective, however, it makes implementation harder for many popular frameworks on the client side. These frameworks typically convert a number type in JSON to a floating-point precision number in the client language, which is a really bad idea for storing amounts of money. Hence, all these clients would be forced to work around this issue.

Furthermore, I don't understand why it has any value to limit the decimal places to 2 digits, which is definitely not enough for some currencies. What is the purpose of that limitation? Just allow a sufficiently larger number of decimal places to be able to handle all currencies that might come in the future.

AntjeGruenewald4789 commented 1 year ago

@cfranzen I can only say something for the interface of our team the Receipts API. We do not have a hard limit to 2 digit, but in our context customer receipts (Rechnung und Erstattungsbelege) currently only for deliveries in Germany, it's does not make sense to provide more than 2 digit, as you can't pay less than a cent. And even if this is a technical solution, we should not show a higher precision here than on corresponding pdf documents.

If there is really a decision for OTTO market to show in future all amounts of our prices as strings, we could care about it in newer versions, so we and all partner using the current interface do not have extra effort. Who has the competence to make such a decision for the whole OTTO market? CoP API? Currently I only see this discussion as a kind of preparation for such a decision. If a decision is taken, I would currently expect to find it for APIs of OTTO market maybe in developer guide for OTTO market APIs, so it's really visible for all teams.

cfranzen commented 1 year ago

@AntjeGruenewald4789 I think if you only show the amount of money as a result it is completely fine to show it only with two digits, since that is how the currency EURO is designed. You would only run into problems when using a different currency as the result, e.g. Dinar. But such foreign currencies might not be a realistic problem for now, I would more probably see that Otto is going to support an "international" currency like Bitcoin. No idea how realistic that is, however.

Besides that, I see this discussion as a "Guideline" containing a suggestion. There is in my opinion no intent to force teams to comply to these discussion or even change existing APIs.

If we agree on above proposal, it will be visible at https://api.otto.de/portal/guidelines. Do you have some better place in mind?

AntjeGruenewald4789 commented 1 year ago

@cfranzen How about adding a link to the project here: https://internal.api.otto.market/docs#section/API-Guidelines.

cfranzen commented 1 year ago

@cfranzen How about adding a link to the project here: https://internal.api.otto.market/docs#section/API-Guidelines.

Sounds reasonable to me

otto-de / api-guidelines