[Bug 31207] New: The OPAC Shelf browser fails to display local cover images
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Bug ID: 31207 Summary: The OPAC Shelf browser fails to display local cover images Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: a.roussos@dataly.gr QA Contact: testopia@bugs.koha-community.org In the OPAC Details page for a biblio, opening the Shelf browser will display adjacent items next to each other based on their call number and also display any cover images found. It's a very useful OPAC feature that allows librarians to have a "virtual" look at their library shelves. This is currently broken *when local covers are exclusively in use*, as it fails to fetch the related local cover images from the DB. Steps to reproduce: TL;DR: Just visit the sandbox URL below and see for yourself ;-) https://opac-shlfbrwsr.sandboxes.biblibre.eu/cgi-bin/koha/opac-detail.pl?biblionumber=344&shelfbrowse_itemnumber=1348#holdings Go to Home > Administration > System preferences in the Staff interface: - in section 'OPAC -> Shelf browser', make sure OPACShelfBrowser is enabled (i.e. set to 'Show') - in section 'Enhanced content -> Local or remote cover images', make sure both LocalCoverImages and OPACLocalCoverImages are enabled (i.e. set to 'Display') - make sure no other cover image provider is enabled - make sure OpacSeparateHoldings and OpacSeparateHoldingsBranch are set to 'Don't separate' and 'home library', respectively Next, in the Staff interface, find a record without any local cover images (either at the biblio or at the item level), having at least one item attached to it that has a Call number (the call number value can be anything -- it's only needed to trigger the display of the 'Browse shelf' hyperlink). Use the 'Images' tab in the Details page of the record to upload a local cover image at the biblio level (do *not* upload any item-level cover image). Finally, in the OPAC, visit the Details page of the record you just added the cover image to, and click on the 'Browse shelf' link found in the 'Call number' column. When the Shelf browser expands, you'll notice that there is no cover image displayed for the item(s) that belong to your test biblio. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #1 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 137950 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137950&action=edit Koha 21.05.16 - OPAC Shelf browser - Local covers OK -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #2 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 137951 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137951&action=edit Koha 21.11.00 - OPAC Shelf browser - Local covers Broken -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28180 --- Comment #3 from Andreas Roussos <a.roussos@dataly.gr> --- With regards to the display of local cover images, the OPAC Shelf browser works correctly in Koha versions < 21.11.00. It's also worth noting that the correct Shelf browser behaviour can be observed in all Koha versions (including master) when other cover image providers (Amazon, etc.) are in use (either on their own or in combination with the local cover images system preferences). The relevant commit that changed the Shelf browser's behaviour is fad97080e97da5bc77724c68b9394aba15518847 (from Bug 28180), where these lines were removed from opac-detail.tt: [% IF OPACLocalCoverImages %] KOHA.LocalCover.GetCoverFromBibnumber(true); [% IF itemloop_has_images OR oheritemloop_has_images %] KOHA.LocalCover.GetCoverFromItemnumber(true); [% END %] [% END %] Adding the above code to opac-detail.tt fixes the problem in master. However, if you look closely you'll notice the code has a small typo. The template parameter in the second [% IF %] conditional is named: oheritemloop_has_images vs. the correct parameter name: otheritemloop_has_images Because of that, the display of local cover images in the OPAC Shelf browser has never worked as intended when the SysPref OpacSeparateHoldings is set to 'Separate'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |a.roussos@dataly.gr Status|NEW |ASSIGNED --- Comment #4 from Andreas Roussos <a.roussos@dataly.gr> --- Self-assigning this as I'm preparing a patch that fixes both issues outlined above. I'm also going to include a Selenium unit test that will (hopefully) make it easier to catch Shelf browser-related bugs in the future. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28180 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28180 [Bug 28180] Use a lightbox gallery to display the images on the detail pages in OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #5 from Andreas Roussos <a.roussos@dataly.gr> --- (In reply to Andreas Roussos from comment #3)
Because of that, the display of local cover images in the OPAC Shelf browser has never worked as intended when the SysPref OpacSeparateHoldings is set to 'Separate'. Apologies, I just double-checked what I wrote above and it's not 100% valid. There is indeed a problem in master that relates to the display of local cover images in the OPAC when OpacSeparateHoldings is set to 'Separate', but it occurs at a different level and under specific conditions (and since it's not OPAC Shelf browser-specific, I think it'd be better if I filed a new bug report for it).
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #6 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 138010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138010&action=edit Bug 31207: Fix the display of local cover images in the OPAC Shelf browser In the OPAC Details page for a biblio, opening the Shelf browser results in a nice display of adjacent items next to each other based on their call number, which also shows any cover images found. Since 21.11.00 this has been broken when local covers are exclusively in use, as it fails to fetch the related images that have been uploaded *at the biblio level*. This patch fixes that by adding back TT code that was removed in commit fad97080e97da5bc77724c68b9394aba15518847 (Bug 28180). Test plan: 0) Under Home > Administration > System preferences: - in section 'OPAC -> Shelf browser', make sure OPACShelfBrowser is enabled (i.e. set to 'Show') - in section 'Enhanced content -> Local or remote cover images', make sure both LocalCoverImages and OPACLocalCoverImages are enabled (i.e. set to 'Show') - make sure no other cover image provider is enabled - make sure OpacSeparateHoldings and OpacSeparateHoldingsBranch are set to 'Don't separate' and 'home library', respectively 1) In the Staff interface, create a record that has: - a local cover image at the biblio level (use the 'Images' tab in the Details page of the record to upload a test image) - at least one item attached to it that has a Call number (the call number value can be anything -- it's only needed to trigger the display of the 'Browse shelf' hyperlink) 2) In the OPAC, visit the Details page of the record you just added the cover image to, and click on the 'Browse shelf' link found in the 'Call number' column. When the Shelf browser expands, you'll notice there's no cover image displayed for the item(s) that belong to your test biblio. 3) Apply the patch and refresh the OPAC biblio Details page. This time the shelf browser should display the local cover images associated with your test record. 4) Make sure the relevant unit test passes: prove -v t/db_dependent/ShelfBrowser.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #7 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 138011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138011&action=edit Bug 31207: Add Selenium test for the OPAC Shelf browser This adds a new Selenium test to automatically test the functionality of the OPAC Shelf browser with regards to the display of local cover images. This unit test has a somewhat narrow scope at the moment (i.e. it only checks if everything is working fine when local covers are in use), but can be expanded in the future to include OPAC Shelf browser tests for other cover image providers, etc. Also added are two sample images to be used by the test when uploading a local cover image. They come from the Unsplash website and have this license: https://unsplash.com/license Attribution to the image creators is given in the test code. Test plan: 1) Apply the other patch from this bug report, then run the new Selenium unit test which should pass without failures: cd t/db_dependent/selenium/ prove -v opacshelfbrowser.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #138011|0 |1 is obsolete| | --- Comment #8 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 138013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138013&action=edit Bug 31207: Add Selenium test for the OPAC Shelf browser This adds a new Selenium test to automatically test the functionality of the OPAC Shelf browser with regards to the display of local cover images. This unit test has a somewhat narrow scope at the moment (i.e. it only checks if everything is working fine when local covers are in use), but can be expanded in the future to include OPAC Shelf browser tests for other cover image providers, etc. Also added are two sample images to be used by the test when uploading a local cover image. They come from the Unsplash website and have this license: https://unsplash.com/license Attribution to the image creators is given in the test code. Test plan: 1) Apply the other patch from this bug report, then run the new Selenium unit test which should pass without failures: cd t/db_dependent/selenium/ prove -v opacshelfbrowser.t Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #9 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 138014 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138014&action=edit Bug 30471: Correct display of default rules To test: 1 - Browse to 'Administration->Circulation and fines rules' 2 - Make sure you are editing 'Standard rules for all libraries' 3 - Set 'Lost item fee refund on return policy' to 'Refund lost item charge and charge new overdue fine' 4 - Swtich to edit rules for a specific library, click on the dropdown for 'Lost item fee refund on return policy' and note default says 'Use default (Refund lost item charge and restore overdue fine)' 5 - Switch default/all libraries rule to 'Refund lost item charge and restore overdue fine' 6 - On specific branch note the default option is now 'Use default (Refund lost item charge and charge new overdue fine)' 7 - Apply patch and repeat, note the default descriptions now display correctly Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #10 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 138015 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138015&action=edit Bug 30458: Include librarian (manager_id) in accountline when using "Payout amount" button When using the "Payout amount" button, accountlines.manager_id is not populated. This is not the case with the "Issue payout" buttons. Test Plan: 1) Create a manual credit 2) Issue a payout using the "Payout amount" button 3) Inspect the payout accountline, note it has no manager_id 4) Apply this patch, restart all the things! 5) Repeat steps 1 and 2 6) Note the new payout accountline has a manager_id! Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> JK: Fix commit title Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #138010|0 |1 is obsolete| | --- Comment #11 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 138016 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138016&action=edit Bug 31207: Fix the display of local cover images in the OPAC Shelf browser In the OPAC Details page for a biblio, opening the Shelf browser results in a nice display of adjacent items next to each other based on their call number, which also shows any cover images found. Since 21.11.00 this has been broken when local covers are exclusively in use, as it fails to fetch the related images that have been uploaded *at the biblio level*. This patch fixes that by adding back TT code that was removed in commit fad97080e97da5bc77724c68b9394aba15518847 (Bug 28180). Test plan: 0) Under Home > Administration > System preferences: - in section 'OPAC -> Shelf browser', make sure OPACShelfBrowser is enabled (i.e. set to 'Show') - in section 'Enhanced content -> Local or remote cover images', make sure both LocalCoverImages and OPACLocalCoverImages are enabled (i.e. set to 'Show') - make sure no other cover image provider is enabled - make sure OpacSeparateHoldings and OpacSeparateHoldingsBranch are set to 'Don't separate' and 'home library', respectively 1) In the Staff interface, create a record that has: - a local cover image at the biblio level (use the 'Images' tab in the Details page of the record to upload a test image) - at least one item attached to it that has a Call number (the call number value can be anything -- it's only needed to trigger the display of the 'Browse shelf' hyperlink) 2) In the OPAC, visit the Details page of the record you just added the cover image to, and click on the 'Browse shelf' link found in the 'Call number' column. When the Shelf browser expands, you'll notice there's no cover image displayed for the item(s) that belong to your test biblio. 3) Apply the patch and refresh the OPAC biblio Details page. This time the shelf browser should display the local cover images associated with your test record. 4) Make sure the relevant unit test passes: prove -v t/db_dependent/ShelfBrowser.t Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #138013|0 |1 is obsolete| | --- Comment #12 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 138017 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138017&action=edit Bug 31207: Add Selenium test for the OPAC Shelf browser This adds a new Selenium test to automatically test the functionality of the OPAC Shelf browser with regards to the display of local cover images. This unit test has a somewhat narrow scope at the moment (i.e. it only checks if everything is working fine when local covers are in use), but can be expanded in the future to include OPAC Shelf browser tests for other cover image providers, etc. Also added are two sample images to be used by the test when uploading a local cover image. They come from the Unsplash website and have this license: https://unsplash.com/license Attribution to the image creators is given in the test code. Test plan: 1) Apply the other patch from this bug report, then run the new Selenium unit test which should pass without failures: cd t/db_dependent/selenium/ prove -v opacshelfbrowser.t Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk Status|Needs Signoff |Signed Off --- Comment #13 from Sally <sally.healey@cheshiresharedservices.gov.uk> --- Sorry for the noise - I messed up on the number of patches to sign off from the sandbox. :( But the patch works great - appreciate the solid test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #14 from Andreas Roussos <a.roussos@dataly.gr> --- (In reply to Sally from comment #13)
Sorry for the noise - I messed up on the number of patches to sign off from the sandbox. :(
But the patch works great - appreciate the solid test plan. No worries, I've marked the non-relevant comments as obsolete. Many thanks for your feedback and the speedy sign off!
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #138014|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #138015|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Hi Andreas, The test is failing for me: kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/selenium/opacshelfbrowser.t t/db_dependent/selenium/opacshelfbrowser.t .. # No tests run! t/db_dependent/selenium/opacshelfbrowser.t .. 1/3 # Failed test 'No tests run for subtest "OPAC Shelf browser - Check display of local cover images (OpacSeparateHoldings set to "Don't separate")"' # at t/db_dependent/selenium/opacshelfbrowser.t line 137. upload_file: no such file: ./opacshelfbrowser-test-image1.jpg at t/db_dependent/selenium/opacshelfbrowser.t line 120. Also: + # test image by Kourosh Qaffari on Unsplash (https://unsplash.com/photos/RrhhzitYizg) The Unspash license does not seem GPLv3 compatible: https://unsplash.com/license To avoid an unnecessary discussion about the license (which is not at all the goal here :D), and to prevent a 50kb to be added to the git repo, shouldn't we go with 1px images? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I would also create a "data" directory, not at the same level of the other .t files. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #17 from Andreas Roussos <a.roussos@dataly.gr> --- (In reply to Jonathan Druart from comment #15)
Hi Andreas, Hi Jonathan!
The test is failing for me:
kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/selenium/opacshelfbrowser.t t/db_dependent/selenium/opacshelfbrowser.t .. # No tests run! t/db_dependent/selenium/opacshelfbrowser.t .. 1/3 # Failed test 'No tests run for subtest "OPAC Shelf browser - Check display of local cover images (OpacSeparateHoldings set to "Don't separate")"' # at t/db_dependent/selenium/opacshelfbrowser.t line 137. upload_file: no such file: ./opacshelfbrowser-test-image1.jpg at t/db_dependent/selenium/opacshelfbrowser.t line 120. I hardcoded a relative path for the location of the JPGs to upload, which is why the test only ran properly if you changed directory to t/db_dependent/selenium/ first. I've now made adjustments to the test
Also: + # test image by Kourosh Qaffari on Unsplash (https://unsplash.com/photos/RrhhzitYizg) The Unspash license does not seem GPLv3 compatible: https://unsplash.com/license
To avoid an unnecessary discussion about the license (which is not at all the goal here :D), and to prevent a 50kb to be added to the git repo, shouldn't we go with 1px images? I agree, let's try that. I tried uploading a 1x1 pixel black PNG but could not get it to work (it might have to do with the resizing that is applied to local cover images when thumbnails are generated). A 2x2 pixel black PNG worked fine though, so I suggest we go with
I appreciate your feedback -- please see my replies inline below: script so that it will run just fine when called from any directory. that instead. The 2x2 PNG file is only 72 bytes long ;-) (In reply to Jonathan Druart from comment #16)
I would also create a "data" directory, not at the same level of the other .t files. Hmmm, there's actually a "data" directory inside t/db_dependent which contains all sorts of files used by other tests. In my revised patches I've chosen to add my sample PNG in there.
Please let me know if you'd like us to handle this differently. Thank you very much for your patience with this bug report! Apologies for taking so long to reply. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #138016|0 |1 is obsolete| | Attachment #138017|0 |1 is obsolete| | --- Comment #18 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 145805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145805&action=edit Bug 31207: Fix the display of local cover images in the OPAC Shelf browser This patch adds back Template Toolkit and JavaScript code that was removed in commit fad97080e97da5bc77724c68b9394aba15518847 (Bug 28180), thus fixing the problem of Shelf browser images not being loaded in the OPAC Details page for a biblio. This feature stopped working in v21.11.00, and these conditions must be met to be able to reproduce the problem: a) Exclusive use of local cover images, and b) Covers uploaded at the biblio level only. Test plan: 0) Configure the following System Preferences: - OPACShelfBrowser => 'Show' - LocalCoverImages => 'Show' - OPACLocalCoverImages => 'Show' (ensure no other cover image provider is enabled) - OpacSeparateHoldings => 'Don't separate' - OpacSeparateHoldingsBranch => 'home library' 1) Create a bibliographic record that has: - a local cover image at the biblio level (use the 'Images' tab in the Details page of the record to upload a test image) - a single item with a Call number (the Call number value can be anything, it's only needed to trigger the Shelf browser) 2) In the OPAC, visit the Details page of the record you just added, and click on the 'Browse shelf' link found in the 'Call number' column of the 'Holdings' tab. When the Shelf browser expands, there will be no cover image displayed. 3) Apply the patch and refresh the page with CTRL-F5. The Shelf browser will now display the local cover image associated with your test record. 4) Make sure the relevant unit test passes: prove -v t/db_dependent/ShelfBrowser.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #19 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 145806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145806&action=edit Bug 31207: Add a Selenium test for the OPAC Shelf browser This adds a new Selenium test to automatically test the functionality of the OPAC Shelf browser with regards to the display of local cover images. This unit test has a somewhat narrow scope at the moment (i.e. it only checks if everything is working fine when local covers are in use), but can be expanded in the future to include OPAC Shelf browser tests for other cover image providers, etc. Also added is a sample 2x2 pixel image to be used by the test when uploading a local cover image. Test plan: 1) Apply the other patch from this bug report, then run the new Selenium unit test which should pass without failures: prove -v t/db_dependent/selenium/opacshelfbrowser.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #20 from Andreas Roussos <a.roussos@dataly.gr> --- The first patch in this patchset has been rebased for the current master. The commit message and test plan have been reworded as well -- hopefully for the better! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- + [% 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. 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 The selenium test could also cover this situation :) Does that make sense? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 pierre.genty@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pierre.genty@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hagud@orex.es --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 33366 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #23 from Lucas Gass <lucas@bywatersolutions.com> --- *** Bug 35858 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #24 from Lucas Gass <lucas@bywatersolutions.com> --- Is this still being worked on? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #25 from Andreas Roussos <a.roussos@dataly.gr> --- (In reply to Lucas Gass from comment #24)
Is this still being worked on? Hi, Lucas!
I'm really sorry for the lack of any response on this bug :-( The TL;DR version is that it's all due to bad communication on my part (I should have asked for further clarifications before setting off to rewrite my patch). I actually tried to follow-up on this after Jonathan's latest feedback in Comment 21. However, in my revised patch I had to modify more than just localcovers.js and opac-image.pl since I couldn't figure out how to implement the suggested fallback parameter. So, I left this idle for a while until I could fix it properly and then, unfortunately, it fell under the radar. I'll ask for Jonathan's advice ASAP. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #26 from Andreas Roussos <a.roussos@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 ;-)
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
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 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #27 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Andreas, Will you still be working on this? Is there anything we can do to help you get it over the line? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #28 from Andreas Roussos <a.roussos@dataly.gr> --- (In reply to Lucas Gass (lukeg) from comment #27)
Andreas,
Will you still be working on this? Is there anything we can do to help you get it over the line? Hi Lucas!
Unfortunately this has fallen under the radar... However, I'm going to attend the Hackfest in Marseille in about a week's time, and hope there will be an opportunity there to work with other people to move this forward. Thank you for your patience. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 --- Comment #29 from Jonathan Druart <jonathan.druart@gmail.com> --- Shouldn't we adjust GetNearbyItems to return the imagenumber instead? That way we could build a "img" tag easily in the template. We would: * get rid of the JS's GetCoverFromBibnumber * remove unecessary GET opac-images when images do not exist (matching what we did already in opac-search, see bug 35558) * follow what we have done on bug 38981 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31207 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|a.roussos@dataly.gr |oleonard@myacpl.org --- Comment #30 from Andreas Roussos <a.roussos@dataly.gr> --- Sorry, but as I don't have time to work on this I've decided to reset the assignee to the default. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org