Bug 22445

Summary: Ability to define a URL for custom cover images
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: CatalogingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: new feature    
Priority: P5 - low CC: black23, hagud, hayleypelham, joonas.kylmala, katrin.fischer, kyle, m.de.rooy, magnus, martin.renvoize, mbugz2, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This development adds the ability to use alternative cover art providers who provide covers openly via consistent URLs. Three new system preferences are introduced, `CustomCoverImagesURL`, `CustomCoverImages` and `OPACCustomCoverImages`.
Version(s) released in:
19.11.00
Bug Depends on:    
Bug Blocks: 26921, 28709, 29172    
Attachments: Bug 22445: Add new prefs *CustomCoverImages*
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url
Bug 22445: Allow marc fields as pattern
Bug 22445: Custom cover images - opac shelfbrowser
Bug 22445: Custom cover images - opac results
Bug 22445: Custom cover images - opac detail
Bug 22445: Custom cover images - opac result (rss)
Bug 22445: Custom cover images - opac shelves
Bug 22445: Custom cover images - opac reviews
Bug 22445: Custom cover images - opac reviews (rss)
Bug 22445: Custom cover images - opac user (checkout list)
Bug 22445: Custom cover images - detail (staff)
Bug 22445: Custom cover images - search resulta (staff)
Bug 22445: Custom cover images - search resulta (staff)
Bug 22455: (follow-up) Use CSS to set a max-width for images - OPAC
Bug 22455: (follow-up) Use CSS to set a max-width for images - Staff client
Bug 22445: Add new pattern %normalized_isbn%
Bug 22445: Add new prefs *CustomCoverImages*
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url
Bug 22445: Allow marc fields as pattern
Bug 22445: Custom cover images - opac shelfbrowser
Bug 22445: Custom cover images - opac results
Bug 22445: Custom cover images - opac detail
Bug 22445: Custom cover images - opac result (rss)
Bug 22445: Custom cover images - opac shelves
Bug 22445: Custom cover images - opac reviews
Bug 22445: Custom cover images - opac reviews (rss)
Bug 22445: Custom cover images - opac user (checkout list)
Bug 22445: Custom cover images - detail (staff)
Bug 22445: Custom cover images - search resulta (staff)
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client
Bug 22445: Add new pattern %normalized_isbn%
Bug 22445: Add new prefs *CustomCoverImages*
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url
Bug 22445: Allow marc fields as pattern
Bug 22445: Custom cover images - opac shelfbrowser
Bug 22445: Custom cover images - opac results
Bug 22445: Custom cover images - opac detail
Bug 22445: Custom cover images - opac result (rss)
Bug 22445: Custom cover images - opac shelves
Bug 22445: Custom cover images - opac reviews
Bug 22445: Custom cover images - opac reviews (rss)
Bug 22445: Custom cover images - opac user (checkout list)
Bug 22445: Custom cover images - detail (staff)
Bug 22445: Custom cover images - search resulta (staff)
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client
Bug 22445: Add new pattern %normalized_isbn%
Bug 22445: Add new prefs *CustomCoverImages*
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url
Bug 22445: Allow marc fields as pattern
Bug 22445: Custom cover images - opac shelfbrowser
Bug 22445: Custom cover images - opac results
Bug 22445: Custom cover images - opac detail
Bug 22445: Custom cover images - opac result (rss)
Bug 22445: Custom cover images - opac shelves
Bug 22445: Custom cover images - opac reviews
Bug 22445: Custom cover images - opac reviews (rss)
Bug 22445: Custom cover images - opac user (checkout list)
Bug 22445: Custom cover images - detail (staff)
Bug 22445: Custom cover images - search resulta (staff)
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client
Bug 22445: Add new pattern %normalized_isbn%
Bug 22445: Replace %% with {}
Bug 22445: Add new prefs *CustomCoverImages*
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url
Bug 22445: Allow marc fields as pattern
Bug 22445: Custom cover images - opac shelfbrowser
Bug 22445: Custom cover images - opac results
Bug 22445: Custom cover images - opac detail
Bug 22445: Custom cover images - opac result (rss)
Bug 22445: Custom cover images - opac shelves
Bug 22445: Custom cover images - opac reviews
Bug 22445: Custom cover images - opac reviews (rss)
Bug 22445: Custom cover images - opac user (checkout list)
Bug 22445: Custom cover images - detail (staff)
Bug 22445: Custom cover images - search resulta (staff)
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client
Bug 22445: Add new pattern %normalized_isbn%
Bug 22445: Replace %% with {}

Description Jonathan Druart 2019-03-01 22:54:30 UTC
So far we have cover images from a list of webservices, COCE, locally, but we cannot define a custom url.
Comment 1 Jonathan Druart 2019-03-01 23:11:57 UTC
Created attachment 85925 [details] [review]
Bug 22445: Add new prefs *CustomCoverImages*

This patch set adds a new method to display cover images for
bibliographic records.
The idea is to provide a flexible way allowing several use cases:
- local path
- remote url
- webservice
which would follow a given pattern.

So far only the following patterns are available:
- %isbn% will be replaced byt the isbn of the record
- %issn%
- using MARC subfields:
  %024$a% to replace with the value in 024$a

For example
- /your/local/images/%isbn%.png
- https://covers.openlibrary.org/b/isbn/%isbn%-M.jpb
- etc.

There are 3 new sysprefs:
CustomCoverImagesURL
=> Define the path/url
CustomCoverImages
=> Turn on/off the display of the custom cover image in the staff client
OPACCustomCoverImages
=> Turn on/off the display of the custom cover image at the OPAC

Sponsored-by: Orex Digital
Comment 2 Jonathan Druart 2019-03-01 23:12:03 UTC
Created attachment 85926 [details] [review]
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital
Comment 3 Jonathan Druart 2019-03-01 23:12:07 UTC
Created attachment 85927 [details] [review]
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital
Comment 4 Jonathan Druart 2019-03-01 23:12:11 UTC
Created attachment 85928 [details] [review]
Bug 22445: Allow marc fields as pattern

%tag$field%

There is a limitation here, only one replacement will work.
Let see if it is needed later to support several patterns

Sponsored-by: Orex Digital
Comment 5 Jonathan Druart 2019-03-01 23:12:14 UTC
Created attachment 85929 [details] [review]
Bug 22445: Custom cover images - opac shelfbrowser

Test plan:
Go to the detail page of a bibliographic record (at the OPAC)
and confirm the at custom cover images are displayed in the shelf
browser

