-
Notifications
You must be signed in to change notification settings - Fork 31
Feature/candidate other spending #732
Feature/candidate other spending #732
Conversation
var expendituresColumns = [ | ||
tables.dateColumn({data: 'expenditure_date'}), | ||
tables.currencyColumn({data: 'expenditure_amount'}), | ||
tables.committeeColumn({data: 'committee', className: 'all', orderable: false}), |
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.
Line is too long.
I am so excited about this! |
Nice! Working well. Can you add sorting to the third and fourth columns as well? |
For the candidate page, can we show the candidate aggregates instead of the line items? So we would just show "$100 Fake PAC oppose" on the candidate page. What you have here, I think, would be perfect for the committee page. Also @noahmanger @emileighoutlaw any ideas how to make it very clear that this money is not spent by the candidate? |
Ah I was wondering about showing itemized vs. aggregates. I think I agree: let's show aggregates here and once we have an IE browse table, we can show the itemized there. And yes, we need a blurb explaining what IEs are and how they are not made in coordination with the candidate. |
Now that I am thinking about it, I think that schedule_e should be in a browse view and committees should have the aggregates too, like the election pages. |
schedule_e is IEs? We have an issue started for getting these on committee pages: #701 |
Yea, #701 says to aggregate by candidate and that sounds right to me. |
var $table = $('table[data-type="independent-expenditure"]'); | ||
var path = ['schedules', 'schedule_e', 'by_candidate']; | ||
var query = { | ||
candidate_id: $table.data('candidate'), |
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.
Identifier 'candidate_id' is not in camel case.
Also adds sorting by committee and support/oppose. h/t @LindsayYoung, @noahmanger
be425f0
to
5b80b43
Compare
Patch updated to use aggregates. |
This looks good to me except "Given by" should be "Spent by". Are we heeding all these Hound warnings? |
Updated. When I wrote the patch, we were still using "given by". I also changed the text on the election other spending tab. For now, I'm paying attention to some Hound warnings more than others. I'm not fanatical about line length requirements, so am going to ignore those for now. We can customize the Hound configuration to skip certain warnings, which I'll get to soon. |
Great. Makes sense to me. Hound does seem helpful for sure. |
Feature/candidate other spending
Resolves #700. Flagging @noahmanger and @LindsayYoung to review behavior and code.