-
Notifications
You must be signed in to change notification settings - Fork 40
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
5215 house and senate overview totals #5443
5215 house and senate overview totals #5443
Conversation
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.
Noticed that the query statements should be tweaked slightly to display the correct totals.
theURL += `&election_year=${electionYear}&office=${window.context.office_code}&api_key=`; | ||
|
||
fetch( | ||
`${theURL}&aggregate_by=office`, |
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.
It appears that this is concatenating two aggregate_by
parameters into the same call. But you can only pass one in a single call. The total raised, spent, and cash on hand dollar figures should be coming from a call that passes aggregate_by=office
. The party breakdown for raised, spent, and cash on hand should be coming from a call that passes aggregate_by=office-party
.
The dollar figures in this feature should match the dollar figures that we have in the summary raising by the numbers and summary spending by the numbers pages.
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.
Oooh, good catch!
Aggregate totals is using /candidates/totals/by_office/
and …by_party/
but the ticket attached to this says to use /candidates/totals/aggregates/
.
Should they be using different endpoints?
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.
Eventually /candidates/totals/by_office/
and /candidates/totals/by_party/
will be deprecated since we found a better and cleaner way to do the totals with the /candidates/totals/aggregates/
endpoint. I'll file a ticket for that later.
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.
When I look at the values coming back from those endpoints, the values aren't the same. Looking specifically at S, 2020, Dem, total receipts,
/candidates/totals/by_office/by_party/?election_full=true&election_year=2020&sort_hide_null=false&sort_null_only=false&per_page=20&sort_nulls_last=false&office=S&page=1&api_key=
returns 1245650818.65
/candidates/totals/aggregates/?election_full=true&is_active_candidate=true&per_page=10&aggregate_by=office-party&election_year=2020&office=S&api_key=
returns 1238433907.94
// Let's do some math to get real total_contributions | ||
usefulResults.total_contributions = { | ||
total: | ||
usefulResults.total_individual_itemized_contributions.total | ||
+ usefulResults.total_other_political_committee_contributions.total | ||
+ usefulResults.total_transfers_from_other_authorized_committee.total, | ||
DEM: | ||
usefulResults.total_individual_itemized_contributions.DEM | ||
+ usefulResults.total_other_political_committee_contributions.DEM | ||
+ usefulResults.total_transfers_from_other_authorized_committee.DEM, | ||
REP: | ||
usefulResults.total_individual_itemized_contributions.REP | ||
+ usefulResults.total_other_political_committee_contributions.REP | ||
+ usefulResults.total_transfers_from_other_authorized_committee.REP, | ||
Other: | ||
usefulResults.total_individual_itemized_contributions.Other | ||
+ usefulResults.total_other_political_committee_contributions.Other | ||
+ usefulResults.total_transfers_from_other_authorized_committee.Other | ||
}; |
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.
@rfultz The total raised should actually be total_receipts
and not the aggregate of all of these put together. This is the discrepancy we're seeing in the raising data.
Codecov Report
@@ Coverage Diff @@
## develop #5443 +/- ##
===========================================
+ Coverage 75.16% 75.18% +0.01%
===========================================
Files 125 125
Lines 8123 8123
Branches 648 649 +1
===========================================
+ Hits 6106 6107 +1
+ Misses 2017 2016 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
<figure class="js-party-money-bars" id="summary-total-raised" data-event-field-id="total_receipts"> | ||
<div class="total-wrapper"> | ||
<h1 class="value js-value-large">$1,234,567,890</h1> | ||
<h2 class="description js-value-large-desc">raised by {{ office | title }} candidates</h2> |
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 we can drop "House" from these so that the cash on hand one doesn't wrap. As in "raised by House candidates" "spent by House candidates" and "cash on hand for House candidates"
@rfultz Great work!!! Just a few odds and ends:
After (As seen on Raising browse data page):
|
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.
Thanks @rfultz, looks good now
Summary
Quite a few changes to /data/elections/house/ and /data/elections/senate/
Required reviewers
Front-end and UX?
Impacted areas of the application
The House & Senate overview pages
Screenshots
Before:
This PR:
Related PRs
None
Known TODOs
How to test
npm i
npm run build
./manage.py runserver