Sponsored-by: Orex Digital
Comment 6 Jonathan Druart 2019-03-01 23:12:19 UTC
Created attachment 85930 [details] [review]
Bug 22445: Custom cover images - opac results

Test plan:
Search for bibliographic records at the OPAC, the custom cover should be
displayed

Sponsored-by: Orex Digital
Comment 7 Jonathan Druart 2019-03-01 23:12:23 UTC
Created attachment 85931 [details] [review]
Bug 22445: Custom cover images - opac detail

Test plan:
Go to the bibliographic record detail page at the OPAC, you should see
the custom cover image.

Sponsored-by: Orex Digital
Comment 8 Jonathan Druart 2019-03-01 23:12:27 UTC
Created attachment 85932 [details] [review]
Bug 22445: Custom cover images - opac result (rss)

This cannot be tested, it seems that the template is broken.

Sponsored-by: Orex Digital
Comment 9 Jonathan Druart 2019-03-01 23:12:31 UTC
Created attachment 85933 [details] [review]
Bug 22445: Custom cover images - opac shelves

Test plan:
Add a bibliographic record (that has a custom image) to a shelf,
view the shelf (OPAC).
You should see the image.

Sponsored-by: Orex Digital
Comment 10 Jonathan Druart 2019-03-01 23:12:35 UTC
Created attachment 85934 [details] [review]
Bug 22445: Custom cover images - opac reviews

Test plan:
- enable OpacShowRecentComments
- make a comment (OPAC)
- approve it (staff)
- click 'Recent comments' (OPAC)
=> You should see the custom cover

Sponsored-by: Orex Digital
Comment 11 Jonathan Druart 2019-03-01 23:12:41 UTC
Created attachment 85935 [details] [review]
Bug 22445: Custom cover images - opac reviews (rss)

Test plan:
Same as previous patch, then click on the feeds link

Sponsored-by: Orex Digital
Comment 12 Jonathan Druart 2019-03-01 23:12:44 UTC
Created attachment 85936 [details] [review]
Bug 22445: Custom cover images - opac user (checkout list)

Test plan:
- Check an item out to a patron
- At the OPAC loggin as this patron
On the checkout list you should see the cover image.

Sponsored-by: Orex Digital
Comment 13 Jonathan Druart 2019-03-01 23:12:48 UTC
Created attachment 85937 [details] [review]
Bug 22445: Custom cover images - detail (staff)

Test plan:
Staff side now, go the detail page of a bibliographic record
You should see the cover image.

Sponsored-by: Orex Digital
Comment 14 Jonathan Druart 2019-03-01 23:12:53 UTC
Created attachment 85938 [details] [review]
Bug 22445: Custom cover images - search resulta (staff)

Test plan:
Search for bibliographic records, the custom covers should be displayed.

Sponsored-by: Orex Digital
Comment 15 Hayley Pelham 2019-03-05 22:46:57 UTC
These patches don't seem to work fully as expected for me, as outlined below.

