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

List supported currencies in API under api/currencies #961

Merged

Conversation

petermaksymo
Copy link
Contributor

For #847

Returns a list of supported currencies under /api/currencies.

This is my first PR for this project, any feedback is greatly appreciated :)

@almet
Copy link
Member

almet commented Dec 5, 2021

Hi, thanks for opening a pull request here. It's appreciated :-) I think adding a test for this would be helpful in order to test that it's behaving the way we think it is.

Let me know if you need help on how to do this properly.

@petermaksymo
Copy link
Contributor Author

Thanks for the feedback, I'm happy to add some tests.

Just to confirm before I do it, I'm thinking of putting them at the start of this test_currencies and add asserts for a good status and check that XXX is within the list. Anything else I should look for?

@almet
Copy link
Member

almet commented Dec 5, 2021

Please, add them in the api tests instead, it's better to keep all the API-related tests together.

@zorun zorun mentioned this pull request Dec 9, 2021
@almet
Copy link
Member

almet commented Dec 10, 2021

Works for me, thanks! We can merge as soon as the build is green :-)

@almet almet merged commit 470c19f into spiral-project:master Dec 13, 2021
@almet
Copy link
Member

almet commented Dec 13, 2021

Thanks :-)

TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
…#961)

* List supported currencies in API under api/currencies

* Added test for /currencies route
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