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

Add 'locate' endpoint to borrow API #9842

Conversation

SivanC
Copy link
Contributor

@SivanC SivanC commented Sep 3, 2024

Closes #9829

Feature. Adds a new endpoint to redirect to a worldcat page based on identifier data provided for a given edition. Prioritizes oclc, then isbn 13, isbn 10, and finally title.

Technical

Adds an extra check to the plugins/upstream/borrow.py borrow class. If the endpoint /books/{olid}/{title}/borrow is reached with the parameter action=locate, a redirect URL is constructed and redirected to.

Testing

  • Navigate to an edition page
  • Add /borrow?action=locate to the end of the url
  • Be redirected to worldcat, ensuring that the identifier with the highest priority has been chosen (oclc, isbn13, isbn10, title). If the edition has an oclc identifier, you should be directed to a book page, otherwise the search page.

Screenshot

Stakeholders

@mekarpeles

Add a new endpoint to redirect to a worldcat page based on identifier data provided for a given edition. Prioritizes oclc, then isbn 13, isbn 10, and finally title.
if edition.get('oclc_numbers', None):
redirect_url += f'title/{edition.oclc_numbers[0]}'
elif edition.get('isbn_13', edition.get('isbn_10', None)):
redirect_url += f'isbn/{edition.isbn_13[0] or edition.isbn_10[0]}'
Copy link
Member

Choose a reason for hiding this comment

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

This is problematic because in line 205 we check whether isbn13 or isbn10 is available... But consider the case where there is an isbn10 but not an isbn13

In line 206, even though there's no isbn13, we're still trying to access the 0th element which doesn't exist.

Copy link
Member

Choose a reason for hiding this comment

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

Also, /isbn converts to /search on worldcat which isn't ideal, but it is what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, good catch!

@mekarpeles mekarpeles added Needs: Testing On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing labels Sep 5, 2024
@mekarpeles
Copy link
Member

@mekarpeles mekarpeles merged commit 7b330d6 into internetarchive:master Sep 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Testing On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new "locate" option to /borrow API endpoint
2 participants