When clicking Edit on the bibliographic record details page and uploading a custom image, all test plans pass except for:
- OPAC reviews (Comment #11 rss - template is still broken it seems), 
- OPAC reviews (Comment #10 Recent Comments), 
- OPAC user (Comment #12 checkout list). 
In the case of the latter two scenarios, instead of displaying the image or the 'no cover image' placeholder image, a link that says 'Cover image' is displayed.

In terms of the url/local path options in the system prefs, is there anything that needs to be done apart from allowing display, setting the path/url and ensuring that the biblio has a url that is present in the path/url? I'm having trouble testing this (I'm pretty new)

Additionally, in most places where the cover image is displayed (such as the item detail page, etc.), the 'Cover image' link is also displayed underneath. I'm assuming this is a bug?
Comment 16 Katrin Fischer 2019-03-06 06:29:18 UTC
Hi Haley, 'Failed QA' is ok for getting answers to your questions. Thx for testing!
Comment 17 Jonathan Druart 2019-03-06 13:50:47 UTC
(In reply to Hayley Mapley from comment #15)
> These patches don't seem to work fully as expected for me, as outlined below.
> 
> When clicking Edit on the bibliographic record details page and uploading a
> custom image, all test plans pass except for:
> - OPAC reviews (Comment #11 rss - template is still broken it seems), 
> - OPAC reviews (Comment #10 Recent Comments), 
> - OPAC user (Comment #12 checkout list). 
> In the case of the latter two scenarios, instead of displaying the image or
> the 'no cover image' placeholder image, a link that says 'Cover image' is
> displayed.
> 
> In terms of the url/local path options in the system prefs, is there
> anything that needs to be done apart from allowing display, setting the
> path/url and ensuring that the biblio has a url that is present in the
> path/url? I'm having trouble testing this (I'm pretty new)
> 
> Additionally, in most places where the cover image is displayed (such as the
> item detail page, etc.), the 'Cover image' link is also displayed
> underneath. I'm assuming this is a bug?

Hello Hayley,
Did you fill CustomCoverImagesURL in? What value?
You are not supposed to attach an image to the record (this is the LocalCoverImages feature) neither add "an url to a biblio" (I did not understand that part in your comment).

To test easily you can use "https://covers.openlibrary.org/b/isbn/%isbn%-M.jpg"
(and not .jpb as written in the test plan, sorry it's a typo!)

Then the feature will behave the same as OpenLibraryCovers (it will pick the same covers).
Comment 18 Hayley Pelham 2019-03-06 20:22:47 UTC
(In reply to Jonathan Druart from comment #17)
> (In reply to Hayley Mapley from comment #15)
> > These patches don't seem to work fully as expected for me, as outlined below.
> > 
> > When clicking Edit on the bibliographic record details page and uploading a
> > custom image, all test plans pass except for:
> > - OPAC reviews (Comment #11 rss - template is still broken it seems), 
> > - OPAC reviews (Comment #10 Recent Comments), 
> > - OPAC user (Comment #12 checkout list). 
> > In the case of the latter two scenarios, instead of displaying the image or
> > the 'no cover image' placeholder image, a link that says 'Cover image' is
> > displayed.
> > 
> > In terms of the url/local path options in the system prefs, is there
> > anything that needs to be done apart from allowing display, setting the
> > path/url and ensuring that the biblio has a url that is present in the
> > path/url? I'm having trouble testing this (I'm pretty new)
> > 
> > Additionally, in most places where the cover image is displayed (such as the
> > item detail page, etc.), the 'Cover image' link is also displayed
> > underneath. I'm assuming this is a bug?
> 
> Hello Hayley,
> Did you fill CustomCoverImagesURL in? What value?
> You are not supposed to attach an image to the record (this is the
> LocalCoverImages feature) neither add "an url to a biblio" (I did not
> understand that part in your comment).
> 
> To test easily you can use
> "https://covers.openlibrary.org/b/isbn/%isbn%-M.jpg"
> (and not .jpb as written in the test plan, sorry it's a typo!)
> 
> Then the feature will behave the same as OpenLibraryCovers (it will pick the
> same covers).

Gah! As confusingly written as my comment was (and excluding the fact that I was mistakenly also testing the local cover images), I was simply missing the -M part of that URL which was stopping it from working.
Comment 19 Hayley Pelham 2019-03-06 20:25:18 UTC
Created attachment 86214 [details] [review]
Bug 22445: Custom cover images - search resulta (staff)

Test plan:
Search for bibliographic records, the custom covers should be displayed.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
All test plans pass using URL in CustomcoverImages syspref
Comment 20 Hugo Agud 2019-03-12 11:27:16 UTC
I have checked all test plan and it works fine, JUST the image size is the original image size, then if the thumbnail is bigger the image appear big... it can be fixed by css for sure.. but perhaps some kind of code limitation? 

it is just a suggestions
Comment 21 Jonathan Druart 2019-03-14 21:52:23 UTC
(In reply to Hugo Agud from comment #20)
> I have checked all test plan and it works fine, JUST the image size is the
> original image size, then if the thumbnail is bigger the image appear big...
> it can be fixed by css for sure.. but perhaps some kind of code limitation? 
> 
> it is just a suggestions

It seems that there is no homogeneity in the different image covers. At first glance it seems that the size is not taken into account for others either, except for adlibris.
We should deal with that problem globally, but on a separate bug report of course as it will complexity this patch set a lot.
Comment 22 Owen Leonard 2019-03-18 17:22:40 UTC
(In reply to Jonathan Druart from comment #21)

> We should deal with that problem globally, but on a separate bug report of
> course as it will complexity this patch set a lot.

All other cover image sources provide a generally-acceptable image size for all cover images. With local cover images, we provide the option for users to upload images but then we have Koha generate a thumbnail. For this feature to all any size image to be displayed is inconsistent with what we have now.

I'm going to submit patches to add classes to the display of cover images and add CSS to specify a maximum width for these covers.
Comment 23 Owen Leonard 2019-03-18 17:31:23 UTC
Created attachment 86726 [details] [review]
Bug 22455: (follow-up) Use CSS to set a max-width for images - OPAC

This patch adds some CSS and markup to OPAC templates so that custom
cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

To test, apply the patch and regenerate the OPAC CSS. Post an oversized
image which corresponds to a record in your OPAC which can be added to a
user's checkouts and to a list.

View the following pages to confirm that the image displays well:

 - OPAC search results
 - Bibliographic detail page
 - Shelf browser
 - List contents
 - Recent comments
 - "Your summary"
Comment 24 Owen Leonard 2019-03-18 17:51:05 UTC
Created attachment 86727 [details] [review]
Bug 22455: (follow-up) Use CSS to set a max-width for images - Staff client

This patch adds some CSS and markup to staff client templates so that
custom cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

This patch also adds a missing </a> to fix display problems.

To test, apply the patch and regenerate the staff client CSS. Post an
oversized image which corresponds to a record in your catalog.

View the following pages to confirm that the image displays well:

 - Catalog search results
 - Bibliographic detail page
Comment 25 Owen Leonard 2019-03-18 18:25:57 UTC
I have a concern which I think may be a blocker for this feature: All other cover image sources use a "normalized_isbn" variable which is provided to the templates. This feature doesn't use a normalized ISBN, so a library might upload an image file with a name like this:

"0553573403.png"

...and find that Koha is trying to load an image with this file name:

"0553573403 | 9780553573404 (pbk.).png"

I think it would be more consistent to have custom cover images use a normalized ISBN too.
Comment 26 Hugo Agud 2019-04-01 12:11:19 UTC
(In reply to Owen Leonard from comment #25)
> I have a concern which I think may be a blocker for this feature: All other
> cover image sources use a "normalized_isbn" variable which is provided to
> the templates. This feature doesn't use a normalized ISBN, so a library
> might upload an image file with a name like this:
> 
> "0553573403.png"
> 
> ...and find that Koha is trying to load an image with this file name:
> 
> "0553573403 | 9780553573404 (pbk.).png"
> 
> I think it would be more consistent to have custom cover images use a
> normalized ISBN too.

Not all books have ISBN for example historical books have not any kind of ISBN, even it should be great to be able to use non isbn and issn fields like 024.

I think force a normalized ISBN will reduce the potential value of it
Comment 27 Hugo Agud 2019-04-01 12:12:11 UTC
I have applied the patch, it appears the class Cover Image, but src is emtpy then no image is published
Comment 28 Owen Leonard 2019-04-01 12:42:02 UTC
(In reply to Hugo Agud from comment #26)

> > I think it would be more consistent to have custom cover images use a
> > normalized ISBN too.
> 
> Not all books have ISBN for example historical books have not any kind of
> ISBN

I'm not suggesting that the feature should be limited to only normalized ISBN. I'm saying that if ISBN is selected, normalized ISBN should be provided.
Comment 29 Jonathan Druart 2019-04-05 01:34:25 UTC
Created attachment 87434 [details] [review]
Bug 22445: Add new pattern %normalized_isbn%

Quite bad to use C4::Koha from Koha::Biblio :-/
Comment 30 Jonathan Druart 2019-04-05 01:35:23 UTC
(In reply to Owen Leonard from comment #28)
> I'm saying that if ISBN is selected, normalized ISBN should be
> provided.

Done, thanks for the follow-ups and the suggestion Owen!
Comment 31 Maria 2019-04-12 11:49:03 UTC
Tried to test. Test plan is divided into pieces and difficult to follow.

When we turned on the new sysprefs (CustomCoverImagesURL etc) the result was that we got a few images.

The images that were added in result list had a link to the image itself, not to the OPAC record. 

Would be good with new consolidated test plan.
Comment 32 Jonathan Druart 2019-04-24 02:03:14 UTC
(In reply to Maria from comment #31)
Hello Maria,

Thanks for testing this!

> Tried to test. Test plan is divided into pieces and difficult to follow.

Yes there is one test plan per patch. The goal is to tell what individual patch does.

> When we turned on the new sysprefs (CustomCoverImagesURL etc) the result was
> that we got a few images.

Noting is displayed if the cover does not exist, which isbn did you try?

> The images that were added in result list had a link to the image itself,
> not to the OPAC record. 

IIRC it is the behavior with other cover services.
Comment 33 Michal Denar 2019-05-22 19:15:46 UTC
Can you rebase this on master, please?

Thank you.
Comment 34 Jonathan Druart 2019-05-23 02:40:12 UTC
Created attachment 89973 [details] [review]
Bug 22445: Add new prefs *CustomCoverImages*

This patch set adds a new method to display cover images for
bibliographic records.
The idea is to provide a flexible way allowing several use cases:
- local path
- remote url
- webservice
which would follow a given pattern.

So far only the following patterns are available:
- %isbn% will be replaced byt the isbn of the record
- %issn%
- using MARC subfields:
  %024$a% to replace with the value in 024$a

For example
- /your/local/images/%isbn%.png
- https://covers.openlibrary.org/b/isbn/%isbn%-M.jpb
- etc.

There are 3 new sysprefs:
CustomCoverImagesURL
=> Define the path/url
CustomCoverImages
=> Turn on/off the display of the custom cover image in the staff client
OPACCustomCoverImages
=> Turn on/off the display of the custom cover image at the OPAC

Sponsored-by: Orex Digital
Comment 35 Jonathan Druart 2019-05-23 02:40:17 UTC
Created attachment 89974 [details] [review]
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital
Comment 36 Jonathan Druart 2019-05-23 02:40:22 UTC
Created attachment 89975 [details] [review]
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital
Comment 37 Jonathan Druart 2019-05-23 02:40:26 UTC
Created attachment 89976 [details] [review]
Bug 22445: Allow marc fields as pattern

%tag$field%

There is a limitation here, only one replacement will work.
Let see if it is needed later to support several patterns

Sponsored-by: Orex Digital
Comment 38 Jonathan Druart 2019-05-23 02:40:30 UTC
Created attachment 89977 [details] [review]
Bug 22445: Custom cover images - opac shelfbrowser

Test plan:
Go to the detail page of a bibliographic record (at the OPAC)
and confirm the at custom cover images are displayed in the shelf
browser

Sponsored-by: Orex Digital
Comment 39 Jonathan Druart 2019-05-23 02:40:34 UTC
Created attachment 89978 [details] [review]
Bug 22445: Custom cover images - opac results

Test plan:
Search for bibliographic records at the OPAC, the custom cover should be
displayed

Sponsored-by: Orex Digital
Comment 40 Jonathan Druart 2019-05-23 02:40:39 UTC
Created attachment 89979 [details] [review]
Bug 22445: Custom cover images - opac detail

Test plan:
Go to the bibliographic record detail page at the OPAC, you should see
the custom cover image.

Sponsored-by: Orex Digital
Comment 41 Jonathan Druart 2019-05-23 02:40:43 UTC
Created attachment 89980 [details] [review]
Bug 22445: Custom cover images - opac result (rss)

This cannot be tested, it seems that the template is broken.

Sponsored-by: Orex Digital
Comment 42 Jonathan Druart 2019-05-23 02:40:48 UTC
Created attachment 89981 [details] [review]
Bug 22445: Custom cover images - opac shelves

Test plan:
Add a bibliographic record (that has a custom image) to a shelf,
view the shelf (OPAC).
You should see the image.

Sponsored-by: Orex Digital
Comment 43 Jonathan Druart 2019-05-23 02:40:52 UTC
Created attachment 89982 [details] [review]
Bug 22445: Custom cover images - opac reviews

Test plan:
- enable OpacShowRecentComments
- make a comment (OPAC)
- approve it (staff)
- click 'Recent comments' (OPAC)
=> You should see the custom cover

Sponsored-by: Orex Digital
Comment 44 Jonathan Druart 2019-05-23 02:40:57 UTC
Created attachment 89983 [details] [review]
Bug 22445: Custom cover images - opac reviews (rss)

Test plan:
Same as previous patch, then click on the feeds link

Sponsored-by: Orex Digital
Comment 45 Jonathan Druart 2019-05-23 02:41:01 UTC
Created attachment 89984 [details] [review]
Bug 22445: Custom cover images - opac user (checkout list)

Test plan:
- Check an item out to a patron
- At the OPAC loggin as this patron
On the checkout list you should see the cover image.

Sponsored-by: Orex Digital
Comment 46 Jonathan Druart 2019-05-23 02:41:06 UTC
Created attachment 89985 [details] [review]
Bug 22445: Custom cover images - detail (staff)

Test plan:
Staff side now, go the detail page of a bibliographic record
You should see the cover image.

Sponsored-by: Orex Digital
Comment 47 Jonathan Druart 2019-05-23 02:41:10 UTC
Created attachment 89986 [details] [review]
Bug 22445: Custom cover images - search resulta (staff)

Test plan:
Search for bibliographic records, the custom covers should be displayed.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
All test plans pass using URL in CustomcoverImages syspref
Comment 48 Jonathan Druart 2019-05-23 02:41:15 UTC
Created attachment 89987 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC

This patch adds some CSS and markup to OPAC templates so that custom
cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

To test, apply the patch and regenerate the OPAC CSS. Post an oversized
image which corresponds to a record in your OPAC which can be added to a
user's checkouts and to a list.

View the following pages to confirm that the image displays well:

 - OPAC search results
 - Bibliographic detail page
 - Shelf browser
 - List contents
 - Recent comments
 - "Your summary"
Comment 49 Jonathan Druart 2019-05-23 02:41:19 UTC
Created attachment 89988 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client

This patch adds some CSS and markup to staff client templates so that
custom cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

This patch also adds a missing </a> to fix display problems.

To test, apply the patch and regenerate the staff client CSS. Post an
oversized image which corresponds to a record in your catalog.

View the following pages to confirm that the image displays well:

 - Catalog search results
 - Bibliographic detail page
Comment 50 Jonathan Druart 2019-05-23 02:41:24 UTC
Created attachment 89989 [details] [review]
Bug 22445: Add new pattern %normalized_isbn%

Quite bad to use C4::Koha from Koha::Biblio :-/
Comment 51 Michal Denar 2019-05-23 08:24:50 UTC
Created attachment 89996 [details] [review]
Bug 22445: Add new prefs *CustomCoverImages*

This patch set adds a new method to display cover images for
bibliographic records.
The idea is to provide a flexible way allowing several use cases:
- local path
- remote url
- webservice
which would follow a given pattern.

So far only the following patterns are available:
- %isbn% will be replaced byt the isbn of the record
- %issn%
- using MARC subfields:
  %024$a% to replace with the value in 024$a

For example
- /your/local/images/%isbn%.png
- https://covers.openlibrary.org/b/isbn/%isbn%-M.jpb
- etc.

There are 3 new sysprefs:
CustomCoverImagesURL
=> Define the path/url
CustomCoverImages
=> Turn on/off the display of the custom cover image in the staff client
OPACCustomCoverImages
=> Turn on/off the display of the custom cover image at the OPAC

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 52 Michal Denar 2019-05-23 08:24:54 UTC
Created attachment 89997 [details] [review]
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 53 Michal Denar 2019-05-23 08:24:58 UTC
Created attachment 89998 [details] [review]
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 54 Michal Denar 2019-05-23 08:25:02 UTC
Created attachment 89999 [details] [review]
Bug 22445: Allow marc fields as pattern

%tag$field%

There is a limitation here, only one replacement will work.
Let see if it is needed later to support several patterns

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 55 Michal Denar 2019-05-23 08:25:07 UTC
Created attachment 90000 [details] [review]
Bug 22445: Custom cover images - opac shelfbrowser

Test plan:
Go to the detail page of a bibliographic record (at the OPAC)
and confirm the at custom cover images are displayed in the shelf
browser

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 56 Michal Denar 2019-05-23 08:25:11 UTC
Created attachment 90001 [details] [review]
Bug 22445: Custom cover images - opac results

Test plan:
Search for bibliographic records at the OPAC, the custom cover should be
displayed

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 57 Michal Denar 2019-05-23 08:25:16 UTC
Created attachment 90002 [details] [review]
Bug 22445: Custom cover images - opac detail

Test plan:
Go to the bibliographic record detail page at the OPAC, you should see
the custom cover image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 58 Michal Denar 2019-05-23 08:25:20 UTC
Created attachment 90003 [details] [review]
Bug 22445: Custom cover images - opac result (rss)

This cannot be tested, it seems that the template is broken.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 59 Michal Denar 2019-05-23 08:25:24 UTC
Created attachment 90004 [details] [review]
Bug 22445: Custom cover images - opac shelves

Test plan:
Add a bibliographic record (that has a custom image) to a shelf,
view the shelf (OPAC).
You should see the image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 60 Michal Denar 2019-05-23 08:25:29 UTC
Created attachment 90005 [details] [review]
Bug 22445: Custom cover images - opac reviews

Test plan:
- enable OpacShowRecentComments
- make a comment (OPAC)
- approve it (staff)
- click 'Recent comments' (OPAC)
=> You should see the custom cover

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 61 Michal Denar 2019-05-23 08:25:33 UTC
Created attachment 90006 [details] [review]
Bug 22445: Custom cover images - opac reviews (rss)

Test plan:
Same as previous patch, then click on the feeds link

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 62 Michal Denar 2019-05-23 08:25:37 UTC
Created attachment 90007 [details] [review]
Bug 22445: Custom cover images - opac user (checkout list)

Test plan:
- Check an item out to a patron
- At the OPAC loggin as this patron
On the checkout list you should see the cover image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 63 Michal Denar 2019-05-23 08:25:44 UTC
Created attachment 90009 [details] [review]
Bug 22445: Custom cover images - detail (staff)

Test plan:
Staff side now, go the detail page of a bibliographic record
You should see the cover image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 64 Michal Denar 2019-05-23 08:25:48 UTC
Created attachment 90010 [details] [review]
Bug 22445: Custom cover images - search resulta (staff)

Test plan:
Search for bibliographic records, the custom covers should be displayed.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
All test plans pass using URL in CustomcoverImages syspref
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 65 Michal Denar 2019-05-23 08:25:57 UTC
Created attachment 90011 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC

This patch adds some CSS and markup to OPAC templates so that custom
cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

To test, apply the patch and regenerate the OPAC CSS. Post an oversized
image which corresponds to a record in your OPAC which can be added to a
user's checkouts and to a list.

View the following pages to confirm that the image displays well:

 - OPAC search results
 - Bibliographic detail page
 - Shelf browser
 - List contents
 - Recent comments
 - "Your summary"

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 66 Michal Denar 2019-05-23 08:26:02 UTC
Created attachment 90012 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client

This patch adds some CSS and markup to staff client templates so that
custom cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

This patch also adds a missing </a> to fix display problems.

To test, apply the patch and regenerate the staff client CSS. Post an
oversized image which corresponds to a record in your catalog.

View the following pages to confirm that the image displays well:

 - Catalog search results
 - Bibliographic detail page

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 67 Michal Denar 2019-05-23 08:26:06 UTC
Created attachment 90013 [details] [review]
Bug 22445: Add new pattern %normalized_isbn%

Quite bad to use C4::Koha from Koha::Biblio :-/

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 68 Kyle M Hall 2019-07-26 17:52:01 UTC
This
Comment 69 Kyle M Hall 2019-07-26 17:53:14 UTC
This looks pretty good, but I have one request; can you use Template Toolkit syntax instead of yet another bespoke syntax? If we standardize on TT for all templating we'll make everyone's lives much simpler.
Comment 70 Jonathan Druart 2019-08-03 23:21:26 UTC
Hi Kyle,
Are you suggestion to replace
  https://my_url/%isbn%_%issn%.png
with
  https://my_url/[% biblio.biblioitem.isbn %]_[% biblio.issn %].png
?

It could work, even if a bit less readable.

However what about %normalized_isbn% and the %field.subfield% patterns? They will be hard (or impossible) to write using the TT syntax.
Comment 71 Katrin Fischer 2019-09-02 06:19:02 UTC
(In reply to Kyle M Hall from comment #69)
> This looks pretty good, but I have one request; can you use Template Toolkit
> syntax instead of yet another bespoke syntax? If we standardize on TT for
> all templating we'll make everyone's lives much simpler.

I am not sure about that tbh, because the TT syntax is hard for users. With the notices we still have trouble because of lack of documentation, lack of 'knowing the right variable names'. There is also a security risk we discussed in other spots, as it allows you to use regex and similar and possibly do harm with it.

That said, I think this is stuck on Jonathan's question about the MARC syntax. Would it be possible to move this forward as is and treat the TT as possible enhancement later on? Or allow TT syntax additionally?
Comment 72 Martin Renvoize 2019-10-08 12:26:40 UTC
With the plethora of schemes out there I must admit I'm not entirely comfortable with the choice of `%` character being used:

> Of the ASCII character set, the characters : / ? # [ ] @ are reserved for use as delimiters of the generic URI components and must be percent-encoded – for example, %3F for a question mark.[15] The characters ! $ & ' ( ) * + , ; = are permitted by generic URI syntax to be used unencoded in the user information, host, and path as delimiters.[10][16] Additionally, : and @ may appear unencoded within the path, query, and fragment; and ? and / may appear unencoded as data within the query or fragment.

So, given the above quote, one could find that we are misinterpreting % in our regex when it's being used as an escape character.

I'm not too worried about whether we mimic TT syntax here or not so long as what we do offer is well documented alongside the preference itself.
Comment 73 Jonathan Druart 2019-10-15 09:35:04 UTC
(In reply to Martin Renvoize from comment #72)
> With the plethora of schemes out there I must admit I'm not entirely
> comfortable with the choice of `%` character being used:
> 
> > Of the ASCII character set, the characters : / ? # [ ] @ are reserved for use as delimiters of the generic URI components and must be percent-encoded – for example, %3F for a question mark.[15] The characters ! $ & ' ( ) * + , ; = are permitted by generic URI syntax to be used unencoded in the user information, host, and path as delimiters.[10][16] Additionally, : and @ may appear unencoded within the path, query, and fragment; and ? and / may appear unencoded as data within the query or fragment.
> 
> So, given the above quote, one could find that we are misinterpreting % in
> our regex when it's being used as an escape character.
> 
> I'm not too worried about whether we mimic TT syntax here or not so long as
> what we do offer is well documented alongside the preference itself.

Which character would you suggest then? :)

The substitutions are like: $url =~ s|%normalized_isbn%|$normalized_isbn|g
So whichever we pick should be safe.
Comment 74 Tomás Cohen Arazi 2019-10-15 13:58:51 UTC
(In reply to Kyle M Hall from comment #69)
> This looks pretty good, but I have one request; can you use Template Toolkit
> syntax instead of yet another bespoke syntax? If we standardize on TT for
> all templating we'll make everyone's lives much simpler.

I personally prefer the bash-like syntax :-P

https://my_url/${normalized_isbn}.png
Comment 75 Martin Renvoize 2019-10-15 14:05:30 UTC
Happy with bash style.. or just plain moustache templating style might be even simpler for end users.

https://my_url/{normalized_isbn}.png
Comment 76 Martin Renvoize 2019-10-15 14:09:38 UTC
Comment on attachment 90013 [details] [review]
Bug 22445: Add new pattern %normalized_isbn%

Review of attachment 90013 [details] [review]:
-----------------------------------------------------------------

Switching to `{` and `}` for the delimiters and adding the  `%field.subfield%` into this doc line would get it past the PQA line for me.. be great to see this feature in community :)

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/enhanced_content.pref
@@ +346,4 @@
>              - "Using the following URL:"
>              - pref: CustomCoverImagesURL
>                class: url
> +            - "You can define it using the following patterns: %isbn%, %issn%, %normalized_isbn%."

Could we also document the ` %field.subfield%` option here :)
Comment 77 Katrin Fischer 2019-10-15 18:05:27 UTC
I like {} too, it's also used in other prefs like SearchForTitleIn.
Comment 78 Katrin Fischer 2019-10-15 18:06:26 UTC
Switching to FQA meaning "Waiting for follow-up".
Comment 79 Tomás Cohen Arazi 2019-10-15 18:49:17 UTC
I agree, 3 out of 4 commenters have agreed on {}. Let's get this into master ASAP!
Comment 80 Jonathan Druart 2019-10-16 07:16:53 UTC
Created attachment 94227 [details] [review]
Bug 22445: Add new prefs *CustomCoverImages*

This patch set adds a new method to display cover images for
bibliographic records.
The idea is to provide a flexible way allowing several use cases:
- local path
- remote url
- webservice
which would follow a given pattern.

So far only the following patterns are available:
- {isbn} will be replaced byt the isbn of the record
- {issn}
- using MARC subfields:
  {024$a} to replace with the value in 024$a
(Note that the selector has been changed from %% to {} in a coming
patch)

For example
- /your/local/images/{isbn}.png
- https://covers.openlibrary.org/b/isbn/{isbn}-M.jpb
- etc.

There are 3 new sysprefs:
CustomCoverImagesURL
=> Define the path/url
CustomCoverImages
=> Turn on/off the display of the custom cover image in the staff client
OPACCustomCoverImages
=> Turn on/off the display of the custom cover image at the OPAC

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 81 Jonathan Druart 2019-10-16 07:16:57 UTC
Created attachment 94228 [details] [review]
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 82 Jonathan Druart 2019-10-16 07:17:02 UTC
Created attachment 94229 [details] [review]
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 83 Jonathan Druart 2019-10-16 07:17:06 UTC
Created attachment 94230 [details] [review]
Bug 22445: Allow marc fields as pattern

%tag$field%

There is a limitation here, only one replacement will work.
Let see if it is needed later to support several patterns

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 84 Jonathan Druart 2019-10-16 07:17:11 UTC
Created attachment 94231 [details] [review]
Bug 22445: Custom cover images - opac shelfbrowser

Test plan:
Go to the detail page of a bibliographic record (at the OPAC)
and confirm the at custom cover images are displayed in the shelf
browser

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 85 Jonathan Druart 2019-10-16 07:17:17 UTC
Created attachment 94232 [details] [review]
Bug 22445: Custom cover images - opac results

Test plan:
Search for bibliographic records at the OPAC, the custom cover should be
displayed

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 86 Jonathan Druart 2019-10-16 07:17:23 UTC
Created attachment 94233 [details] [review]
Bug 22445: Custom cover images - opac detail

Test plan:
Go to the bibliographic record detail page at the OPAC, you should see
the custom cover image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 87 Jonathan Druart 2019-10-16 07:17:28 UTC
Created attachment 94234 [details] [review]
Bug 22445: Custom cover images - opac result (rss)

This cannot be tested, it seems that the template is broken.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 88 Jonathan Druart 2019-10-16 07:17:33 UTC
Created attachment 94235 [details] [review]
Bug 22445: Custom cover images - opac shelves

Test plan:
Add a bibliographic record (that has a custom image) to a shelf,
view the shelf (OPAC).
You should see the image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 89 Jonathan Druart 2019-10-16 07:17:38 UTC
Created attachment 94236 [details] [review]
Bug 22445: Custom cover images - opac reviews

Test plan:
- enable OpacShowRecentComments
- make a comment (OPAC)
- approve it (staff)
- click 'Recent comments' (OPAC)
=> You should see the custom cover

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 90 Jonathan Druart 2019-10-16 07:17:42 UTC
Created attachment 94237 [details] [review]
Bug 22445: Custom cover images - opac reviews (rss)

Test plan:
Same as previous patch, then click on the feeds link

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 91 Jonathan Druart 2019-10-16 07:17:46 UTC
Created attachment 94238 [details] [review]
Bug 22445: Custom cover images - opac user (checkout list)

Test plan:
- Check an item out to a patron
- At the OPAC loggin as this patron
On the checkout list you should see the cover image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 92 Jonathan Druart 2019-10-16 07:17:51 UTC
Created attachment 94239 [details] [review]
Bug 22445: Custom cover images - detail (staff)

Test plan:
Staff side now, go the detail page of a bibliographic record
You should see the cover image.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 93 Jonathan Druart 2019-10-16 07:17:56 UTC
Created attachment 94240 [details] [review]
Bug 22445: Custom cover images - search resulta (staff)

Test plan:
Search for bibliographic records, the custom covers should be displayed.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
All test plans pass using URL in CustomcoverImages syspref
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 94 Jonathan Druart 2019-10-16 07:18:01 UTC
Created attachment 94241 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC

This patch adds some CSS and markup to OPAC templates so that custom
cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

To test, apply the patch and regenerate the OPAC CSS. Post an oversized
image which corresponds to a record in your OPAC which can be added to a
user's checkouts and to a list.

View the following pages to confirm that the image displays well:

 - OPAC search results
 - Bibliographic detail page
 - Shelf browser
 - List contents
 - Recent comments
 - "Your summary"

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 95 Jonathan Druart 2019-10-16 07:18:06 UTC
Created attachment 94242 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client

This patch adds some CSS and markup to staff client templates so that
custom cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

This patch also adds a missing </a> to fix display problems.

To test, apply the patch and regenerate the staff client CSS. Post an
oversized image which corresponds to a record in your catalog.

View the following pages to confirm that the image displays well:

 - Catalog search results
 - Bibliographic detail page

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 96 Jonathan Druart 2019-10-16 07:18:11 UTC
Created attachment 94243 [details] [review]
Bug 22445: Add new pattern %normalized_isbn%

Quite bad to use C4::Koha from Koha::Biblio :-/

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 97 Jonathan Druart 2019-10-16 07:18:17 UTC
Created attachment 94244 [details] [review]
Bug 22445: Replace %% with {}
Comment 98 Jonathan Druart 2019-10-16 07:19:12 UTC
Thanks everybody for your feedback, much appreciated!
Comment 99 Martin Renvoize 2019-10-22 13:59:42 UTC
Created attachment 94559 [details] [review]
Bug 22445: Add new prefs *CustomCoverImages*

This patch set adds a new method to display cover images for
bibliographic records.
The idea is to provide a flexible way allowing several use cases:
- local path
- remote url
- webservice
which would follow a given pattern.

So far only the following patterns are available:
- {isbn} will be replaced byt the isbn of the record
- {issn}
- using MARC subfields:
  {024$a} to replace with the value in 024$a
(Note that the selector has been changed from %% to {} in a coming
patch)

For example
- /your/local/images/{isbn}.png
- https://covers.openlibrary.org/b/isbn/{isbn}-M.jpb
- etc.

There are 3 new sysprefs:
CustomCoverImagesURL
=> Define the path/url
CustomCoverImages
=> Turn on/off the display of the custom cover image in the staff client
OPACCustomCoverImages
=> Turn on/off the display of the custom cover image at the OPAC

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 100 Martin Renvoize 2019-10-22 13:59:47 UTC
Created attachment 94560 [details] [review]
Bug 22445: Add new method Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 101 Martin Renvoize 2019-10-22 13:59:51 UTC
Created attachment 94561 [details] [review]
Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 102 Martin Renvoize 2019-10-22 13:59:55 UTC
Created attachment 94562 [details] [review]
Bug 22445: Allow marc fields as pattern

%tag$field%

There is a limitation here, only one replacement will work.
Let see if it is needed later to support several patterns

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 103 Martin Renvoize 2019-10-22 13:59:59 UTC
Created attachment 94563 [details] [review]
Bug 22445: Custom cover images - opac shelfbrowser

Test plan:
Go to the detail page of a bibliographic record (at the OPAC)
and confirm the at custom cover images are displayed in the shelf
browser

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 104 Martin Renvoize 2019-10-22 14:00:04 UTC
Created attachment 94564 [details] [review]
Bug 22445: Custom cover images - opac results

Test plan:
Search for bibliographic records at the OPAC, the custom cover should be
displayed

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 105 Martin Renvoize 2019-10-22 14:00:09 UTC
Created attachment 94566 [details] [review]
Bug 22445: Custom cover images - opac detail

Test plan:
Go to the bibliographic record detail page at the OPAC, you should see
the custom cover image.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 106 Martin Renvoize 2019-10-22 14:00:14 UTC
Created attachment 94567 [details] [review]
Bug 22445: Custom cover images - opac result (rss)

This cannot be tested, it seems that the template is broken.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 107 Martin Renvoize 2019-10-22 14:00:17 UTC
Created attachment 94568 [details] [review]
Bug 22445: Custom cover images - opac shelves

Test plan:
Add a bibliographic record (that has a custom image) to a shelf,
view the shelf (OPAC).
You should see the image.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 108 Martin Renvoize 2019-10-22 14:00:21 UTC
Created attachment 94569 [details] [review]
Bug 22445: Custom cover images - opac reviews

Test plan:
- enable OpacShowRecentComments
- make a comment (OPAC)
- approve it (staff)
- click 'Recent comments' (OPAC)
=> You should see the custom cover

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 109 Martin Renvoize 2019-10-22 14:00:26 UTC
Created attachment 94570 [details] [review]
Bug 22445: Custom cover images - opac reviews (rss)

Test plan:
Same as previous patch, then click on the feeds link

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 110 Martin Renvoize 2019-10-22 14:00:30 UTC
Created attachment 94571 [details] [review]
Bug 22445: Custom cover images - opac user (checkout list)

Test plan:
- Check an item out to a patron
- At the OPAC loggin as this patron
On the checkout list you should see the cover image.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 111 Martin Renvoize 2019-10-22 14:00:34 UTC
Created attachment 94572 [details] [review]
Bug 22445: Custom cover images - detail (staff)

Test plan:
Staff side now, go the detail page of a bibliographic record
You should see the cover image.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 112 Martin Renvoize 2019-10-22 14:00:38 UTC
Created attachment 94573 [details] [review]
Bug 22445: Custom cover images - search resulta (staff)

Test plan:
Search for bibliographic records, the custom covers should be displayed.

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 113 Martin Renvoize 2019-10-22 14:00:42 UTC
Created attachment 94574 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - OPAC

This patch adds some CSS and markup to OPAC templates so that custom
cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

To test, apply the patch and regenerate the OPAC CSS. Post an oversized
image which corresponds to a record in your OPAC which can be added to a
user's checkouts and to a list.

View the following pages to confirm that the image displays well:

 - OPAC search results
 - Bibliographic detail page
 - Shelf browser
 - List contents
 - Recent comments
 - "Your summary"

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Martin Renvoize 2019-10-22 14:00:46 UTC
Created attachment 94575 [details] [review]
Bug 22445: (follow-up) Use CSS to set a max-width for images - Staff client

This patch adds some CSS and markup to staff client templates so that
custom cover images have a maximum width of 140px (to match the width of
generated local cover image thumbnails). This prevents oversized images
from displaying inconsistently compared to images from other services.

This patch also adds a missing </a> to fix display problems.

To test, apply the patch and regenerate the staff client CSS. Post an
oversized image which corresponds to a record in your catalog.

View the following pages to confirm that the image displays well:

 - Catalog search results
 - Bibliographic detail page

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 115 Martin Renvoize 2019-10-22 14:00:50 UTC
Created attachment 94576 [details] [review]
Bug 22445: Add new pattern %normalized_isbn%

Quite bad to use C4::Koha from Koha::Biblio :-/

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 116 Martin Renvoize 2019-10-22 14:00:54 UTC
Created attachment 94577 [details] [review]
Bug 22445: Replace %% with {}

Sponsored-by: Orex Digital
Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 117 Martin Renvoize 2019-10-22 14:01:34 UTC
Looks good and works well.. passes the QA scripts...

Passing QA
Comment 118 Martin Renvoize 2019-10-22 14:06:15 UTC
Nice work!

Pushed to master for 19.11.00
Comment 119 Joonas Kylmälä 2019-11-08 06:58:48 UTC
The database upgrade breaks on my kohadevbox setup (mysql-server package version 5.7.27-0ubuntu0.18.04.1):

> $ updatedatabase 
> Upgrade to 19.06.00.042 done (Bug 22445 - Add new pref *CustomCoverImages*)
> DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'IGNORE TABLE accountlines
>                 ADD
>                   debit_type_code ' at line 1 [for Statement "
>                 ALTER IGNORE TABLE accountlines
>                 ADD
>                   debit_type_code varchar(80) DEFAULT NULL
>                 AFTER
>                   accounttype
>               "] at /home/koha/kohaclone/installer/data/mysql/updatedatabase.pl line 19888.
> DBD::mysql::db do failed: Key column 'debit_type_code' doesn't exist in table [for Statement "
>             ALTER TABLE accountlines ADD CONSTRAINT `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES `account_debit_types` (`code`) ON DELETE RESTRICT ON UPDATE CASCADE
>               "] at /home/koha/kohaclone/installer/data/mysql/updatedatabase.pl line 19901.
> DBD::mysql::db do failed: Unknown column 'debit_type_code' in 'field list' [for Statement "
>         UPDATE accountlines SET debit_type_code = accounttype, accounttype = NULL WHERE accounttype IN (SELECT code from account_debit_types)
>         "] at /home/koha/kohaclone/installer/data/mysql/updatedatabase.pl line 19910.
> Upgrade to 19.06.00.043 done (Bug 23049 - Add account debit_types)
> DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'IGNORE TABLE accountlines
>                 ADD
>                   credit_type_code' at line 1 [for Statement "
>                 ALTER IGNORE TABLE accountlines
>                 ADD
>                   credit_type_code varchar(80) DEFAULT NULL
>                 AFTER
>                   accounttype
>               "] at /home/koha/kohaclone/installer/data/mysql/updatedatabase.pl line 19992.
> DBD::mysql::db do failed: Key column 'credit_type_code' doesn't exist in table [for Statement "
>                 ALTER TABLE accountlines
>                 ADD CONSTRAINT
>                   `accountlines_ibfk_credit_type`
>                 FOREIGN KEY (`credit_type_code`) REFERENCES `account_credit_types` (`code`)
>                 ON DELETE RESTRICT
>                 ON UPDATE CASCADE
>               "] at /home/koha/kohaclone/installer/data/mysql/updatedatabase.pl line 20005.
> DBD::mysql::db do failed: Unknown column 'credit_type_code' in 'field list' [for Statement "
>           UPDATE
>             accountlines 
>           SET
>             credit_type_code = accounttype, accounttype = NULL
>           WHERE accounttype IN (SELECT code from account_credit_types)
>         "] at /home/koha/kohaclone/installer/data/mysql/updatedatabase.pl line 20067.
> Upgrade to 19.06.00.044 done (Bug 23805 - Add account credit_types)
> Upgrade to 19.06.00.045 done (Bug 23866 - Set HEA syspref to prompt for review)
> Upgrade to 19.06.00.046 done (Bug 15260 - Option for extended loan with useDaysMode)
> Upgrade to 19.06.00.047 done (Bug 14697 - Extend and enhance 'Claims returned' lost status)
> Upgrade to 19.06.00.048 done (Bug 22581 - add new OPACShowMusicalInscripts and OPACPlayMusicalInscripts system preferences)
> Upgrade to 19.06.00.049 done (Bug 13958 - Add a SuspensionsCalendar syspref)
> Upgrade to 19.06.00.050 done (Bug 23293 - Add 'OPACFineNoRenewalsIncludeCredits' system preference)
Comment 120 Joonas Kylmälä 2019-11-08 07:34:49 UTC
(In reply to Joonas Kylmälä from comment #119)
> The database upgrade breaks on my kohadevbox setup (mysql-server package
> version 5.7.27-0ubuntu0.18.04.1):

Looking again at the error message the bug is probably from Bug 23049 and not this. Sorry for the noise.
Comment 121 Martin Renvoize 2019-11-08 08:18:50 UTC
Fix pushed.
Comment 122 Hugo Agud 2020-10-27 06:23:23 UTC
We have identified one issue this this functionality, when there is no value in the variable isbn, issn.. or wathever the patch generate the <img src> anyway generating an empty thumnail link.
Comment 123 Katrin Fischer 2020-11-01 12:36:28 UTC
(In reply to Hugo Agud from comment #122)
> We have identified one issue this this functionality, when there is no value
> in the variable isbn, issn.. or wathever the patch generate the <img src>
> anyway generating an empty thumnail link.

Please move this to a separate bug report linked to this one - this is already in released versions so it needs to be fixed on a new bug report.
Comment 124 Katrin Fischer 2021-10-05 09:55:55 UTC
I did't manage to use 001 - is there a special syntax to use for control fields?