stats4sd / aec_portfolio

A proof of concept for the AEC Consortium Project Management / Assessment System
GNU General Public License v3.0
0 stars 0 forks source link

Add Currency conversion #103

Closed dave-mills closed 1 year ago

dave-mills commented 1 year ago

Fixes #77

This PR adds the extra features required to let users enter initiative budgets in any currency, and add an exchange rate to let the platform convert the budget into a single per-organisation currency.

Notes on Requirements

Implimentation

Retrieving data from the API

The platform is linked up to the service at https://freecurrencyapi.com/. Instead of making requests each time to the API, the platform has a set of commands:

This way, we can get all the needed information with the same number of API calls, regardless of how large the user base grows.

Storing the data

The platform now includes data models and tables for currencies and exchange_rates.

Creating an Initiative

The user is asked for the initiative currency, and then has the option of automatically retrieving the exchange rate:

CleanShot 2023-06-30 at 16 19 54

If the user has entered a start_date and that date is in the past, that date is used to retrieve the exchange rate. Otherwise, the latest exchange rate is used.

Limitations of this approach and possible future upgrades

1. Only 32 of the most common currencies are supported by the free API.

2. The Free API does not give "today's" rates. Only yesterday's.

3. No 'single' currency

Right now, the budgets are converted to individual institution's currencies. This is fine for the institution level. Eventually, the goal is to get an overview of funding flows towards Agroecology across the entire system, which means converting the budgets of all inititives into a common currency (likely EUR).

This is probably an enhancement to add in a future version of the platform.

dave-mills commented 1 year ago

To facilitate testing, I have added:

One possible issue with this approach is that we will end up with: (32 32 365) = ~400,000 records per year. I assume the database will cope with this, but it's worth testing that.

I am going to run the full set of API Jobs locally over night to review this.

dan-tang-ssd commented 1 year ago

When creating a new initiative, maybe we should ask user for project start date and end date first before asking about currency and budget. So that exchange rate can be retrieved for a specified project start date.

image

dave-mills commented 1 year ago

When creating a new initiative, maybe we should ask user for project start date and end date first before asking about currency and budget. So that exchange rate can be retrieved for a specified project start date.

Already fixed in #104 - merged in because it affected the recording of the training videos.

dan-tang-ssd commented 1 year ago

For exchange rate API, do we have new item to add in .env file? I cannot run the command for getting exchange rate history...

image

dave-mills commented 1 year ago

Yes. I forgot to add it into the .env.example file. But, if you read the changed code, you'll find reference to:

config('services.currency.api-key'), which leads to =>

'currency' => [
        'api-key' => env('CURRENCY_API_KEY'),
    ],

in the services.php config file ;)

dave-mills commented 1 year ago

I've also put the API key in 1Password: (Freecurrency API in the Stats4SD IT vault) https://share.1password.com/s#gHoMb19Smkta9s6yKWO6GK_8yaCFkKZdudZXKIxGZHw

dan-tang-ssd commented 1 year ago

I added new item for free currency API key in .env.example file.

I also updated .env with API key stored in 1Password, it works fine now.

When we have more and more exchange_rates record, we may need to add index for base_currency_id, target_currency_id and date to avoid possible performance issue.

image

dan-tang-ssd commented 1 year ago

I performed testing in front end, by creating a project with budget HKD 1. Suppose we should have EUR 0.11xxx, But the converted EUR amount is incorrect. It looks like the FROM currency and TO currency after interchanged...

image

image

image

dave-mills commented 1 year ago

When we have more and more exchange_rates record, we may need to add index for base_currency_id, target_currency_id and date to avoid possible performance issue.

Indeed - see database/migrations/2023_06_28_133458_add_indexes_to_exchange_rates.php.

It looks like the FROM currency and TO currency after interchanged...

Hmm, yes. Fixed in the latest commit, so now the initiative currency is the "base" and the institution currency is the "target".

dan-tang-ssd commented 1 year ago

It works fine now. I think it is good to deploy.