-
Notifications
You must be signed in to change notification settings - Fork 38
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
Fixes noticed during user testing(part-1) #3450
Conversation
hypha/settings/base.py
Outdated
@@ -20,7 +20,8 @@ | |||
# Hypha custom settings | |||
|
|||
# Set the currency symbol to be used. | |||
CURRENCY_SYMBOL = env.str('CURRENCY_SYMBOL', '$') | |||
CURRENCY_CODE = env.str('CURRENCY_CODE', 'USD') | |||
LOCALE = env.str('LOCALE', 'en_US') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is only used for currency should we call it CURRENCY_LOCALE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think for other places we can use LANGUAGE_CODE only currency needs to be strict so I'll rename it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or can't we just use LANGUAGE_CODE for currency as well(as @theskumar suggests)? It is also a similar setting and fixed for an organization. Or it might create any differences/issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frjo @sandeepsajan0 please take a look at my comment here: #3448 (comment)
hypha/settings/base.py
Outdated
@@ -20,7 +20,8 @@ | |||
# Hypha custom settings | |||
|
|||
# Set the currency symbol to be used. | |||
CURRENCY_SYMBOL = env.str('CURRENCY_SYMBOL', '$') | |||
CURRENCY_CODE = env.str('CURRENCY_CODE', 'USD') | |||
LOCALE = env.str('LOCALE', 'en_US') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend picking this locale
value from the LANGUAGE_CODE
setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is logical but have some practical issues.
LANGUAGE_CODE
specifies what locales files to read translations from. So for no one has translated Hypha into another language as far as I know. What if they still want to pick another currency?
Regarding currency setup see my comment at #3448 (comment) |
@sandeepsajan0 Can you please rebase this PR? It got some merge conflicts after I merged in all the approved PRs to main. |
…it auto scrollable to comment
d052f02
to
1765435
Compare
@frjo I have rebased the PR, we can put it back on the test site. |
Refs: #3448