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

Carrier Billing Payment API documentation + Yaml update #32

Closed bigludo7 closed 1 year ago

bigludo7 commented 1 year ago

Hello Please see a proposal for Carrier Billing Payment API documentation. I followed QoD API template. Source puml are provided for the illustration. A new version of the yaml is also provided.

Mohammed-nayeemuddin commented 1 year ago

Dear Robert,

I went through the Yaml and I have few queries, if you could please answer.

  1. /payments/confirm/{paymentId}

       We need to return 204 in success when there is no response but currently you are returning 200.
    
    2.  /payments/prepare   - paymentId - I don’t see in the response of 1 step or 2 step . Can you please highlight in Yaml
    
    3.  purchase notification callback – is it valid in case of 2 step or 1 step or both?
    1. I don’t see any resource for OTP in the yaml.
bigludo7 commented 1 year ago

Following @naymohammed.c@stc.com.sa review, Yaml updated:

bigludo7 commented 1 year ago

Hi Nayeemuddin

Thanks for your review – this is really helpful

1/ You’re perfectly right – 200 is not good. It should be 202 I think (202 (Accepted) code indicated that the request has been accepted to be processed, but it has not ended.) – as specified here: WorkingGroups/API-design-guidelines.md at main · camaraproject/WorkingGroups (github.com) --> fixed

2/ you’re right. My bad ! I need to add it --> fixed

3/ Yes in both case – for my perspective notification could be triggered for any payment status change

4/ For me this outside of the scope of this API as we work on a separate OTP API that could be used in this UC. I imagine in our different company we can have separate way to get user consent for this payment so I prefer to keep this outside the core function of payment.

PedroDiez commented 1 year ago

@bigludo7 it appears a conflict, maybe you can fix it and after that merge this PR. So additional PR can be created after to update Payment State engine picture

bigludo7 commented 1 year ago

@PedroDiez I have fixed 2 issues for the conflict. If still issue we can have a quick call tomorrow to fix it together?

PedroDiez commented 1 year ago

sure Ludovic!

PedroDiez commented 1 year ago

seems OK, you can merge this PR and later the one with updated picture