Bug 35647 - Coce images won't load on the staff results page
Summary: Coce images won't load on the staff results page
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Lucas Gass (lukeg)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-22 21:39 UTC by Lucas Gass (lukeg)
Modified: 2024-04-18 13:35 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 35647: Use callback when waiting for Coce to load (2.41 KB, patch)
2023-12-22 21:49 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 35647: Use callback when waiting for Coce to load (2.45 KB, patch)
2023-12-23 06:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 35647: (follow-up) Ensure Coce is on (850 bytes, patch)
2024-01-29 21:18 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2023-12-22 21:39:27 UTC
To recreate:

1. Turn on all CoceProviders and IntranetCoce.
2. Set CoceHost to 'https://coce.bywatersolutions.com'
3. Do a search and notice that no Coce images load on the results page.
4. In intranet-tmpl/js/coce.js add a debug line inside the `$("[id^=coce-thumbnail]").each(function() {` loop.
5. Nothing happens! This div is being prematurely removed via intranet-tmpl/prog/js/pages/results.js

I don't think it is safe to remove the Coce div's without first knowing that KOHA.coce is finished. In results.js:

102 $(window).load(function() {
103     verify_cover_images();
104 });


Just using a window load is not good enough. We either need to not remove the div ( hiding it is safer ) or we need to use a callback, promise, or async/await.
Comment 1 Lucas Gass (lukeg) 2023-12-22 21:45:38 UTC
There is a wait_for_images() callback used elsewhere, it seems like we need it here too.
Comment 2 Lucas Gass (lukeg) 2023-12-22 21:49:27 UTC
Created attachment 160279 [details] [review]
Bug 35647: Use callback when waiting for Coce to load

1. Turn on all CoceProviders and IntranetCoce.
2. Set CoceHost to 'https://coce.bywatersolutions.com'
3. Do a search and notice that no Coce images load on the results page.
4. Apply patch, clear browser cache and try again.
5. Images should load.
Comment 3 David Nind 2023-12-23 06:57:05 UTC
Created attachment 160280 [details] [review]
Bug 35647: Use callback when waiting for Coce to load

1. Turn on all CoceProviders and IntranetCoce.
2. Set CoceHost to 'https://coce.bywatersolutions.com'
3. Do a search and notice that no Coce images load on the results page.
4. Apply patch, clear browser cache and try again.
5. Images should load.

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Jonathan Druart 2024-01-25 11:13:42 UTC
If Coce is disabled you are going to wait 5 seconds, always!
Comment 5 Lucas Gass (lukeg) 2024-01-29 21:18:12 UTC
Created attachment 161616 [details] [review]
Bug 35647: (follow-up) Ensure Coce is on
Comment 6 Marcel de Rooy 2024-04-18 13:35:12 UTC
    if (!loaded && counter_wait < 50) {// Do not wait more than 5 seconds
        window.setTimeout(function(){wait_for_images(cb);}, 100);
    } else {
        if (counter_wait >= 50 ) {
            console.log("Could not retrieve the images")
        }
        cb();
    }
Shouldnt you differentiate loaded is true and !loaded && counter_wait > 50 ?
Now you always go the callback?

$(window).load(function() {
    if ( PREF_IntranetCoce && PREF_CoceProviders ) {
        wait_for_images(verify_cover_images);
    }
});
What about the other source of images? Amazon or whatever? Is that condition really correct?