-
Notifications
You must be signed in to change notification settings - Fork 1
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
Convert available funds / contributions/expenditures from area to step charts #216
base: main
Are you sure you want to change the base?
Changes from 1 commit
3da50d3
e5796ec
8c7b59a
26d670d
8a95238
4338d93
99fca26
f21a76e
16dfa3d
a3f98a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
from collections import namedtuple | ||
from datetime import datetime | ||
|
||
from django.db import connection, models | ||
from django.utils import timezone | ||
|
@@ -946,16 +945,9 @@ def trends(self, since="2010"): | |
# Balances and debts | ||
summed_filings = """ | ||
SELECT | ||
SUM(f.total_contributions) + \ | ||
SUM(COALESCE(f.total_supplemental_contributions, 0)) AS total_contributions, | ||
SUM(f.total_expenditures) AS total_expenditures, | ||
SUM(COALESCE(f.total_loans, 0)) AS total_loans, | ||
SUM(COALESCE(f.total_unpaid_debts, 0)) AS total_unpaid_debts, | ||
SUM(f.closing_balance) AS closing_balance, | ||
SUM(f.opening_balance) AS opening_balance, | ||
SUM(f.total_debt_carried_forward) AS debt_carried_forward, | ||
f.filed_date, | ||
MIN(fp.initial_date) AS initial_date | ||
f.filed_date | ||
FROM camp_fin_filing AS f | ||
JOIN camp_fin_filingperiod AS fp | ||
ON f.filing_period_id = fp.id | ||
|
@@ -973,45 +965,16 @@ def trends(self, since="2010"): | |
|
||
cursor.execute(summed_filings, [self.id]) | ||
|
||
columns = [c[0] for c in cursor.description] | ||
filing_tuple = namedtuple("Filings", columns) | ||
|
||
summed_filings = [filing_tuple(*r) for r in cursor] | ||
|
||
balance_trend, debt_trend = [], [] | ||
|
||
if summed_filings: | ||
for filing in summed_filings: | ||
filing_date = filing.filed_date | ||
date_array = [filing_date.year, filing_date.month, filing_date.day] | ||
debts = -1 * filing.total_unpaid_debts | ||
if filing.closing_balance: | ||
balance_trend.append([filing.closing_balance, *date_array]) | ||
debt_trend.append([debts, *date_array]) | ||
|
||
if summed_filings[0].opening_balance: | ||
first_opening_balance = summed_filings[0].opening_balance | ||
else: | ||
first_opening_balance = 0 | ||
|
||
if summed_filings[0].debt_carried_forward: | ||
first_debt = summed_filings[0].debt_carried_forward | ||
else: | ||
first_debt = 0 | ||
|
||
init_date = summed_filings[0].initial_date | ||
|
||
first_initial_date = [int(since), 1, 1] | ||
|
||
# If the first available filing date is on or before the start date | ||
# passed into this method, use that as the first date in the trendline | ||
if init_date: | ||
init_date_parts = [init_date.year, init_date.month, init_date.day] | ||
if datetime(*init_date_parts) <= datetime(*first_initial_date): | ||
first_initial_date = init_date_parts | ||
|
||
debt_trend.insert(0, [first_debt, *first_initial_date]) | ||
balance_trend.insert(0, [first_opening_balance, *first_initial_date]) | ||
for ( | ||
total_unpaid_debts, | ||
closing_balance, | ||
filed_date, | ||
) in cursor: | ||
filing_date = (filed_date.year, filed_date.month, filed_date.day) | ||
balance_trend.append([closing_balance, *filing_date]) | ||
debt_trend.append([total_unpaid_debts * -1, *filing_date]) | ||
|
||
output_trends = {"balance_trend": balance_trend, "debt_trend": debt_trend} | ||
|
||
|
@@ -1020,15 +983,15 @@ def trends(self, since="2010"): | |
SELECT | ||
months.year, | ||
months.month, | ||
{table}.amount | ||
COALESCE({table}.amount, 0) AS amount | ||
FROM ( | ||
SELECT | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. optional suggestion: this looks like a good place to use a CTE. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CTEs aren't supported in the version of Postgres we're using for local development (which might not be a constraint in deployment, actually, but don't feel like upgrading Postgres here). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, we should have the development env match up, so I can open an issue for that. fine not to change that here. |
||
DISTINCT DATE_PART('year', month) AS year, | ||
GENERATE_SERIES(1, 12) AS month | ||
FROM {table}_by_month | ||
ORDER BY year, month | ||
) months | ||
LEFT JOIN ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wait if we don't need to do LEFT JOIN here, then that suggests that every month we need is already in the generated table, in which case why do we need this subquery at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we don't need that left join here, then we should probably move the coalese logic up to the aggregate functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that would be awesome, because right now, the aggregate tables are sparse, so we can't distinguish between when there were $0 in transactions or we don't have data. That's why I did away with this left join - for the months on either end of the range, it reported $0 in transactions, but that might not actually be accurate. |
||
JOIN ( | ||
SELECT | ||
{table}.amount AS amount, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. optional suggestion, tbc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added up above! |
||
DATE_PART('month', {table}.month) AS month, | ||
|
@@ -1038,23 +1001,20 @@ def trends(self, since="2010"): | |
AND {table}.month >= '{year}-01-01'::date | ||
) {table} | ||
USING (year, month) | ||
ORDER BY year, month | ||
""" | ||
|
||
contributions_query = monthly_query.format(table="contributions", year=since) | ||
|
||
cursor.execute(contributions_query, [self.id]) | ||
|
||
donation_trend = [ | ||
[amount or 0, year, month, 1] for year, month, amount in cursor | ||
] | ||
donation_trend = [[amount, year, month, 1] for year, month, amount in cursor] | ||
|
||
expenditures_query = monthly_query.format(table="expenditures", year=since) | ||
|
||
cursor.execute(expenditures_query, [self.id]) | ||
|
||
expend_trend = [ | ||
[(amount or 0) * -1, year, month, 1] for year, month, amount in cursor | ||
] | ||
expend_trend = [[amount * -1, year, month, 1] for year, month, amount in cursor] | ||
|
||
output_trends["donation_trend"] = donation_trend | ||
output_trends["expend_trend"] = expend_trend | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,10 +83,10 @@ <h4> | |
{% endblock %} | ||
|
||
{% block charts %} | ||
<h3><i class="fa fa-fw fa-area-chart"></i> Net funds over time</h3> | ||
<h3><i class="fa fa-fw fa-area-chart"></i> Funds available and debts by filing date</h3> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Net is a difference in values. We're showing both values. |
||
<div id='net-funds-chart'></div> | ||
|
||
<h3><i class="fa fa-fw fa-bar-chart"></i> Donations and expenditures over time</h3> | ||
<h3><i class="fa fa-fw fa-bar-chart"></i> Contributions and expenditures by month</h3> | ||
<div id='expend-chart'></div> | ||
|
||
{% endblock %} | ||
|
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'm not sure about doing this inference, since the other values are as-of the filing date, and we don't have the date for the filing before the first one in our database. i omitted it, but open to your thoughts.
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 think it's fine to omit.