-
Notifications
You must be signed in to change notification settings - Fork 614
system-events: Add balance.{order,refund,refund_reversal,dispute} events #8596
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
8496730 to
8d76e4e
Compare
frankie567
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things there and there, but that's the right approach :)
On another level, I'm wondering if we should add tax_amount in the events metadata as well: merchants indeed only care about the raw amount, but I'm wondering if we might need the tax amount in the future. That said, it can be a rabbit hole as it'll also require to add tax_country/tax_state to fully make sense 🤔
Thank you for the good feedback!
That's a good question. If we think that it would be useful we might as well add it now I guess. I also realised that we do revert_refund transactions which we might want to have an event for. Either as a negative amount with the same refund event, or a new event. |
3e737d3 to
8c55571
Compare
I gotta feeling (woohooo) that it'll be useful in the future. So I would say let's add it (+tax country/state).
Yeah let's do |
8c55571 to
8551ea3
Compare
Perfect, I'll add it.
Should we do the same with the |
8551ea3 to
80b79b0
Compare
So, the trick is that we only write the transaction when the dispute is closed (won or lost). If it's won, we write a Dispute + Dispute Reversal. If it's lost, just a Dispute. EDIT: Actually, if we want to be clear about fees (see below), we probably need to have Now that I think of it, there is also the fees dimension 😅 In this implementation, we store the amounts before Polar fees. If we want to show graphs with "net" amounts, we probably also need to store |
82f4fbb to
14dc6ba
Compare
These can be used to build balance metrics in the correct currency
14dc6ba to
9246534
Compare
To be able to track the fees separately
6899d98 to
595cf43
Compare
|
I think this is good to go now! |
These can be used to build balance metrics in the correct currency