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

Make /lists/add support POST with json body #8455

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

cdrini
Copy link
Collaborator

@cdrini cdrini commented Oct 25, 2023

To support Scott Hewitt's work, and to make consistent with our other APIs, POST /lists/add now supports a JSON body containing the list document. Previously it only supported URL parameters.

Technical

Testing

Tests added.

✅ You can test locally on the local host instance by running this in your browser console while logged in and on localhost:8080

await fetch('/lists/add', {
    method: 'POST',
    headers: {
        'Content-Type': "application/json",
    },
    body: JSON.stringify({
        name: 'foo data',
        description: 'bar',
        seeds: [{'key': '/books/OL1M'}, {'key': '/books/OL2M'}],
    }),
}).then(r => r.json())

Should create the list!

✅ You can also create one using url parameters, like before:

await fetch('/lists/add?name=hello&seeds=OL2M,OL3M', {
    method: 'POST',
    headers: {
        'Content-Type': "application/json",
    },
}).then(r => r.json())

Screenshot

Stakeholders

@cdrini cdrini force-pushed the fix/lists-add-json-body branch 4 times, most recently from 089fe72 to 7c0eaae Compare October 26, 2023 00:32
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2023

Codecov Report

Attention: 54 lines in your changes are missing coverage. Please review.

Comparison is base (a3a6529) 16.62% compared to head (c07c6c1) 16.44%.
Report is 2 commits behind head on master.

Files Patch % Lines
...enlibrary/plugins/openlibrary/js/handlers/index.js 0.00% 27 Missing and 7 partials ⚠️
openlibrary/plugins/openlibrary/js/list_books.js 0.00% 16 Missing ⚠️
openlibrary/plugins/openlibrary/js/index.js 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8455      +/-   ##
==========================================
- Coverage   16.62%   16.44%   -0.18%     
==========================================
  Files          85       86       +1     
  Lines        4416     4476      +60     
  Branches      767      783      +16     
==========================================
+ Hits          734      736       +2     
- Misses       3199     3246      +47     
- Partials      483      494      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cdrini cdrini force-pushed the fix/lists-add-json-body branch 9 times, most recently from 4f84b18 to 1c36a73 Compare October 27, 2023 08:47
@cdrini cdrini force-pushed the fix/lists-add-json-body branch 2 times, most recently from c07c6c1 to 4e1510f Compare November 24, 2023 18:56
@cdrini cdrini marked this pull request as ready for review November 24, 2023 20:45
@cdrini cdrini added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Nov 24, 2023
@mekarpeles mekarpeles self-assigned this Nov 27, 2023
@mekarpeles mekarpeles added the Priority: 2 Important, as time permits. [managed] label Nov 27, 2023
@mekarpeles mekarpeles merged commit 17982d9 into internetarchive:master Nov 30, 2023
3 checks passed
@mekarpeles
Copy link
Member

lgtm, reviewed w/ @cdrini

@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 2 Important, as time permits. [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants