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

Add invoice paid event callback struct #124

Merged
merged 3 commits into from
May 19, 2023

Conversation

rjjatson
Copy link

@rjjatson rjjatson commented May 17, 2023

Overview

#123

Changes

I am thinking to add callback event struct directly under xendit package with EventXXXXXXXXX format name. im refering a bit to how stripe place their event struct https://github.com/stripe/stripe-go/blob/master/event.go#L86.

I am referring to this documentation and e wallet invoice paid event payload i got on production to create the struct. i want this struct to be a common struct to handle invoice paid callback event for any payment methods.

let me know if there is anything i need to fix. i could help with the other events if this one looks OK to you.

Thank you !

Copy link
Contributor

@adityarx adityarx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjjatson thank you for your contribution! We really appreciate the time you've taken to contribute to our libraries. Our team has reviewed and left a few comments on the PR. Additionally, we may ask a developer from the Invoice team to also review the PR. Thanks!

event.go Outdated
@@ -0,0 +1,28 @@
package xendit

type EventInvoicePaid struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be renamed to InvoiceCallback to reflect what we define on the API Reference here? Thanks!

Could we also move this to the invoice folder over here since this logic is specific to Invoices?

Copy link
Author

@rjjatson rjjatson May 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good !
i renamed it to InvoiceCallback and moved it to invoice package.

event.go Outdated
MerchantName string `json:"merchant_name"`
PaymentMethod string `json:"payment_method"`
PaymentChannel string `json:"payment_channel"`
PaymentDetails InvoicePaymentDetail `json:"payment_details"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you declare this as a pointer since this field is optional? *InvoicePaymentDetail

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!
i also fixed it here 930783e

event_test.go Outdated
"github.com/xendit/xendit-go"
)

func TestEventInvoicePaid_EWallet(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help to rename this test to reflect the new callback struct, thank you!

Copy link
Author

@rjjatson rjjatson May 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks ! i renamed it to match the InvoiceCallback
i also add test for bank, retail outlet, and expired cases.
copy and pasted the test payload from the docs

invoice/event.go Outdated
import "github.com/xendit/xendit-go"

type InvoiceCallback struct {
ID string `json:"id"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for your work @rjjatson! We have one small request left for you. Our Invoice team asked if you can match this struct in terms of fields and types. They noted that some fields were missing in your initial commit. Many thanks!

type InvoiceCallback struct {
    ID                           string            `json:"id"`
	UserID                       string            `json:"user_id"`
	ExternalID                   string            `json:"external_id"`
	IsHigh                       bool              `json:"is_high"`
	Status                       string            `json:"status"`
	MerchantName                 string            `json:"merchant_name"`
	Amount                       float64           `json:"amount"`
	Created                      string            `json:"created"`
	Updated                      string            `json:"updated"`
	PayerEmail                   string            `json:"payer_email,omitempty"`
	Description                  string            `json:"description,omitempty"`
	PaymentID                    string            `json:"payment_id,omitempty"`
	PaidAmount                   float64           `json:"paid_amount,omitempty"`
	PaymentMethod                string            `json:"payment_method,omitempty"`
	BankCode                     string            `json:"bank_code,omitempty"`
	EwalletType                  string            `json:"ewallet_type,omitempty"`
	CallbackAuthenticationToken  string            `json:"callback_authentication_token,omitempty"`
	CallbackURL                  string            `json:"callback_url,omitempty"`
	BusinessID                   string            `json:"business_id,omitempty"`
	BusinessName                 string            `json:"business_name,omitempty"`
	OnDemandLink                 string            `json:"on_demand_link,omitempty"`
	RecurringPaymentID           string            `json:"recurring_payment_id,omitempty"`
	CreditCardChargeID           string            `json:"credit_card_charge_id,omitempty"`
	Currency                     string            `json:"currency,omitempty"`
	InitialCurrency              string            `json:"initial_currency,omitempty"`
	InitialAmount                float64           `json:"initial_amount,omitempty"`
	PaidAt                       *string           `json:"paid_at,omitempty"`
	MidLabel                     string            `json:"mid_label,omitempty"`
	PaymentChannel               string            `json:"payment_channel,omitempty"`
	PaymentDestination           string            `json:"payment_destination,omitempty"`
	SuccessRedirectURL           string            `json:"success_redirect_url,omitempty"`
	FailureRedirectURL           string            `json:"failure_redirect_url,omitempty"`
	CreditCardToken              string            `json:"credit_card_token,omitempty"`
	PaymentMethodID              string            `json:"payment_method_id,omitempty"`
	ShouldAuthenticateCreditCard bool              `json:"should_authenticate_credit_card,omitempty"`
	PaymentDetails               *PaymentDetail    `json:"payment_details,omitempty"`
	Fees                         *[]InvoiceFee            `json:"fees,omitempty"`
	Items                        *[]InvoiceItem           `json:"items,omitempty"`
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for reaching out to the team !
i have matched the struct with the one provided, re-run the tests and it passed.
3024ee5

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thanks! Checking with the team before merging and publishing a new package~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! thanks @adityarx let me know if there is anything else i need to add /fix !

@suyumin82
Copy link

pipeline seems to be failing, other than that all good for me!

@adityarx adityarx merged commit 0bb367e into xendit:master May 19, 2023
@adityarx
Copy link
Contributor

Merged and released! Thanks a lot for your contribution @rjjatson, we really appreciate it 🙏

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

Successfully merging this pull request may close these issues.

3 participants