Display payment data in USD #390
Conversation
|
I have modeled this pretty much after HQ. I am completely open to remove this app and simply cache the exchange rates in redis and lookup them from cache for calculations. Or even any other simpler method as well. But this will let us use the Currency model wherever payment data is involved. Same can be used to populate choices for Opportunity.currency. |
sravfeyn
force-pushed
the
sr/currency
branch
from
10802c5
to
35ced03
Compare
|
|
||
|
|
||
| def create_task(apps, schema_editor): | ||
| schedule, _ = CrontabSchedule.objects.get_or_create( |
There was a problem hiding this comment.
I have followed the existing way of defining periodic tasks. But I am wondering why are we using the DatabaseScheduler instead of simply defining the schedules via the settings.CELERY_BEAT_SCHEDULE?
I think we want to do a one time conversion for the day of the visit. That way the numbers don't change for historical reporting, and they are much closer to the actual values paid out. I was imagining the conversion being stored on the payment itself rather than for the currency. |
| else: | ||
| exchange_rate = get_exchange_rate(currency, date_paid) | ||
| if not exchange_rate: | ||
| raise Exception(f"Invalid currency code {currency}") |
There was a problem hiding this comment.
May be ignore the error and just print instead?
There was a problem hiding this comment.
I think this is fine. Everything should be valid at this point, and we know what the currencies are. I think one opportunity has a typo in the currency, that we can fix before you deploy, but it would be good to have a hard failure to force us to do so.
| return [currency_code, date.toordinal() if date else None] | ||
|
|
||
|
|
||
| @quickcache(vary_on=_cache_key, timeout=60 * 60) |
There was a problem hiding this comment.
Since these are daily rates, we can probably cache for much longer
https://dimagi.atlassian.net/browse/CCCT-478
Note that the payment amounts (that occur on different dates) are summed and then converted to USD as of that day's exchange rate. It doesn't make much sense to sum them using each day's exchange rate without displaying the exchange rates for each of those dates to user.