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 enable candidate financial totals export #3253

Closed
wants to merge 1 commit into from

Conversation

fec-jli
Copy link
Contributor

@fec-jli fec-jli commented Jun 29, 2018

Summary (required)

in order to enable “candidate financial totals” data in election summery page, we need refactor election endpoint to extend ApiResource and overwrite 'build_query()'

openFEC Issue: Refactor elections (ElectionView) endpoint to extend ApiResource
#3254

How to test the changes locally

This PR involves changes on both fec-cms and openFEC side.

  1. checkout fec-cms feature branch: feature/enable_export_link_candidate_info
    export FEC_WEB_API_KEY_PUBLIC=xxxxxxx
    export FEC_WEB_API_KEY=xxxxxxx
    export FEC_CMS_ENVIRONMENT=LOCAL
    export FEC_API_URL=http://localhost:5000
    export DATABASE_URL=postgresql://:@/cfdm_cms_test

2)check out openFEC feature branch: feature/fix_elections_download
setup openFEC local (env api variables)
export SQLA_CONN=xxxxxxx
export access_key_id=
export secret_access_key=
export bucket=
export region=

3)start redis-server

4)Setup Celery Worker and start
export SQLA_CONN=xxxxxxx
export access_key_id=
export secret_access_key=
export bucket=
export region=

5)example url: http://127.0.0.1:8000/data/elections/senate/VA/2018/
Click "Export" on Candidate financial totals section, download the file.

1)refactor ElectionView extand ApiResource.
2)add CandidateCommitteeLink.committee_designation.in_(['P', 'A'])
to remove joint fundraisers committees inflated figures from total
@fec-jli fec-jli requested review from vrajmohan and lbeaufort June 29, 2018 14:22
@fec-jli fec-jli changed the title Remove the joint fundraisers inflating receipts and enable candidate financial totals export [WIP]Remove the joint fundraisers inflating receipts and enable candidate financial totals export Jun 29, 2018
@vrajmohan
Copy link
Contributor

Apropos of our conversation on a clean git history, it would be better to break up the commits into 2:

  1. For the refactor to use ApiResource
  2. Remove the joint fundraisers inflating receipts

Also, it would be easier for the reviewer if you rebased this on develop to remove the extraneous commits "d9fb37e6d59a2fe6dc742c8c6442fc49a6c1bba0" and "f920bddf0ddc6299feb6867e0f068e324656f6f4".

@lbeaufort
Copy link
Member

@vrajmohan since this is targeting the release branch, should @fec-jli rebase off the release branch?

@vrajmohan
Copy link
Contributor

Right, good catch!

@@ -280,6 +288,7 @@ def join_candidate_totals(query, kwargs, totals_model):
sa.and_(
CandidateHistory.candidate_id == CandidateCommitteeLink.candidate_id,
CandidateHistory.two_year_period == CandidateCommitteeLink.fec_election_year,
CandidateCommitteeLink.committee_designation.in_(['P', 'A']),
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment on why this is needed may be useful. Also, should we create a test that would add other committee designations and assert that they are not picked up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is a comment on why this is needed:
Joint Fundraising Committees raise money for multiple candidate committees and transfer that money to those committees. By limiting the committee designations to A and P you eliminate J (joint) and thus do not inflate the candidate's money by including it twice and by including money that was raised and transferred to the other committees in the joint fundraiser.

@fec-jli fec-jli changed the title [WIP]Remove the joint fundraisers inflating receipts and enable candidate financial totals export Remove the joint fundraisers inflating receipts and enable candidate financial totals export Jul 2, 2018
@fec-jli fec-jli changed the title Remove the joint fundraisers inflating receipts and enable candidate financial totals export [WIP]Remove the joint fundraisers inflating receipts and enable candidate financial totals export Jul 2, 2018
@fec-jli fec-jli changed the title [WIP]Remove the joint fundraisers inflating receipts and enable candidate financial totals export [WIP]refactor elections endpoint to enable candidate financial totals export Jul 2, 2018
@lbeaufort lbeaufort changed the base branch from release/public-20180705 to develop July 6, 2018 19:59
@lbeaufort
Copy link
Member

@fec-jli I changed the target branch back to develop because this didn't get merged. Is it still a [WIP]?

@JonellaCulmer
Copy link
Contributor

@fec-jli Closing this PR in favor of a more up-to-date PR.

@fec-jli fec-jli deleted the feature/fix_elections_download branch July 30, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants