Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Return contributor details from list API and include in CSV download #1005

Merged
merged 4 commits into from
Mar 30, 2020
Merged

Return contributor details from list API and include in CSV download #1005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6786260
Return contributor details from list API and include in CSV download
jwalgran Mar 25, 2020
32eae9f
Prevent contributor names and list names from containing |
jwalgran Mar 27, 2020
77a4ea1
Show a determinate progress bar when download facility CSV
jwalgran Mar 27, 2020
0ade4fb
Update CHANGELOG
jwalgran Mar 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
## [Unreleased]

### Added
- Return contributor details from list API and include in CSV download [#1005](https://github.com/open-apparel-registry/open-apparel-registry/pull/1005)

### Changed

Expand Down
21 changes: 21 additions & 0 deletions src/app/src/__tests__/utils.facilitiesCSV.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ it('creates a new facility row array from a feature', () => {
country_code: 'country_code',
country_name: 'country_name',
oar_id: 'oar_id',
contributors: [
{
id: 1,
name: 'contributor_name (list_name)',
verified: false,
},
{
id: 2,
name: 'contributor_2_name (list_2_name)',
verified: true,
},
],
},
geometry: {
coordinates: [
Expand All @@ -32,6 +44,7 @@ it('creates a new facility row array from a feature', () => {
'country_name',
'lat',
'lng',
'contributor_name (list_name)|contributor_2_name (list_2_name)',
];

expect(isEqual(
Expand All @@ -49,6 +62,13 @@ it('creates a 2-d array including headers for exporting as a CSV', () => {
country_code: 'country_code',
country_name: 'country_name',
oar_id: 'oar_id',
contributors: [
{
id: 1,
name: 'contributor_name',
verified: false,
},
],
},
geometry: {
coordinates: [
Expand All @@ -69,6 +89,7 @@ it('creates a 2-d array including headers for exporting as a CSV', () => {
'country_name',
'lat',
'lng',
'contributor_name',
],
];

Expand Down
13 changes: 12 additions & 1 deletion src/app/src/components/FilterSidebarFacilitiesTab.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { arrayOf, bool, func, number, string } from 'prop-types';
import { connect } from 'react-redux';
import { Link } from 'react-router-dom';
import CircularProgress from '@material-ui/core/CircularProgress';
import LinearProgress from '@material-ui/core/LinearProgress';
import Dialog from '@material-ui/core/Dialog';
import DialogTitle from '@material-ui/core/DialogTitle';
import DialogContent from '@material-ui/core/DialogContent';
Expand Down Expand Up @@ -82,6 +83,9 @@ const facilitiesTabStyles = Object.freeze({
height: '45px',
margin: '5px 0',
}),
downloadLabelStyles: Object.freeze({
margin: '5px 0',
}),
});

function FilterSidebarFacilitiesTab({
Expand Down Expand Up @@ -191,6 +195,10 @@ function FilterSidebarFacilitiesTab({
const LoginLink = props => <Link to={authLoginFormRoute} {...props} />;
const RegisterLink = props => <Link to={authRegisterFormRoute} {...props} />;

const progress = facilitiesCount
? get(data, 'features', []).length * 100 / facilitiesCount
: 0;

const listHeaderInsetComponent = (
<div style={facilitiesTabStyles.listHeaderStyles} className="results-height-subtract">
<Typography
Expand All @@ -204,7 +212,10 @@ function FilterSidebarFacilitiesTab({
downloadingCSV
? (
<div style={facilitiesTabStyles.listHeaderButtonStyles}>
<CircularProgress />
<div style={facilitiesTabStyles.downloadLabelStyles}>
Downloading...
</div>
<LinearProgress variant="determinate" value={progress} />
</div>)
: (
<Button
Expand Down
2 changes: 1 addition & 1 deletion src/app/src/components/FilterSidebarSearchTab.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ function mapDispatchToProps(dispatch, {
return dispatch(resetAllFilters());
},
searchForFacilities: vectorTilesAreActive => dispatch(fetchFacilities({
pageSize: vectorTilesAreActive ? FACILITIES_REQUEST_PAGE_SIZE : 500,
pageSize: vectorTilesAreActive ? FACILITIES_REQUEST_PAGE_SIZE : 50,
pushNewRoute: push,
})),
submitFormOnEnterKeyPress: makeSubmitFormOnEnterKeyPressFunction(
Expand Down
2 changes: 1 addition & 1 deletion src/app/src/util/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const OTHER = 'Other';

export const FACILITIES_REQUEST_PAGE_SIZE = 100;
export const FACILITIES_REQUEST_PAGE_SIZE = 50;

// This choices must be kept in sync with the identical list
// kept in the Django API's models.py file
Expand Down
3 changes: 3 additions & 0 deletions src/app/src/util/util.facilitiesCSV.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const csvHeaders = Object.freeze([
'country_name',
'lat',
'lng',
'contributors',
]);

export const createFacilityRowFromFeature = ({
Expand All @@ -20,6 +21,7 @@ export const createFacilityRowFromFeature = ({
country_code,
country_name,
oar_id,
contributors,
},
geometry: {
coordinates: [
Expand All @@ -35,6 +37,7 @@ export const createFacilityRowFromFeature = ({
country_name,
lat,
lng,
contributors ? contributors.map(c => c.name).join('|') : '',
]);

export const facilityReducer = (acc, next) =>
Expand Down
4 changes: 2 additions & 2 deletions src/django/api/pagination.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ class PageAndSizePagination(PageNumberPagination):
class FacilitiesGeoJSONPagination(GeoJsonPagination):
page_query_param = 'page'
page_size_query_param = 'pageSize'
page_size = 500
max_page_size = 500
page_size = 50
max_page_size = 50
55 changes: 25 additions & 30 deletions src/django/api/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,12 @@ def validate_status(self, value):
class FacilitySerializer(GeoFeatureModelSerializer):
oar_id = SerializerMethodField()
country_name = SerializerMethodField()
contributors = SerializerMethodField()

class Meta:
model = Facility
fields = ('id', 'name', 'address', 'country_code', 'location',
'oar_id', 'country_name')
'oar_id', 'country_name', 'contributors')
geo_field = 'location'

# Added to ensure including the OAR ID in the geojson properties map
Expand All @@ -430,13 +431,33 @@ def get_oar_id(self, facility):
def get_country_name(self, facility):
return COUNTRY_NAMES.get(facility.country_code, '')

def get_contributors(self, facility):
def format_source(source):
if type(source) is Source:
return {
'id': source.contributor.admin.id
if source.contributor else None,
'name': source.display_name,
'is_verified': source.contributor.is_verified
if source.contributor else False,
}
return {
'name': source,
}
request = self.context.get('request') \
if self.context is not None else None
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be shortened to

if self.context else None

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use this construction a few places in models.py too. In general I tend to prefer to use explicit None checks when I can to avoid type coercion ambiguity, like empty objects being falsey.

>>> if {'key': 1}:
...   print('true')
...
true
>>> if {}:
...   print('true')
...
>>>

user = request.user if request is not None else None
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An explicit None check is less helpful here, but I have opted to use the same construction for consistency.

return [
format_source(source)
for source
in facility.sources(user=user)
]


class FacilityDetailsSerializer(GeoFeatureModelSerializer):
oar_id = SerializerMethodField()
class FacilityDetailsSerializer(FacilitySerializer):
other_names = SerializerMethodField()
other_addresses = SerializerMethodField()
other_locations = SerializerMethodField()
contributors = SerializerMethodField()
country_name = SerializerMethodField()
claim_info = SerializerMethodField()

Expand All @@ -447,10 +468,6 @@ class Meta:
'country_name', 'claim_info', 'other_locations')
geo_field = 'location'

# Added to ensure including the OAR ID in the geojson properties map
def get_oar_id(self, facility):
return facility.id

def get_other_names(self, facility):
return facility.other_names()

Expand Down Expand Up @@ -499,28 +516,6 @@ def get_other_locations(self, facility):

return facility_locations + facility_matches

def get_contributors(self, facility):
def format_source(source):
if type(source) is Source:
return {
'id': source.contributor.admin.id
if source.contributor else None,
'name': source.display_name,
'is_verified': source.contributor.is_verified
if source.contributor else False,
}
return {
'name': source,
}
request = self.context.get('request') \
if self.context is not None else None
user = request.user if request is not None else None
return [
format_source(source)
for source
in facility.sources(user=user)
]

def get_country_name(self, facility):
return COUNTRY_NAMES.get(facility.country_code, '')

Expand Down
30 changes: 29 additions & 1 deletion src/django/api/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ def create(self, request, *args, **kwargs):
if name is None:
raise ValidationError('name cannot be blank')

if '|' in name:
raise ValidationError('name cannot contain the "|" character')

if description is None:
raise ValidationError('description cannot be blank')

Expand Down Expand Up @@ -293,6 +296,9 @@ def put(self, request, pk, *args, **kwargs):
if name is None:
raise ValidationError('name cannot be blank')

if '|' in name:
raise ValidationError('Name cannot contain the "|" character.')

if description is None:
raise ValidationError('description cannot be blank')

Expand Down Expand Up @@ -646,7 +652,14 @@ def list(self, request):
"address" "facility address_1",
"country_code": "US",
"country_name": "United States",
"oar_id": "OAR_ID_1"
"oar_id": "OAR_ID_1",
"contributors": [
{
"id": 1,
"name": "Brand A (2019 Q1 List)",
"is_verified": false
}
]
}
},
{
Expand All @@ -662,6 +675,18 @@ def list(self, request):
"country_code": "US",
"country_name": "United States",
"oar_id": "OAR_ID_2"
"contributors": [
{
"id": 1,
"name": "Brand A (2019 Q1 List)",
"is_verified": false
},
{
"id": 2,
"name": "MSI B (2020 List)",
"is_verified": true
}
]
}
}
]
Expand Down Expand Up @@ -1845,6 +1870,9 @@ def create(self, request):
else:
name = os.path.splitext(csv_file.name)[0]

if '|' in name:
raise ValidationError('Name cannot contain the "|" character.')

if 'description' in request.data:
description = request.data['description']
else:
Expand Down