Add support for Euros

[duncte123]

Contributing to the Donation Tracker

First of all, thank you for taking the time to contribute!

Please fill out the template below and check the following boxes:

• I've added tests or modified existing tests for the change.
• I've humanly end-to-end tested the change by running an instance of the tracker.

Issue from Pivotal Tracker

N/A

Description of the Change

Some events in Europe raise money in Euros instead of USD. This change adds support for Euros to the tracker.

Verification Process

I set up a local instance of the tracker and tested donations in Euros and USD, both work as intended.

edit: This branch has been used for BSG2023 during an event and works like a charm, all pages that display currency show the correct one.

[faultyserver]

This is a great change to make! There are a few implications elsewhere in the codebase, but this gets far enough along that events in other currencies would at least be able to accept and display donations in those currencies in a reasonable way.

There are a few other pages that would need to be updated as well (For example, even the new processing pages), but I think those can happen in a followup PR, as the work needed to support those is rather different than what is necessary here.

[duncte123]

@faultyserver Thank you for the amazing feedback!

I'll have a look to see if I can also implement the reader pages with this PR

[duncte123]

The "Tracker Package" pipeline seems to complain about missing permissions. I assume that's because I'm an outside collaborator?

[faultyserver]

The "Tracker Page" pipeline seems to complain about missing permissions. I assume that's because I'm an outside collaborator?

Hmm. I'm not sure why, since the pipeline runs inside of the GDQ github organization and uses a single set of permissions, but it might be something on Azure's end noticing that the branch is coming from a fork or something. I'll look into it eventually, but it won't be blocking merges. Thanks for the heads up about it!

[duncte123]

In theory the processing code works, it's the only part that I haven't been able to verify in my browser since I don't really understand how pending bids exactly work if you create them manually. Other than that, the admin pages work as expected

[duncte123]

Did a lot of manual testing and everything seems to work as it should. I double checked that with people that have worked a lot with the tracker.

[duncte123]

There currently is one bug that is on the prizes UI that I found. If you click a prize from the donate screen the currency is transported fine is it's still stored in the store. But if you refresh the page, the currency is no longer provided in the props and so it falls back to USD.
This is only for the react version of the UI, the other UI that is rendered with a table uses the locale system to render the currency symbol so that is unaffected by the settings in the first place.

This has now been fixed

[uraniumanchor]

Hi, any chance you can rebase this based off current master? I finally got around to adding a check in Azure that won't try and package PRs from forks (which doesn't work anyway), so this should theoretically pass CI and I can give it a proper review. Thanks.

[duncte123]

I've merged master into this branch to make it up-to-date again.
Also while I'm at it, what is your opinion on adding more than just the euro currency? EG the British pounds
Sorting this list by ISO code shows how widely used the currencies are https://en.wikipedia.org/wiki/List_of_circulating_currencies

[uraniumanchor]

In a perfect world, any further work regarding currency would just extend it to all ISO currencies, at least the ones that Intl.NumberFormat supports: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#currency_2

If this can be done in a standardized way, ideally.

I'll take a closer look at this today, though it looks like Jon already had plenty to say about it. Sorry this took so long to get back around to.

[uraniumanchor]

Unfortunately the very first thing I did after switching to this branch was noticing that the custom money filter does not seem to be aware of Euros, so it still displays the dollar sign:

https://github.com/BSGmarathon/donation-tracker/blob/e8856f055a3bfbf84c54257e8170de697d3282eb/tracker/templatetags/donation_tags.py#L172
https://github.com/BSGmarathon/donation-tracker/blob/e8856f055a3bfbf84c54257e8170de697d3282eb/tracker/templates/tracker/index.html#L22

I'm not actually sure if there's an elegant way of being able to avoid passing this information to every place where this filter is invoked, but it also made me realize there's another wrinkle that already existed: Any page that displays the aggregate of -all- events is going to get the math wrong if there's more than one currency involved. In practice I'm not sure how much of a problem that is, and it shouldn't necessarily block this PR, but the issue of the money filter still needs to be resolved.

Edit: Might be better to change the money filter to a tag since that can handle more complicated parsing. If you don't want to tackle this yourself I can probably work on it next week.

[duncte123]

I'll see what I can do to alter the money filter, a quick look reveals that it currently takes the system locale to get the currency symbol hence it displaying as euros for me. This is likely why I missed it.

My idea is to allow the filter to take in a nullable "event" parameter and take the paypalcurrency from there so it can fall back to system locale on the front-page, but that indeed does not solve the aggregate issue.
If you're ok with this solution for now I'll happily adapt the filter. I will happily leave the tag conversion to you as it seems that you have a decent idea on how to do this already in regards to conversion rates etc.

[uraniumanchor]

If that's the case (system locale) I can probably accept it as is and modify the tag myself later, thanks. Especially since once I looked at what the current code is doing it is in need a rewrite anyhow (it's messing with global state and that's bad).

I'll try and get this another look on Tuesday.

[duncte123]

@uraniumanchor Did you manage to have a look at the PR?

I tried to figure out why the migration need to be merged but locally did not show such conflicts