Midtrans / midtrans-nodejs-client

Official Midtrans Payment API Client for Node JS | https://midtrans.com
MIT License
180 stars 57 forks source link

Suggestion: Rewrite in Typescript? #16

Closed brijmcq closed 4 years ago

brijmcq commented 4 years ago

Thanks for this awesome SDK. Are there any plans to rewrite this in Typescript? I would love to help

rizdaprasetya commented 4 years ago

Hi @brijmcq thanks for the idea & suggestion.

While we absolutely agree Typescript is nice and have many benefits, we did review the possibility of that some time ago, but we decided it is not yet good fit for this package due to:

However, here's the good news: You are very much welcomed to fork and/or rewrite this package in TS. Let us know if you do, so we can appreciate your open source contribution by listing it on our site & docs and credits you as the author.

Thanks!

dimaqq commented 4 years ago

https://twitter.com/_developit/status/1251176832424181762

brijmcq commented 4 years ago

@rizdaprasetya thanks for the reply. I have a client that wants to use your platform and I was kinda late to see that this library existed so I basically recreate some of the functions you already have.

I will share it once we started using it in production

brijmcq commented 4 years ago

@dimaqq I'm not asking for the maintainer to rewrite it in TS, I'm offering help just in case there's an effort already on going to do it. Of course, if I really want it I'll just fork the project and do it on my own :D

dimaqq commented 4 years ago

@brijmcq consider definitelytyped instead

brijmcq commented 4 years ago

@dimaqq that could work. But I also want to rewrite some of the code to make it easier to set it up ( might not be practical given the small number of users )

Take a look at the example code for notification

apiClient.transaction.notification(mockNotificationJson)
    .then((statusResponse)=>{
        let orderId = statusResponse.order_id;
        let transactionStatus = statusResponse.transaction_status;
        let fraudStatus = statusResponse.fraud_status;

        console.log(`Transaction notification received. Order ID: ${orderId}. Transaction status: ${transactionStatus}. Fraud status: ${fraudStatus}`);

        // Sample transactionStatus handling logic

        if (transactionStatus == 'capture'){
            if (fraudStatus == 'challenge'){
                // TODO set transaction status on your databaase to 'challenge'
            } else if (fraudStatus == 'accept'){
                // TODO set transaction status on your databaase to 'success'
            }
        } else if (transactionStatus == 'cancel' ||
          transactionStatus == 'deny' ||
          transactionStatus == 'expire'){
          // TODO set transaction status on your databaase to 'failure'
        } else if (transactionStatus == 'pending'){
          // TODO set transaction status on your databaase to 'pending' / waiting payment
        }
    });

This notification is handling transactions for credit card users, but other payment channels are not yet considered here. Imagine adding more code to handle other payment channels.

If possible, it would be better to have a smooth developer experience where they could integrate your platform in less than 5 or 10 minutes in a custom backend

A developer would just provide their code on how to handle these events.

Maybe something like this:

Where the 2nd parameter will handle the necessary events

apiClient.transaction.notification(mockNotificationJson, { 
  onTransactionChallenge,
  onTransactionDeny,
  // more code
})

Or handle it by payment type since they have different return data. For instance, transanction_status for GoPay are "pending, settlement, expire and deny" while for credit card are "capture, deny and authorize"

apiClient.transaction.notification(mockNotificationJson, { 
  creditCard: foo,
  gopay: bar
})

Your platform reminds me of Stripe. I guess more devs will use this if there''s more real-world examples. That's just my two cents

rizdaprasetya commented 4 years ago

Hehe, no worries guys, we were expecting this question will shows up. It's open source world, so people can (and will have) preferences. At least now we can use this issue as reference if similar question re-occur.

@brijmcq Good to hear you are already writing similar functionality in TS, would be good options for those prefer TS. Feel free to publish to NPM, and (if you wish) let us know if you made one, we'll list it on our docs so people can choose their preference.

That's some interesting idea with callback/event-based function. We initially kept the sample generic & broad, to allow API users to implement any custom logic they might want to apply, instead of trying-to-guess (or limit) their use case (And according to our experience, API users implementation use-case vary a lot, to the extend that we can never imagine 😂) But yes once we have the chance, might be good time to explore further specific samples 👍

brijmcq commented 4 years ago

@rizdaprasetya just curious what are those use cases?

In my experience, I guess the hardest part is the notification. Sending transaction is very easy, but handling the notification is hard since there are a lot of variables that could affect the payment and many different scenarios.

rizdaprasetya commented 4 years ago

Well I can't exactly describe the unusual ones. But even with usual scenarios, there are many variations.

that is largely due to these variable:

So if one want to create callback/event-based function, he should consider based on which: one of those? two of those? or all of those? Which then will impact the "permutation" number of callbacks, eg: onCapture vs onCaptureAccept vs onCardCaptureAccept, then multiply that with BankTransfer, then Gopay, etc.

So that what's majorly different with Stripe, in Indonesia the payment channels is more fragmented and each payment channels have its own limitation/behaviour (e.g: only card have capture status, other debit based channels do not. Each debit based channels have its own default expiry-time set by the payment provider. Different refund policy. etc.)

brijmcq commented 4 years ago

I definitely agree that those 3 variables made it very hard to integrate it with a custom backend. That's also the source of my pain points now.

Right now, we're planning to enable selected payment type first ( gopay and bank transfer ). I'm still thinking what would be the best way to handle this complexity that's why I'm asking for edge cases :D Basically, we're doing a divide and conquer here because just like what you've said, there's a lot of combinations here that we may need to handle.

I guess implementing all the payment type at once is not practical and would take a lot of time instead of focusing on the other features of our app.

If I have more time, I'll try to create a tutorial about this

rizdaprasetya commented 4 years ago

@brijmcq yes, nice 👍

I can give some insight with Gopay & BankTransfer. It is quite straight forward overall for both, there's only these major status: pending, then can become settlement or expire (further details here). The tricky things probably both have different default expiry-time. Gopay short lived 15 mins. So your edge case can be exipry-time related. Which can be overriden with expiry JSON param during transaction creation.

Other thing: you can re-fetch VA number via /status API on banktransfer, but not on Gopay.

brijmcq commented 4 years ago

Thanks for the tip.

| Gopay short lived 15 mins. I didn't found this in the docs, could you point me where it is located?

rizdaprasetya commented 4 years ago

Oops sorry missed the notification for this reply. For now it's not actually documented, but it can be seen from this menu on Midtrans Dashboard: Dashboard > Settings > Snap Preference > System Settings it will shows all default expiry time.

Can also be overriden by expiry params on Snap JSON payload.