Bug 26037

Summary: openlibrary.org is hit on every Koha requests
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: OPACAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: major    
Priority: P5 - low CC: aleisha, amitddng135, dcook, heather_hernandez, lucas, martin.renvoize
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24201
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03
Attachments: Bug 26037: openlibrary.org is hit on every Koha requests
Bug 26037: openlibrary.org is hit on every Koha requests
Bug 26037: openlibrary.org is hit on every Koha requests

Description Jonathan Druart 2020-07-22 08:51:15 UTC
If OpenLibrary sysprefs are enabled, a query is done on every page we load.
If the Koha page is not opac-detail or opac-result, a 404 is returned (the URL is not correctly generated).

To recreate: 
Turn on OpenLibraryCovers and OpenLibrarySearch
Hit the Koha main page
Note you have a 404 in the console:
https://openlibrary.org/api/volumes/brief/json?callback=jQuery34106125725859943835_1595407548302&show_all_items=true&_=1595407548303
Comment 1 Jonathan Druart 2020-07-22 08:52:36 UTC
We should either test the existence of ol_readapi_book, or move the inclusion of openlibrary.js to opac-detail and opac-result
Comment 2 Jonathan Druart 2020-07-22 08:53:54 UTC
To confirm:
1. Is staff impacted as well?
2. What about the other services we include in opac-bottom.inc?
Comment 3 Martin Renvoize 2020-07-22 09:16:42 UTC
I was a little shocked to see how much we include in -bottom.inc when I recently QA'd something.. I feel like we should really only load certain libraries where we actually use them and only when we reach a certain threshold should they be moved to a more system wide load location like that opac-bottom.inc.

So.. my feeling here would be to move them into just opac-detail and opac-result.
Comment 4 Martin Renvoize 2020-07-22 09:51:56 UTC
See bug 24201 comment 59; Although in that case we're conditionally loading based on a template variable, I felt it would be clearer to just load the Asset via the Asset plugin on the page rather than set a variable and have the asset loaded via an include.
Comment 5 David Cook 2020-07-23 04:24:32 UTC
This one does drive me a bit nuts... so it would be great to see a fix.
Comment 6 Owen Leonard 2020-07-24 19:55:35 UTC
The section of openlibrary.js related to the Read API seems to be to blame, but I'm not sure why. This raises the question... Has this feature ever worked?
Comment 7 Jonathan Druart 2020-07-27 09:49:10 UTC
(In reply to Owen Leonard from comment #6)
> The section of openlibrary.js related to the Read API seems to be to blame,
> but I'm not sure why. This raises the question... Has this feature ever
> worked?

Yes, it currently works: https://snipboard.io/Ya7SbK.jpg
It's the "Read now" button.
Comment 8 Owen Leonard 2020-07-27 12:25:11 UTC
(In reply to Jonathan Druart from comment #7)
> Yes, it currently works: https://snipboard.io/Ya7SbK.jpg

Thanks Jonathan, try as I might I couldn't find a title which would show me the button! Now I see it.
Comment 9 Owen Leonard 2020-07-27 14:07:45 UTC
Created attachment 107429 [details] [review]
Bug 26037: openlibrary.org is hit on every Koha requests

This patch moves the inclusion of Open Library JavaScript from
opac-bottom.inc to the three templates where it might be used.

In addition, the Open Library JS has been modified to check for the
existence of the relevant markup on the page before executing a query.

Unrelated: The Open Library label is now hidden when the status returned
is "Not found." Previously this information was displayed.

To test, apply the patch and enable the OpenLibrarySearch system
preference.

 - Perform a search in the OPAC which will return results from Open
   Library. Some ISBNs: 0140423826, 0395082552, 0679436405.
   - On the search result page there should be an Open Library "Read
     now" button for each result that can be opened for reading in Open
     Library.
 - Add these titles to a list and view the list.
   - The "Read now" button should appear on the list page.
 - View the details for one of these titles.
   - The "Read now" button should appear on the page below the holdings.
 - Watch the browser's developer tools on these and other pages in the
   OPAC. There should be no errors in the console and no 404 errors in
   the network tab.
Comment 10 Lucas Gass 2020-07-31 22:08:07 UTC
Created attachment 107658 [details] [review]
Bug 26037: openlibrary.org is hit on every Koha requests

This patch moves the inclusion of Open Library JavaScript from
opac-bottom.inc to the three templates where it might be used.

In addition, the Open Library JS has been modified to check for the
existence of the relevant markup on the page before executing a query.

Unrelated: The Open Library label is now hidden when the status returned
is "Not found." Previously this information was displayed.

To test, apply the patch and enable the OpenLibrarySearch system
preference.

 - Perform a search in the OPAC which will return results from Open
   Library. Some ISBNs: 0140423826, 0395082552, 0679436405.
   - On the search result page there should be an Open Library "Read
     now" button for each result that can be opened for reading in Open
     Library.
 - Add these titles to a list and view the list.
   - The "Read now" button should appear on the list page.
 - View the details for one of these titles.
   - The "Read now" button should appear on the page below the holdings.
 - Watch the browser's developer tools on these and other pages in the
   OPAC. There should be no errors in the console and no 404 errors in
   the network tab.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Katrin Fischer 2020-08-05 15:36:03 UTC
Created attachment 107842 [details] [review]
Bug 26037: openlibrary.org is hit on every Koha requests

This patch moves the inclusion of Open Library JavaScript from
opac-bottom.inc to the three templates where it might be used.

In addition, the Open Library JS has been modified to check for the
existence of the relevant markup on the page before executing a query.

Unrelated: The Open Library label is now hidden when the status returned
is "Not found." Previously this information was displayed.

To test, apply the patch and enable the OpenLibrarySearch system
preference.

 - Perform a search in the OPAC which will return results from Open
   Library. Some ISBNs: 0140423826, 0395082552, 0679436405.
   - On the search result page there should be an Open Library "Read
     now" button for each result that can be opened for reading in Open
     Library.
 - Add these titles to a list and view the list.
   - The "Read now" button should appear on the list page.
 - View the details for one of these titles.
   - The "Read now" button should appear on the page below the holdings.
 - Watch the browser's developer tools on these and other pages in the
   OPAC. There should be no errors in the console and no 404 errors in
   the network tab.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Jonathan Druart 2020-08-06 09:38:17 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 13 Lucas Gass 2020-08-14 21:22:52 UTC
backported to 20.05.x for 20.05.03
Comment 14 Aleisha Amohia 2020-08-16 22:42:35 UTC
does not apply cleanly on 19.11.x. please rebase if needed.