Skip to content

Add 'locate' endpoint to borrow API#9842

Merged
mekarpeles merged 3 commits into
internetarchive:masterfrom
SivanC:9829/feature/borrow-api-locate-action
Sep 5, 2024
Merged

Add 'locate' endpoint to borrow API#9842
mekarpeles merged 3 commits into
internetarchive:masterfrom
SivanC:9829/feature/borrow-api-locate-action

Conversation

@SivanC
Copy link
Copy Markdown
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.
Comment thread openlibrary/plugins/upstream/borrow.py Outdated
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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!

Comment thread openlibrary/plugins/upstream/models.py Outdated
Comment thread openlibrary/plugins/upstream/models.py Outdated
@mekarpeles
Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add new "locate" option to /borrow API endpoint

2 participants