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 LCP support #279

Merged
merged 3 commits into from
Sep 24, 2020

Conversation

vbessonov
Copy link
Contributor

@vbessonov vbessonov commented Aug 25, 2020

Description

This PR adds support for LCP license files (application/vnd.readium.lcp.license.v1.0+json) returned by Circulation Manager during fulfilment of LCP protected books.

Linked PRs:

@vbessonov
Copy link
Contributor Author

@hokei, could you review this PR?

@kristojorg
Copy link
Member

@vbessonov While this will allow a button labeled "LCP License" to show in the UI, I don't think this is the right approach for LCP content, as it doesn't mean anything to the user. How does LCP content work? Does the user download a license file? What do they do with it from there?

Copy link
Member

@kristojorg kristojorg left a comment

Choose a reason for hiding this comment

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

I think we need a better understanding of how this should work from a user perspective before we support LCP files in the web client

@vbessonov
Copy link
Contributor Author

@kristojorg, you can find a detailed description here.

Also, you can download an LCP book from the LCP testbed (you can find its description and credentials in the Lyrasis wiki). Once you download a book, you can notice that it doesn't have an extension and because of that it cannot be opened in Thorium or other LCP-compatible client applications without manually adding an extension. This PR force opds-web-client to add .lcpl extension to the LCP books

Copy link
Member

@kristojorg kristojorg left a comment

Choose a reason for hiding this comment

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

Approving this because it matches our level of support for ACSM. In the future we should show users how to deal with these files, though

@EdwinGuzman
Copy link
Member

@vbessonov I can merge this in but can you update the version in packages/opds-web-client/package.json and also update packages/opds-web-client/changelog?

@vbessonov
Copy link
Contributor Author

@EdwinGuzman, sure, should I increase the minor version or the patch one?

@EdwinGuzman
Copy link
Member

I think a patch one is fine for this.

@vbessonov
Copy link
Contributor Author

@EdwinGuzman, sure, will do. Could you tell me how I should update version in package-lock.json?
I tried to use npm update and npm install --package-lock-only but none of them worked. Can I update package-lock.json manually?

@EdwinGuzman
Copy link
Member

You can update it manually if you update the version in package.json, then delete package-lock.json, and then run npm install again.

@vbessonov
Copy link
Contributor Author

@EdwinGuzman, I tried to delete package-lock.json and then run npm install but it didn't work, new package-lock.json for some reason wasn't created. So I ended up with manually updating the version in package-lock.json, I hope it won't break anything

Copy link
Member

@EdwinGuzman EdwinGuzman left a comment

Choose a reason for hiding this comment

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

Great, thanks.

@EdwinGuzman EdwinGuzman merged commit 57da39c into NYPL-Simplified:master Sep 24, 2020
@EdwinGuzman
Copy link
Member

Just published 0.5.7 to npm.

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.

3 participants