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

VirtualCard: Support refund transactions #6149

Merged
merged 4 commits into from
Jun 14, 2021

Conversation

kewitz
Copy link
Contributor

@kewitz kewitz commented Jun 11, 2021

This covers the missing edge case of refunded virtual card charges.
Notice I'm only creating the transaction pair here, I don't think it makes sense to create an expense with a negative value.

@kewitz kewitz self-assigned this Jun 11, 2021
@kewitz kewitz requested a review from znarf June 11, 2021 17:09
privacyTransaction: Transaction,
opts?: { host?: any; collective?: any; hostCurrencyFxRate?: number },
): Promise<any> => {
): Promise<typeof models.Expense | undefined> => {
const amount = privacyTransaction.settled_amount;
Copy link
Member

Choose a reason for hiding this comment

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

What is our guarantee that settled_amount is in USD here. Is it really always the case? Even if the charge is in another currency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they don't even list currency on their schema.

Copy link
Member

@znarf znarf left a comment

Choose a reason for hiding this comment

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

I'm good with the change.

@Betree have a look so you have the context about this behavior.

Co-authored-by: François Hodierne <francois@hodierne.net>
@kewitz kewitz merged commit 07f961a into main Jun 14, 2021
@kewitz kewitz deleted the refact/privacy-refund-transaction branch June 14, 2021 18:28
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.

2 participants