https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207
--- Comment #26 from Andreas Roussos <[email protected]> --- (In reply to Jonathan Druart from comment #21) Hi, Jonathan! I'm sorry for the (much) belated reply. As per my previous message to Lucas, I realise this bug has been left lingering for too long. I'm keen to move this forward, so please see my comments inline below: > + [% IF itemloop_has_images OR otheritemloop_has_images %] > + KOHA.LocalCover.GetCoverFromItemnumber(true); > + [% END %] > > > This is not correct, that's not what we want here. > > You don't want to get the image at the item level if the *current* biblio > has image at the item level. Fair point, I agree. To decide which local cover image to display for the items shown in the shelf browser we shouldn't look at the current biblio but rather handle each item independently. In fact, I think that the algorithm should operate as follows: 1) If only a biblio-level image exists: => show that for all biblio items 2) If only item-level images exist: => show the respective image for each item 3) If both biblio-level and item-level images exist: => show the item-level image for the items that have one => fall back to the biblio-level image for all other items > If you want to implement that you need to: > 1. Create a new JS function in localcovers.js (say > GetCoverFromItemnumberOrBibnumber) that would call opac-image.pl with a new > parameter (say biblio-fallback=1) > 2. Modify opac-image.pl to retrieve this parameter and return > $item->cover_image || $biblio->cover_image I'm afraid this is the part that I don't understand fully ;-) When would this new biblio-fallback parameter be set? And how would it work in conjunction with opac-image.pl? As per the DESCRIPTION section of opac-image.pl, the script currently accepts an imagenumber or a biblionumber, returning a random image if the latter was provided (side note: I think it's not actually 100% random but rather the first cover image): 72 elsif ( defined $biblionumber ) { 73 my $biblio = Koha::Biblios->find($biblionumber); 74 unless ($biblio) { 75 print $input->redirect("/cgi-bin/koha/errors/404.pl"); 76 exit; 77 } 78 my $cover_images = $biblio->cover_images; 79 if ( $cover_images->count ) { 80 $image = $cover_images->next; 81 } 82 } AIUI, unless we modify opac-image.pl to also accept an itemnumber parameter we have no way of implementing the cover image selection algorithm I described above. To do this I actually had to also modify opac-detail.tt, shelfbrowser.inc, and C4/ShelfBrowser.pm. > The selenium test could also cover this situation :) OK, I reckon that should be easy to implement since we already have a test action that uploads an item-level local cover image. What's left to be done is to write some more tests to check if the correct local cover image is returned in each situation. > Does that make sense? I think what confused me in the past (not your fault!) was that you mentioned two changes that needed to be made (localcovers.js and opac-image.pl), and because I had to change more files I considered my revised patch a failure. Apologies for not asking for your feedback earlier. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
