Skip to content
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

[WIP]Refactor elections endpoint to handle candidate finance totals export function #3292

Merged
merged 3 commits into from
Jul 20, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions tests/test_downloads.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ def test_download_cached(self, export, get_cached):
assert not export.delay.called

def test_download_forbidden(self):
with pytest.raises(ApiError):
self.client.post_json(api.url_for(resource.DownloadView, path='elections', office='house', cycle=2018))
response = self.app.get(api.url_for(resource.DownloadView, path='elections', office='house', cycle=2018))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unclear about what this test is trying to accomplish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear either, Can we remove this test case function? Thanks @vrajmohan

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there another view that does not support downloads that can be used for this test instead?

Copy link
Contributor Author

@fec-jli fec-jli Jul 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question. I look at most of our endpoints, we do have some endpoints don't support download, but they are not added into this test_download_forbidden function.
How about change function name to donttest_download_forbidden for now? in case, we need test other endpoints in the future? Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand "they are not added into this test_download_forbidden function". Why can't we use one of those paths instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, I add '/elections/search' (resource.ElectionListView) endpoint into test_download_forbidden function. work well. Thanks.

self.assertEqual(response.status_code, 405)

@mock.patch('webservices.resources.download.MAX_RECORDS', 2)
@mock.patch('webservices.resources.download.get_cached_file')
Expand Down
5 changes: 2 additions & 3 deletions tests/test_elections.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ def test_conditional_missing_params(self):
self.assertEquals(response.status_code, 200)

def test_empty_query(self):
results = self._results(api.url_for(ElectionView, office='senate', cycle=2012, state='ZZ', per_page=0))
self.assertEqual(len(results), 0)
results = self._results(api.url_for(ElectionView, office='senate', cycle=2012, state='ZZ'))
assert len(results) == 0

def test_elections(self):
results = self._results(api.url_for(ElectionView, office='senate', cycle=2012, state='NY'))
Expand Down Expand Up @@ -245,7 +245,6 @@ def test_electionview_excludes_jfc(self):
assert_dicts_subset(results[0], expected)
assert set(results[0]['committee_ids']) == set(each.committee_id for each in self.committees)


def test_election_summary(self):
results = self._response(api.url_for(ElectionSummary, office='senate', cycle=2012, state='NY'))
totals = [each for each in self.totals if each.cycle == 2012]
Expand Down
9 changes: 2 additions & 7 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,13 @@ def test_hide_null_election(self):

]
db.session.flush()
arg_map = {}
arg_map['office'] = 'senate'
arg_map['cycle'] = 2016
arg_map['state'] = 'MO'
#arg_map['district'] = '00'

electionView = elections.ElectionView()
query, columns = sorting.sort(electionView._get_records(arg_map), 'total_disbursements', model=None)
query, columns = sorting.sort(electionView.build_query(office='senate', cycle=2016, state='MO'), 'total_disbursements', model=None)

#print(str(query.statement.compile(dialect=postgresql.dialect())))
self.assertEqual(len(query.all()), len(candidates))
query, columns = sorting.sort(electionView._get_records(arg_map), 'total_disbursements', model=None, hide_null=True)
query, columns = sorting.sort(electionView.build_query(office='senate', cycle=2016, state='MO'), 'total_disbursements', model=None, hide_null=True)
#Taking this assert statement out because I believe, at least how the FEC interprets null (i.e. none) primary
#committees for a candidate is that they have in fact raised/spent 0.0 dollars, this can be shown as true
#using the Alabama special election as an example
Expand Down
25 changes: 14 additions & 11 deletions webservices/resources/elections.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,20 @@ def _filter_zip(self, query, kwargs):
description=docs.ELECTIONS,
tags=['financial']
)
class ElectionView(utils.Resource):

@use_kwargs(args.paging)
@use_kwargs(args.elections)
@use_kwargs(args.make_sort_args(default='-total_receipts'))
@marshal_with(schemas.ElectionPageSchema())
def get(self, **kwargs):
query = self._get_records(kwargs)
return utils.fetch_page(query, kwargs, cap=0)
class ElectionView(ApiResource):
schema = schemas.ElectionSchema
page_schema = schemas.ElectionPageSchema
@property
def args(self):
return utils.extend(
args.paging,
args.elections,
args.make_sort_args(
default='-total_receipts',
),
)

def _get_records(self, kwargs):
def build_query(self, **kwargs):
utils.check_election_arguments(kwargs)
totals_model = office_totals_map[kwargs['office']]
pairs = self._get_pairs(totals_model, kwargs).subquery()
Expand Down Expand Up @@ -179,7 +182,7 @@ def _get_latest(self, pairs):
).subquery()
return db.session.query(
latest.c.candidate_id,
sa.func.sum(sa.func.coalesce(latest.c.cash_on_hand_end_period,0.0)).label('cash_on_hand_end_period'),
sa.func.sum(sa.func.coalesce(latest.c.cash_on_hand_end_period, 0.0)).label('cash_on_hand_end_period'),
).group_by(
latest.c.candidate_id,
)
Expand Down
2 changes: 2 additions & 0 deletions webservices/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,8 @@ class ElectionSchema(ma.Schema):
coverage_end_date = ma.fields.Date()
augment_schemas(ElectionSchema)

ElectionPageSchema = make_page_schema(ElectionSchema)
register_schema(ElectionPageSchema)

class ScheduleABySizeCandidateSchema(ma.Schema):
candidate_id = ma.fields.Str()
Expand Down