[Koha-bugs] [Bug 31207] The OPAC Shelf browser fails to display local cover images

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207

--- Comment #26 from Andreas Roussos <a.roussos at dataly.gr> ---
(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.