Unembedded resources (from API refactor) #104
Conversation
Known issues: - Candidate pages may show multiple primary committees - Financials (totals/reports) are not ready
| committee['candidates'].append(candidate) | ||
| # we should get rid of this after the refactor | ||
| cands.append(cand['candidate_id']) | ||
| url = fields.Function(lambda x: url_for('committee_page', c_id=x['committee_id'])) |
There was a problem hiding this comment.
If we use flask-marshmallow, this can be slightly cleaner--something like
url = fields.URLFor('committee_page', c_id='<committee_id>')Conflicts: openfecwebapp/api_caller.py
- Also added missing committee_id
... to display gracefully
Conflicts: openfecwebapp/api_caller.py requirements.txt
... except: - There are TODOs which are also captured in 18F/openFEC/#695 - Some tests are failing
This is a bit overkill to get just the years; we should really have a little endpoint that just throws back a candidate's years
- And changed the name of the year range filter to match
- using has_authorized for committee display checks - uses is_primary and is_authorized to discern committees to display stats/charts for
- only request financial data for primary and authorized committees
arowla
changed the title
... instead of CandidateHistory
|
|
||
| @CommitteeSchema.error_handler | ||
| def handle_committee_errors(schema, errors, obj): | ||
| raise ValueError('An error occurred:\n{} \n\n...while marshalling the data:\n{}\n\n...for schema: {}'.format(errors, obj, schema)) |
There was a problem hiding this comment.
Not sure what raising a ValueError instead of the default ValidationError buys us. Both will raise a 500 in the app and throw a reasonable-looking traceback. If we want to throw a non-500 error on invalid data, we could call abort here instead.
There was a problem hiding this comment.
Originally I wasn't using strict, and it was failing silently. But this
probably isn't needed now that strict is turned on.
On Tue, May 5, 2015 at 5:10 PM, Joshua Carp notifications@github.com
wrote:
In openfecwebapp/models/committees.py
#104 (comment):
- organization_type = fields.Str(allow_none=True)
- organization_type_full = fields.Str(allow_none=True)
- party = fields.Str(allow_none=True)
- party_full = fields.Str(allow_none=True)
- class Meta:
additional = ('committee_id', 'name', 'committee_type', 'committee_type_full','treasurer_name', 'designation', 'designation_full','treasurer_name', 'street_1', 'city', 'state', 'zip')+@CommitteeSchema.error_handler
+def handle_committee_errors(schema, errors, obj):- raise ValueError('An error occurred:\n{} \n\n...while marshalling the data:\n{}\n\n...for schema: {}'.format(errors, obj, schema))
Not sure what raising a ValueError instead of the default ValidationError
buys us. Both will raise a 500 in the app and throw a reasonable-looking
traceback. If we want to throw a non-500 error on invalid data, we could
call abort here instead.—
Reply to this email directly or view it on GitHub
https://github.com/18F/openFEC-web-app/pull/104/files#r29713320.
Alison Rowland
Technical Lead, https://fbopen.gsa.gov | 18F
202-317-0124
Conflicts: templates/committees-single.html
- Now in the style of (*args, **kwargs), _call_api will build up path components from the positional arguments and run `os.path.join` on them, and filters are passed in as keyword arguments
Conflicts: __init__.py
- So that the default() filter can still do its job - Changed so default() is the last step - Moved all filters into __init__ - Standardized on registering filters with a decorator
- Giving a high limit so that candidates with lots of committees don't get important things like their primary committee paginated out of the results
- render_page() -> render_candidate(), render_committee() - Also stopped using Marshmallow for nested types - Harmonized totals/financials data structure between candidate and committee pages and templates
- Also refactored out the type_map from the views - Renamed the get_results_or_500 method to reflect that it pulls just the first result - Removed now-tautological data_prep tests
arowla
force-pushed
the
feature/unembed-resources
branch
from
bcc5fdf
to
6bb7097
Compare
| return render_template('candidates-single.html', **tmpl_vars) | ||
|
|
||
|
|
||
| def get_first_result_or_raise_500(data): |
There was a problem hiding this comment.
This behavior predates this patch, so we can defer discussion until later, but I'm not sure we should be throwing a 500 here. This behavior doesn't give much feedback to the user or to the developer, and it can potentially give incorrect feedback: for example, browsing to /candidate/fakecandidate should throw a 404, but this logic will throw a 500 instead.
There was a problem hiding this comment.
Agreed, let's talk about this at some point.
|
Added a few very minor comments inline, but LGTM 👍 |
- Also removed other extraneous print calls
Conflicts: templates/partials/financial-summary.html
Known issues: