Skip to content
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

Provide csv export option for folio_set view #784

Merged
merged 2 commits into from
Sep 19, 2023

Conversation

dchiller
Copy link
Collaborator

@dchiller dchiller commented Sep 15, 2023

This PR:

  • adds CSVRenderer for use in Django Rest Framework responses.
  • implements the renderer at the existing /folio-set/manuscript/<manuscript-id> endpoint. Requests to this endpoint with a text/csv accept header will return a csv.

Copy link

@jacobdgm jacobdgm left a comment

Choose a reason for hiding this comment

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

This all looks reasonable!

In the PR initial comment, you say "Requests to this endpoint with a text/csv accept header will return a csv.`

Looking at the code, I see hints that this might happen (lines 13-14 in csv_renderer.py), but I don't really understand how this behavior is being effected. Is it a bit of Django REST Framework magic?

@dchiller
Copy link
Collaborator Author

Looking at the code, I see hints that this might happen (lines 13-14 in csv_renderer.py), but I don't really understand how this behavior is being effected. Is it a bit of Django REST Framework magic?

Yes, it is ... I had to learn more about how Django REST Framework does content negotiation, and it's one of those things where it's really nice that it works, but also really opaque. See additional comments on specific lines.

@@ -17,8 +17,10 @@


class ManuscriptFolioSetView(APIView):
serializer_class = SearchSerializer
renderer_classes = (JSONRenderer,)
renderer_classes = (
Copy link
Collaborator Author

@dchiller dchiller Sep 19, 2023

Choose a reason for hiding this comment

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

Here we define the available renderers with this view. So whereas previously we only JSONRenderer, behind the scenes, django rest framework will look at incoming requests and try to determine which of these two renderers (JSONRenderer or CSVRenderer) should be used for the request.

result).
"""

media_type = "text/csv"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, we say that the CSVRenderer should be used when the requested media type is "text/csv"...so when our ManuscriptFolioSetView is now determining which renderer to use, it will choose the CSVRenderer when the request is for "text/csv"

@dchiller dchiller merged commit a2e82a4 into DDMAL:main Sep 19, 2023
2 checks passed
@dchiller dchiller deleted the i-783-folio-mapping-csv-export branch September 19, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants