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 Project Runeberg as a trusted book provider #9984

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

Freso
Copy link
Contributor

@Freso Freso commented Nov 3, 2024

Closes #9983

feature

Technical

NOTE: This depends on #9981 and as such builds on top of #9982 – hence the commit from there is in included in this PR currently. #9982 is a lot simpler than this PR though, so hopefully that will get merged before this one and this can be rebased on top of the main branch before getting undrafted.

This is mostly copy/pasting of existing support for Project Gutenberg, with a s/[gG]utenberg/[rR]uneberg/ replacement, but some things have had additional adjustment (like the download_options HTML page).

Testing

Screenshot

Stakeholders

Copy link
Contributor Author

@Freso Freso left a comment

Choose a reason for hiding this comment

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

A few things I need to fix later or that I need to ask about. 📝

@Freso
Copy link
Contributor Author

Freso commented Nov 11, 2024

Running this in a GitPod instance and trying to add the identifier paaski to a book and it seems like it doesn't save the identifier at all? It also seems to not save other identifiers, so probably not an issue with this PR(?), but I have no clue how to debug. The logs seem to suggest that the data (with identifier) gets POST’ed alright, but then… it just isn’t saved/stored?

web-1           | 0.06 (1): 200 POST /openlibrary.org/save_many {'query': '[{"title": "PAA SKI OVER GR\\u00d8NLAND", "authors": [{"author": {"key": "/authors/OL5A"}}], "k
ey": "/works/OL1W", "type": {"key": "/type/work"}, "latest_revision": 1, "revision": 1, "created": {"type": "/type/datetime", "value": "2024-11-11T13:59:43.093028"}, "las
t_modified": {"type": "/type/datetime", "value": "2024-11-11T13:59:43.093028"}, "description": null, "subjects": null, "subject_people": null, "subject_places": null, "su
bject_times": null, "subtitle": null, "excerpts": null, "links": null}, {"works": [{"key": "/works/OL1W"}], "title": "PAA SKI OVER GR\\u00d8NLAND", "publishers": ["Runebe
rg"], "publish_date": "2010", "key": "/books/OL5M", "type": {"key": "/type/edition"}, "latest_revision": 1, "revision": 1, "created": {"type": "/type/datetime", "value": 
"2024-11-11T13:59:43.093028"}, "last_modified": {"type": "/type/datetime", "value": "2024-11-11T13:59:43.093028"}, "identifiers": {"project_runeberg": ["paaski"]}, "class
ifications": {}, "weight": null, "table_of_contents": null, "contributors": [], "providers": [], "title_prefix": null, "subtitle": null, "publish_places": null, "copyrigh
t_date": null, "edition_name": null, "series": null, "by_statement": null, "languages": null, "translation_of": null, "translated_from": null, "description": null, "notes
": null, "other_titles": null, "physical_format": null, "number_of_pages": null, "pagination": null, "first_sentence": null, "roles": []}]', 'comment': '', 'action': 'edi
t-book', 'data': '{}'}

@RayBB
Copy link
Collaborator

RayBB commented Nov 12, 2024

Edit: I now see you were having the issue with books not authors. Let me take a look.
@Freso
Gitpod from master it working fine to add identifiers.
Gitpod from your branch is also working fine to add identifiers including Runeberg

Perhaps try stopping and starting the docker compose again?

image

@RayBB
Copy link
Collaborator

RayBB commented Nov 12, 2024

Adding runeburg IDs to a book worked (showing on the book page)
But the book page also shows
/openlibrary/openlibrary/templates/book_providers/runeberg_download_options.html: error in processing template: AttributeError: 'str' object has no attribute 'zip' (falling back to default template)

Which seems to be related to

<li><a href="https://runeberg.org/$runeberg_id.zip" title="$_('Download all scanned images from Project Runeberg')">$_("Scanned images")</a></li>

However, I don't know enough about your templating engine to fix it. Maybe @cdrini can chime in

@Freso Freso force-pushed the project_runeberg branch 2 times, most recently from bd6534b to 4cc2b64 Compare November 22, 2024 12:43
@Freso Freso marked this pull request as ready for review November 22, 2024 13:20
@Freso
Copy link
Contributor Author

Freso commented Nov 22, 2024

I’ve been able to test this now on GitPod, and it seems to work fine. My question in #9984 (comment) is still unanswered, though.

Also, #9982 hasn’t been reviewed (let alone merged) in the almost 3 weeks since these were opened; so just a heads-up that this also contains the commit from that PR (since this PR depends on the other PR, as mentioned in the OP). I was planning to undraft this when #9982 had been merged (so I could rebase on master with the #9982 commit), but going ahead and undrafting now. #9982 has been merged and this PR has been rebased on master.

This is mostly copy/pasting of existing support for Project Gutenberg,
with a `s/[gG]utenberg/[rR]uneberg/` replacement, but some things have
had additional adjustment (like the `download_options` HTML page).

Fixes internetarchive#9983
@mekarpeles
Copy link
Member

@Freso, this week can @scottbarnes and I meet with you and we can go through this together and get it unblocked + merged? There's a lot here we need to test in a local environment.

@mekarpeles mekarpeles added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Dec 16, 2024
@Freso
Copy link
Contributor Author

Freso commented Dec 17, 2024

Absolutely! I have been pretty much away from anything OL this last week, but let’s schedule something in Slack.

@mekarpeles mekarpeles merged commit e010b2a into internetarchive:master Dec 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 1 Do this week, receiving emails, time sensitive, . [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Project Runeberg as trusted book provider
5 participants