-
Notifications
You must be signed in to change notification settings - Fork 106
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
Add national party schedule a/b endpoints #5791
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5791 +/- ##
===========================================
+ Coverage 85.77% 86.18% +0.41%
===========================================
Files 81 83 +2
Lines 8624 8918 +294
===========================================
+ Hits 7397 7686 +289
- Misses 1227 1232 +5 ☔ View full report in Codecov by Sentry. |
d89d129
to
fee4f9a
Compare
3c6f9c0
to
a522d31
Compare
8d9be99
to
050db6e
Compare
050db6e
to
399457a
Compare
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.
Can we remove these null fields. I think ideally they wouldn't be in the MV, but can we at least take them out of the model? Can someone confirm what I'm seeing--that they all are null?
organization_type
organization_type_full
candidate_first_name
candidate_id
candidate_last_name
candidate_middle_name
candidate_name
candidate_office
candidate_office_district
candidate_office_full
candidate_office_state
candidate_office_state_full
conduit_committee_city
conduit_committee_id
conduit_committee_name
conduit_committee_state
conduit_committee_street1
conduit_committee_street2
conduit_committee_zip
candidate_prefix
candidate_suffix
Sure, I can take a look and confirm if all the fields are null. |
399457a
to
9ed935f
Compare
9ed935f
to
fba21ff
Compare
@PaulClark2 suggested removing the organization and conduit* fields since party committees typically lack that data. However, he recommended keeping the candidate information since some party committees do have relevant data for it. |
fba21ff
to
5b8e1f7
Compare
I removed the organization and conduit* fields from the NPA model file |
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.
Fabulous job @pkfec! Thank you so much!
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 works as expected.
Could we consider creating a separate ticket to add a 'Zipcode' validation function in args.py? This would help eliminate the excessive duplication of Zipcode validation code in many places.
Thanks for the feedback @fec-jli. I can make up a ticket to move the Zipcode validation function to webservies.utils.py instead of args.py as most of the validations happens in utils.py |
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.
Really awesome job @pkfec!
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.
Great work!
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.
Endpoints look good . Great job Priya!
5b8e1f7
to
6149d17
Compare
I added |
@pkfec Just one thing for the endpoints:
|
f168327
to
2f762f5
Compare
2f762f5
to
ab254ab
Compare
@JonellaCulmer I updated ^^ filter descriptions in docs.py |
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.
This looks really great, @pkfec. Thanks for all your hard work on this.
Summary
Create two new national party account endpoints (Transaction level) under swagger docs
/national_party/schedule_a/
/national_party/schedule_b/
Resolves #5786
Related #5811
Required reviewers
1 frontend and 2 or more backend developers
Screenshots
How to test
On local:
run
git checkout feature/nation-party-endpoints
run
pyenv activate venv-api
run
pip install -r requirements.txt
run
pip install -r requirements-dev.txt
run
pytest
run
flask run
test filters under national_party/schedule_a endpoint:
http://127.0.0.1:5000/v1/national_party/schedule_a/?page=1&per_page=20&sort=-contribution_receipt_date&sort_hide_null=false&sort_null_only=false
test filters under national_party/schedule_b endpoint:
http://127.0.0.1:5000/v1/national_party/schedule_b/?page=1&per_page=20&sort=-disbursement_date&sort_hide_null=false&sort_null_only=false
OR
Deploy a test branch to Dev space and test the NPA filters