camaraproject / CarrierBillingCheckOut

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

Aligment in "transactionOperationStatus" transitions and potential need of new Exception PAYMENT_DENIED #110

Closed PedroDiez closed 6 months ago

PedroDiez commented 9 months ago

Problem description

It has been detected that in 2-STEP Payment there may be conditions where a given payment can be denied, e.g. User phone_number is blocked for any reason in the OB.

To deal with scenarios an exception is needed. The proposal is to be HTTP 403 CARRIER_BILLING.PAYMENT_DENIED

Related to this, think is important we can have alignment in the understanding of "transactionOperationStatus" so maybe other impacts are required and take advantage of this to reflect appropiate documentation in the context of Issue#106

Then:

1-STEP PAYMENT

2-STEP PAYMENT

cc @bigludo7

Possible evolution

Once aligned behaviour. 1) Document this "transactionOperationStatus" information associated to the track of Improving Documentation Issue#106 2) Consider new event type "PAYMENT_DENIED" (aligned with Commonalities Notifications Design) 3) Add new exception 403 "CARRIER_BILLING.PAYMENT_DENIED" in several methods:

Additional context N/A

bigludo7 commented 9 months ago

Hello @PedroDiez

  1. yes sure. We have crafted the lifecycle here: https://github.com/camaraproject/CarrierBillingCheckOut/blob/main/documentation/API_documentation/resources/Carrier_Billing_State_Engine.JPG and perhaps this one needs to be reworked.

  2. I guess we need to think the event type defined for this API. Do we want to have one generic event for all transactionOperationStatus change or do we want to introduce specific event type per status? We can also have both.

  3. For the new exception, to be sure to understand, you want it for POST/payments, POST/payments/prepare & POST/Payments/{paymentId}/confirm correct? I tend to think that payment denied should exist as REST resource and query-able. I make a distinction between payment request failing to pass 'surface' control, from the one which are good from a technical perspective but fail business check and get denied. The former are not created as REST resource while the later should be. But if we doing that I'm not sure about proposed 403 usage. Another topic for thinking: when the payment is denied, we probably needs to indicate why.

Happy to continue the discussion...

PedroDiez commented 8 months ago

@PedroDiez comment here the transitions in 1-STEP, 2-STEP before taking API Spec decisions

PedroDiez commented 8 months ago

1. Regarding Transactions (also considering the topic indicated in Issue#116).

For the analysis here, "waiting for validation" will be named as "pending_validation" Will draft image tomorrow (within Issue#116)

1-STEP

2-STEP (validation is optional depending Business Scenario)

FIRST STEP

VALIDATE (OPTIONAL)

SECOND STEP

PedroDiez commented 8 months ago
  1. In the current PR (we can follow that scheme so far in that PR, until output of this iteration which will be a second PR), we have different events, but basically the same model for all of them.
PedroDiez commented 8 months ago
  1. Mainly based on Point 1. Proposal would be:

With regards to the use of HTTP 403 is to remark a scenario where API consumer have right Access Token to invoke procedure, but there is a situation where the user is not allowed to perform the payment so it is forbidden.

PedroDiez commented 8 months ago

Hi @bigludo7 analysis completed

PedroDiez commented 7 months ago

Commented during meeting 21/NOV and aligned to be considered in the next PR.

Also agreed to add "deny_reason" optional field for "denied" event model.