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

Fix paypal adaptive transactions fx rate #8216

Merged
merged 2 commits into from
Dec 1, 2022

Conversation

Betree
Copy link
Member

@Betree Betree commented Nov 24, 2022

Fix opencollective/opencollective#4021

All cases left in DB where PayPal expenses paid manually.

I believe I remember a bug where we would use USD as the default currency somewhere in the "Mark as paid" process, which probably messed up most of these expenses. I was not able to find the relevant issue/PR.

There are a few (mostly, opencollective inc) where I can't really explain what happens, the FX rate was wrong even though we were only using USD.

This PR:

  • Fixes the abovementioned issue by considering that inputted amounts were using the source currency. We're therefore just re-computing them with FX rate set to 1.
  • Prevents paying PayPal expenses with adaptive when host.currency !== collective.currency. We know this can cause problems, and since we're Deprecate/Remove PayPal adaptive opencollective#5923 it doesn't seem worth working on a fix.
  • Adds a transaction.data.isManual flag on manually paid expenses, as it would have simplified the investigation to have an easy flag to surface that these expenses were paid manually

Changelog

When run, the script will impact the following:

Transaction group b6f982d2-7f6b-41a1-80bf-0472e116fa31 (host=kendraio-org, collective=kendraio)
Transaction group d0c995fd-9c80-4a79-984c-7b1f9459e031 (host=flarum-foundation, collective=flarum)
Transaction group 12fc34fc-5345-485b-810f-6ea9c4eac7f5 (host=flarum-foundation, collective=flarum)
Transaction group ffd45053-1fa9-4f6b-b75b-a53585124d6c (host=europe, collective=sunbeam-city)
Transaction group ea33817d-a1ac-4138-af46-8979194bf22b (host=allforclimate, collective=xr-belgium)
Transaction group 8aa7e13a-6dba-4333-9a8c-bd11e0cc535c (host=europe, collective=sunbeam-city)
Transaction group 0c562334-3ec4-47b0-87e1-a1c358f0dab5 (host=europe, collective=tealwiki)
Transaction group e7102964-fc96-4b1c-9ff9-7caa6583f3e5 (host=opencollective, collective=opencollective)
Transaction group 48cb4241-1015-4737-bf87-df57e8caba46 (host=opencollective, collective=opencollective)
Transaction group cde123ed-b898-4c6f-9b78-73da7742b035 (host=opencollective, collective=opencollective)
Transaction group 79623c31-fdc8-4c79-a097-51149297a3d0 (host=opencollective, collective=opencollective)
Transaction group b788acf2-23e4-4cad-b97e-58d63383da4a (host=opencollective, collective=opencollective)
Transaction group d998e12b-a1de-421c-8f79-9cc55802d393 (host=opencollective, collective=opencollective)
Transaction group 064d5388-c3ce-4bf2-b88a-432fd49a6948 (host=opencollective, collective=opencollective)
Transaction group 50152e15-e30d-436c-9b86-36c69a5c7a82 (host=opencollective, collective=opencollective)
Transaction group a23787c7-bff8-4c86-aba7-d21d84d2301b (host=opencollective, collective=opencollective)
Transaction group b0fe6963-86a1-44e9-8563-1bdc1fa6d7cc (host=opencollective, collective=opencollective)
Transaction group 342e32ce-016a-4e10-ade2-b84acfdb2bf9 (host=opencollective, collective=opencollective)
Transaction group 6823b948-83bc-4245-ba74-857e04d96fee (host=opencollective, collective=opencollective)
Transaction group 8ec96c9c-35bd-460c-b1cb-60e54ae43d27 (host=opencollective, collective=opencollective)
Transaction group 63da9eb1-973b-4e26-a6a8-f119e4763dc4 (host=opencollective, collective=opencollective)
Transaction group 9afdfd4c-f1f7-4e71-b693-23825a2588bc (host=opencollective, collective=opencollective)
Transaction group 61729d7d-e73f-434d-9827-4cd0123cb7e2 (host=opencollective, collective=opencollective)
Transaction group bc7a6510-f40f-44b8-b640-4081b3e1b92e (host=opencollective, collective=opencollective)
Transaction group fee6ed49-f706-446b-98f9-be598dc42d43 (host=opencollective, collective=opencollective)
Transaction group fe4b1348-9311-4bea-8281-6bd7b690e7b9 (host=opencollective, collective=opencollective)
Transaction group 3e1a9a05-8d20-4103-bed4-d2a4791ccab4 (host=opencollective, collective=opencollective)
Transaction group 4a1d7667-5b7c-4d6f-a524-323cddadaf1d (host=opencollective, collective=opencollective)
Transaction group ac1c8682-3f33-4cfa-8d4e-7d5e1018e0ec (host=opencollective, collective=opencollective)
Transaction group 911088ce-72f7-4c0e-ab00-a8239469673f (host=opencollective, collective=opencollective)
Transaction group 19808cbc-3494-4890-9975-c340f4505421 (host=opencollective, collective=opencollective)
Transaction group b438e294-f4f3-4030-bcb1-9429d122cfde (host=opencollective, collective=opencollective)
Transaction group b919e3a2-4df7-45c7-bf86-4c415c47c8b8 (host=opencollective, collective=opencollective)
Transaction group 2512a8cc-a4f4-4dc9-a2ae-5efa63bfd090 (host=europe, collective=huneeds)

@Betree Betree self-assigned this Nov 24, 2022
@Betree Betree requested a review from znarf November 24, 2022 16:20
@Betree Betree marked this pull request as ready for review November 25, 2022 09:48
@Betree Betree force-pushed the fix/paypal-adaptive-transactions-fx-rate branch from dc0c4b0 to ede5860 Compare November 25, 2022 10:34
// Validate
try {
await models.Transaction.validate(debit, { validateOppositeTransaction: false });
await models.Transaction.validate(credit, { validateOppositeTransaction: false });
Copy link
Member

Choose a reason for hiding this comment

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

Nice

@@ -1876,6 +1876,12 @@ export async function payExpense(req: express.Request, args: Record<string, unkn
try {
// Pay expense based on chosen payout method
if (payoutMethodType === PayoutMethodTypes.PAYPAL) {
if (expense.collective.currency !== host.currency) {
Copy link
Member

Choose a reason for hiding this comment

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

Ok!

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.

Looks good to me!

@Betree
Copy link
Member Author

Betree commented Dec 1, 2022

Script ran successfully, deploying the fix to prod

@Betree Betree merged commit 90b4300 into main Dec 1, 2022
@Betree Betree deleted the fix/paypal-adaptive-transactions-fx-rate branch December 1, 2022 08:12
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.

Incorrect hostCurrencyFxRate and amountInHostCurrency with some PayPal expenses
2 participants