TreyWW / MyFinances

MyFinances is a web application that can help you as an individual, or team, manage your finances!
https://docs.myfinances.cloud
GNU Affero General Public License v3.0
100 stars 144 forks source link

Replacing forexAPI with currency converter (#410) #435

Open fos9191 opened 3 months ago

fos9191 commented 3 months ago

Description

Removed forexAPI and replaced it with Currency Converter, an opensource python library (https://github.com/alexprengere/currencyconverter). @TreyWW I saw that you mentioned just using an API would be better than introducing another python dependency but I wasn't sure how to embed API keys in the project without hardcoding them...? I can work on this in the future if you think just using an API would be better. For now at least, the conversion functionality should be working.

Checklist

What type of PR is this?

Added/updated tests?

TreyWW commented 3 months ago

Hey @fos9191, welcome to the project and thank you for the PR! The main reason I was kind of inactive for the issue was just because I thought maybe it's worth removing the feature entirely. It doesn't fit in with our feature set at the moment and is pointless being worked on - even more so if we are adding an extra dependency that needs trusting and updating to get new rates.

Like you say; the idea of an API would be better, at least no dependency and up to date rates all the time, an API Key could be added to the .env file with instructions somewhere on the documentation.

I'll think about whether merging this PR at the moment is worth it, apologies if this gets inactive.

fos9191 commented 3 months ago

Yeah that makes sense and I understand if you don't want to merge.

If its a case of wanting to keep the feature but only with an API being used, let me know and I can work on that.

TreyWW commented 1 month ago

Hey @fos9191, apologies for the delay. I'd like to remove the feature from the UI, but like you mentioned, it may still be handy to have this somewhere in our backend.

If you're up for it still, you may add an API (via our public system), aswell as a service layer, which allows for our normal backend code to call these functions, incase we want to use it internally too.

I'll be merging #462, however if you would like to re-make this, with the new library, and make it available in them places do feel free to. If not that's absolutely fine, we can close the PR.

Sorry about the wasted time, I'll make a greater effort to list features that are no longer in use or are soon to be removed

fos9191 commented 3 days ago

Hey @TreyWW, sorry for taking so long to reply to this.

I'm probably not going to work on this in the near future so feel free to close the PR.

TreyWW commented 3 days ago

Hey @fos9191 no probs! This isn't a priori so it's okay. Up to you :)