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

Added Euros and fixed exchange rate selection page #5566

Merged
merged 2 commits into from
Apr 13, 2021

Conversation

jmcameron
Copy link
Collaborator

Added support for Euros and added related currency and exchange rate tests.

WARNINGS

  • This PR includes migration code. When migrating production sites, you should probably update the default exchange rate for Euros to be a more current value.

TESTING

  • Use bhima_test
  • Go to page: Administration / Exchange Rate
  • Notice the new entry for Euros
  • Click on the [Update Exchange Rate(s)] button to get the modal for changing the exchange rate
  • Select Euros from the drop-down currency selector
  • Notice that the current exchange rate is updated for Euros (just below the date field)
  • Try changing the value and submitting
  • Notice the new value is shown on the main page
  • Go back into the modal, select Euros again and verify that they new value is shown

NOTES

  • The process of adding a new currency requires multiple steps and should be documented in the Wiki.

@jmcameron jmcameron requested a review from jniles April 8, 2021 18:54
Copy link
Collaborator

@jniles jniles left a comment

Choose a reason for hiding this comment

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

This works surprisingly well. Well done!

The only thing that doesn't work that I could find is the handlebars rendering for euro. You'll find that in the file server/lib/template/helpers/finance.js. You need to add the configuration for Euro, inc,luding the currency symbol. The FC and USD are already defined there. It will make our receipts render euro when they are paid in euro instead of the default $.

Here is an example of the logic working, but not the currency rendering:
image

Copy link
Collaborator

@jniles jniles left a comment

Choose a reason for hiding this comment

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

Tested and works well. Thanks @jmcameron !

I made one comment about unifying the format of the euro on the client and server (handlebars). I think they both should use commas for decimals and a thin non-breaking whitespace for the thousands space.

client/src/i18n/currency/eur.json Outdated Show resolved Hide resolved
@jmcameron
Copy link
Collaborator Author

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 13, 2021

Merge conflict.

@jmcameron
Copy link
Collaborator Author

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 13, 2021

Merge conflict.

@jmcameron
Copy link
Collaborator Author

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 13, 2021

Build succeeded:

@bors bors bot merged commit 0674833 into Third-Culture-Software:master Apr 13, 2021
@jmcameron jmcameron deleted the add_euros branch April 13, 2021 20:30
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