Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Rewrite in Typescript? #16

Closed
brijmcq opened this issue Apr 19, 2020 · 13 comments
Closed

Suggestion: Rewrite in Typescript? #16

brijmcq opened this issue Apr 19, 2020 · 13 comments

Comments

@brijmcq
Copy link

brijmcq commented Apr 19, 2020

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

@rizdaprasetya
Copy link
Collaborator

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:

  • The scale of this package is less than medium-big (medium-big project is where TS shines)
  • Keeping dependency slim is good for size & security
  • We want to avoid introducing too much complexity, for easier contribution from people without TS background
  • Easier/less maintenance (for this scale)
  • The nature of JSON param & response is dynamic, which might not well suited the TS static-typing paradigm

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
Copy link

dimaqq commented Apr 22, 2020

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

@brijmcq
Copy link
Author

brijmcq commented Apr 22, 2020

@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
Copy link
Author

brijmcq commented Apr 22, 2020

@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

@brijmcq brijmcq closed this as completed Apr 22, 2020
@dimaqq
Copy link

dimaqq commented Apr 22, 2020

@brijmcq consider definitelytyped instead

@brijmcq
Copy link
Author

brijmcq commented Apr 22, 2020

@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
Copy link
Collaborator

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
Copy link
Author

brijmcq commented Apr 22, 2020

@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
Copy link
Collaborator

rizdaprasetya commented Apr 23, 2020

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:

  • transaction_status
  • fraud_status
  • payment_type

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
Copy link
Author

brijmcq commented Apr 23, 2020

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
Copy link
Collaborator

@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
Copy link
Author

brijmcq commented Apr 25, 2020

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants