search

camaraproject / CarrierBillingCheckOut

Repository to describe, develop, document and test the Carrier Billing Check Out API family
Apache License 2.0
10 stars 9 forks source link

Default values for page and perPage query params in `retrievePayments` endpoint #109

Closed PedroDiez closed 9 months ago

PedroDiez commented 1 year ago

Problem description Based on the feedback of our developers about API implementation, they have feedback that the abscense of default values for page and perPage query params in retrievePayments endpoint ( GET /payments) can lead to different implementations when no other query parameter (i.e. filter criteria) is indicated.

Possible evolution

As per their feedback in order to have an aligment regarding this scenario, they comment it can be settled for default values: page: 1 perPage: 10

Then, when no explicit indication and no other query parameters indicated, API can return the 10 latest payments which is a good UX approach and also can have a good perfomance (API response time)

Another consideration is to set a maximum value for perPage (they have commneted to have a limit of 100, in order to avoid long response times)

Additional context

Maybe these considerations could be also commented in Commonalities to set-up this reference values across any API that can have pagination in future

PedroDiez commented 11 months ago

@bigludo7 I agree besides indicating default values for page, perPage if not explicitly included independently have an exception to manage uncoherent situations from API Server side. Based on your comment in the last meeting

Something like:

400 "CARRIER_BILLING.INVALID_QUERY_PARAMS_COMBINATION" message: "Invalid query params combination provided in the request"

bigludo7 commented 11 months ago

@PedroDiez My point was more about handling the case when too many matching records found. In this case we should send something like this

400 "CARRIER_BILLING.TOO_MANY_MATCHING_RECORDS" message: "too many matching records found. Specify additional criteria to limit the number of records."

WDYT?

PedroDiez commented 10 months ago

Hi @bigludo7,

Then I think we can move forward with this in retrievePayments operation:

Setting default values for page, PerPage:

image

Adding and exception about handling the case when too many matching records found. (I mean in a wide sense, BE logic may expect a more suitable combination of query params)

400 "CARRIER_BILLING.TOO_MANY_MATCHING_RECORDS" message: "too many matching records found. Specify additional/suitable criteria to limit the number of records."

bigludo7 commented 10 months ago

Good for me !