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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Taking a look now. |
rajadain
approved these changes
Mar 30, 2020
| 'name': source, | ||
| } | ||
| request = self.context.get('request') \ | ||
| if self.context is not None else None |
There was a problem hiding this comment.
Can this be shortened to
if self.context else None?
There was a problem hiding this comment.
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')
...
>>>| } | ||
| request = self.context.get('request') \ | ||
| if self.context is not None else None | ||
| user = request.user if request is not None else None |
There was a problem hiding this comment.
An explicit None check is less helpful here, but I have opted to use the same construction for consistency.
The complex permission and visibility checks required make fetching contributor details for each facility slow, so we are cutting the maximum results per page from 500 to 50. Contributor names and list names can have commas, so we join them with a vertical bar in the CSV to make machine parsing of multiple contributors possible.
We are using the | character to separate contributors in CSV downloads so we want to prevent using that character in names of contributors or lists.
We know the total number of facilities so it is possible to show percent progress. We have opted for a linear progress bar rather than a circular one because it looks better in this situation where our progress jumps rather than smoothly increases.
jwalgran
force-pushed
the
feature/jcw/download-affilations
branch
from
1853ac6
to
0ade4fb
Compare
|
Thanks for the review. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Return contributor details from list API and include in CSV download.
Connects #997
Demo
Notes
The complex permission and visibility checks required make fetching contributor details for each facility slow, so we are cutting the maximum results per page from 500 to 50.
Contributor names and list names can have commas, so we join them with a vertical bar in the CSV to make machine parsing of multiple contributors possible. We add validations that prevent contributors from including the vertical bar in their name or their list names.
We know the total number of facilities so it is possible to show percent progress. We have opted for a linear progress bar rather than a circular one because it looks better in this situation where our progress jumps rather than smoothly increases.
Testing Instructions
./scripts/resetdbChecklist
fixup!commits have been squashed