Update facility details API #1333
Conversation
There was a problem hiding this comment.
I was able to curl facilities with additional field values and splitting the data processing into a set of smaller helper functions helps with readability.
I made some suggestions around multiple submissions and dissociation that I think we want to handle but weren't fully described in the initial issue.
src/django/api/serializers.py
Outdated
| list_items = FacilityListItem.objects.filter( | ||
| facility=facility, | ||
| source__contributor=contributor) |
There was a problem hiding this comment.
Not captured in the original issue description is the need to allow contributors to dissociate from facilities they no longer do business with, which we allow for via the is_active flags. I expect that adding 2 additional filter statements to this call to .filter() should take care of this
source__is_active=True,
facilitymatch__is_active=True
Although the same facility could be submitted multiple times and therefore have multiple line items matched to it, I think that we should assume that the most recent, active submission is the one from which we should pull the nonstandard field values. I expect that adding an order_by('-created_at') and then taking just the first matching row will take care of this.
| @@ -6494,3 +6495,143 @@ def test_querydict_in_api_data(self): | |||
| # We plan to only store raw data as CSV or JSON in the future. Legacy | |||
| # data can be ignored | |||
| self.assertNotIn('extra_2', content) | |||
|
|
|||
|
|
|||
| class ContributorFieldsApiTest(APITestCase): | |||
There was a problem hiding this comment.
Love to see a new test case appear.
src/django/api/serializers.py
Outdated
| def assign_contributor_field_values(list_items, fields): | ||
| contributor_fields = [{ | ||
| 'label': f.display_name, 'values': [], 'column_name': f.column_name | ||
| } for f in fields] | ||
|
|
||
| for item in list_items: | ||
| if item.source.source_type == 'SINGLE': | ||
| contributor_fields = get_single_contributor_field_values( | ||
| item, contributor_fields | ||
| ) | ||
| else: | ||
| contributor_fields = get_list_contributor_field_values( | ||
| item, contributor_fields | ||
| ) | ||
|
|
||
| return [{ | ||
| 'values': f['values'], 'label': f['label'] | ||
| } for f in contributor_fields] |
There was a problem hiding this comment.
Although the same facility could be submitted multiple times and therefore have multiple line items matched to it, I think that we should assume that the most recent, active submission is the one from which we should pull the nonstandard field values. I mentioned this at the point in the code where the QuerySet is built and, as a result, we should be able to take a single list item as a parameter to this function and use a single 'value': None to initialize the return rather than 'values': []'.
|
Ready for another look. |
jwalgran
added
the
task 004
The embedded map feature supports a contributor showing additional fields they have added that are not visible on OAR. When a contributor id and embed=1 are passed to the facility details API, an array of additional fields, as defined by that contributor's embedded map settings, are returned with the facility details.
TaiWilkin
force-pushed
the
tw/update-facility-details-api
branch
from
5551bf2
to
ffa7a81
Compare
Overview
The embedded map feature supports a contributor showing additional
fields they have added that are not visible on OAR. When a contributor
id and embed=1 are passed to the facility details API, an array of
additional fields, as defined by that contributor's embedded map
settings, are returned with the facility details.
Connects #1218
Demo
Notes
We previously discussed the format of the fields being:
{ "label": "Test Field", "value": "1234" }This works well when the contributor has only submitted the facility once; however, in some cases contributors submit a facility multiple times, which can result in multiple available values for a field.Currently, instead, all of the available values for the field are returned as an array
{ "label": "Test Field", "value": ["1234", "abc"] }. This seemed like the most honest presentation of the available data, but other options would be to only use the most recently submitted FacilityListItem for the facility (regardless of whether the submission includes all of the fields in question), or to use the most recently submitted FacilityListItem where the field is present for each field.Testing Instructions
vagrant sshand./scripts/test.GETthe details for the facilities you submitted, passing embed=1 and your contributor id as query params.http://localhost:6543/api/facilities/{facility id}/?embed=1&contributor={id}Checklist
fixup!commits have been squashed