|
Description
Jonathan Druart
2019-03-01 22:54:30 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 Created attachment 85926 [details] [review] Bug 22445: Add new method Koha::Biblio->custom_cover_image_url Sponsored-by: Orex Digital Created attachment 85927 [details] [review] Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url Sponsored-by: Orex Digital 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 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 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 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 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 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 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 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 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 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 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 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? Hi Haley, 'Failed QA' is ok for getting answers to your questions. Thx for testing! (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). (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. 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 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 (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. (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. 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" 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 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. (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 I have applied the patch, it appears the class Cover Image, but src is emtpy then no image is published (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. Created attachment 87434 [details] [review] Bug 22445: Add new pattern %normalized_isbn% Quite bad to use C4::Koha from Koha::Biblio :-/ (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! 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. (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. Can you rebase this on master, please? Thank you. 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 Created attachment 89974 [details] [review] Bug 22445: Add new method Koha::Biblio->custom_cover_image_url Sponsored-by: Orex Digital Created attachment 89975 [details] [review] Bug 22445: Add tests for Koha::Biblio->custom_cover_image_url Sponsored-by: Orex Digital 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 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 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 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 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 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 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 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 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 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 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 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" 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 Created attachment 89989 [details] [review] Bug 22445: Add new pattern %normalized_isbn% Quite bad to use C4::Koha from Koha::Biblio :-/ 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> This 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. 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. (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? 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.
(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. (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 Happy with bash style.. or just plain moustache templating style might be even simpler for end users. https://my_url/{normalized_isbn}.png 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 :) I like {} too, it's also used in other prefs like SearchForTitleIn.
Switching to FQA meaning "Waiting for follow-up". I agree, 3 out of 4 commenters have agreed on {}. Let's get this into master ASAP!
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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> Created attachment 94244 [details] [review] Bug 22445: Replace %% with {} Thanks everybody for your feedback, much appreciated! 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> Looks good and works well.. passes the QA scripts... Passing QA Nice work! Pushed to master for 19.11.00 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)
(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. Fix pushed. 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. (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. I did't manage to use 001 - is there a special syntax to use for control fields? |
