-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add income and spending insight cards to dashboard #668
Add income and spending insight cards to dashboard #668
Conversation
/claim #645 |
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.
The structure of this looks good to me, just added some notes regarding tests / exchange rate conversions for handling multi-currency.
I think this is easier to read and understand whats happening at a glance
b622d36
to
d3b4d55
Compare
I've had a bit of a refactor and added a test in a similar fashion to the other account snapshot. Hopefully this is a bit more manageable now without straying too far from Active Record conventions. Here's my copy of the test cases, updated to include spending and income on a rolling basis: |
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.
Looks great, thanks for adding those test cases! I have copy/pasted the expected results into that main spreadsheet for now and we'll eventually get all those into an easier format to collaborate on.
Ah, that's an interesting solution. I was starting to get a hang of the models, the associations between them, and the TimeSeries, but even if I got a few more days, I doubt I would've completed the feature. I guess I'll hang around in this repo and slowly learn how the whole thing is put together. You all are doing amazing work. |
This PR implements the income and spending insight cards for the dashboard.
Requirements
Family
, scoped toPeriod.last_30_days
/claim #645
Resolves #645