Bug 1633 - Add ability to take book cover images from local img db
Summary: Add ability to take book cover images from local img db
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.8
Hardware: All All
: P3 enhancement with 4 votes (vote)
Assignee: Jared Camins-Esakov
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 7587
Blocks: 8312
  Show dependency treegraph
 
Reported: 2007-11-29 07:17 UTC by Chris Cormack
Modified: 2014-05-26 21:04 UTC (History)
15 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Patch to enable local cover images (37.83 KB, patch)
2011-11-06 09:37 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (62.75 KB, patch)
2011-12-10 19:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (62.77 KB, patch)
2011-12-11 13:16 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (63.59 KB, patch)
2011-12-12 01:12 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (30.67 KB, patch)
2011-12-13 13:41 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Display local cover images (31.83 KB, patch)
2011-12-13 13:41 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (30.74 KB, patch)
2011-12-13 13:45 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Display local cover images (31.89 KB, patch)
2011-12-13 13:45 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (30.74 KB, patch)
2011-12-13 13:47 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Display local cover images (31.89 KB, patch)
2011-12-13 13:47 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Add support for uploading images to Koha (30.79 KB, patch)
2011-12-13 16:25 UTC, Koustubha Kale
Details | Diff | Splinter Review
Display local cover images (31.95 KB, patch)
2011-12-13 16:27 UTC, Koustubha Kale
Details | Diff | Splinter Review
Improved styling and no-JavaScript handling for the OPAC (7.47 KB, patch)
2011-12-13 17:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 1633 - Add ability to take book cover images from local img db (7.85 KB, patch)
2011-12-13 18:30 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Don't show image tab when inappropriate (4.27 KB, patch)
2011-12-14 02:06 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
SIGNED-OFF : Add ability to take book cover images from local img db (7.91 KB, patch)
2011-12-14 04:00 UTC, Koustubha Kale
Details | Diff | Splinter Review
SIGNED-OFF : Don't show image tab when inappropriate (4.33 KB, patch)
2011-12-14 04:01 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (29.97 KB, patch)
2011-12-14 15:56 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Display local cover images (31.75 KB, patch)
2011-12-14 15:56 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 - Add ability to take book cover images from local img db (7.84 KB, patch)
2011-12-14 15:57 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Don't show image tab when inappropriate (4.26 KB, patch)
2011-12-14 15:58 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Follow-up fix (11.41 KB, patch)
2011-12-14 17:16 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 1633 - Interface revisions for the staff client (11.47 KB, patch)
2011-12-16 02:36 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add support for uploading images to Koha (30.03 KB, patch)
2011-12-30 13:57 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Display local cover images (33.55 KB, patch)
2011-12-30 13:57 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 - Add ability to take book cover images from local img db (7.86 KB, patch)
2011-12-30 13:58 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Don't show image tab when inappropriate (5.33 KB, patch)
2011-12-30 13:58 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 - Interface revisions for the staff client (11.52 KB, patch)
2011-12-30 13:58 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Signed-off patch (30.13 KB, patch)
2011-12-30 16:29 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 1633: Display local cover images (33.62 KB, patch)
2011-12-30 16:34 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 1633: Display local cover images (33.62 KB, patch)
2011-12-30 16:37 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 1633 - Add ability to take book cover images from local img db (7.93 KB, patch)
2011-12-30 16:38 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 1633: Don't show image tab when inappropriate (5.40 KB, patch)
2011-12-30 16:38 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 1633 - Interface revisions for the staff client (11.70 KB, patch)
2011-12-30 16:39 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Add support for uploading images to Koha (30.35 KB, patch)
2012-01-14 08:49 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Display local cover images (33.67 KB, patch)
2012-01-14 08:53 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Add support for uploading images to Koha (30.35 KB, patch)
2012-01-14 08:54 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Add support for uploading images to Koha (30.35 KB, patch)
2012-01-14 08:56 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Display local cover images (33.67 KB, patch)
2012-01-14 08:57 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633 - [SIGNED-OFF] Add ability to take book cover images from local img db (7.98 KB, patch)
2012-01-14 08:58 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Don't show image tab when inappropriate (5.45 KB, patch)
2012-01-14 08:58 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633 - [SIGNED-OFF] Interface revisions for the staff client (11.83 KB, patch)
2012-01-14 08:59 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633 - Interface revisions for the staff client (11.73 KB, patch)
2012-01-14 13:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: QA follow-up (39.22 KB, patch)
2012-01-18 21:32 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Display local cover images (33.82 KB, patch)
2012-01-21 16:25 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: QA follow-up (39.28 KB, patch)
2012-01-21 16:34 UTC, Koustubha Kale
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Add support for uploading images to Koha (30.35 KB, patch)
2012-01-22 14:30 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Display local cover images (33.82 KB, patch)
2012-01-22 14:30 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 - [SIGNED-OFF] Add ability to take book cover images from local img db (7.98 KB, patch)
2012-01-22 14:31 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: [SIGNED-OFF] Don't show image tab when inappropriate (5.45 KB, patch)
2012-01-22 14:31 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 - Interface revisions for the staff client (11.73 KB, patch)
2012-01-22 14:31 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: QA follow-up (39.28 KB, patch)
2012-01-22 14:32 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633: Add missing permissions (5.83 KB, patch)
2012-01-24 13:43 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 follow-up: Fix jenkins tests (7.76 KB, patch)
2012-01-24 14:23 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 follow-up: Fix jenkins tests (8.44 KB, patch)
2012-01-24 15:40 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 1633 follow-up: Fix jenkins tests (8.40 KB, patch)
2012-01-24 19:30 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 1633 follow-up: Fix jenkins tests (8.46 KB, patch)
2012-01-24 19:30 UTC, Chris Cormack
Details | Diff | Splinter Review
Followup on kohastructure (1011 bytes, patch)
2012-01-26 08:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:32:50 UTC


---- Reported by rch@liblime.com 2007-11-29 19:17:28 ----

look first for local content, then query external source e.g. amazon



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:32 UTC  ---

This bug was previously known as _bug_ 1633 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=1633

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was rch@liblime.com.
CC member kdr@ckls.org does not have an account here

Comment 1 Koustubha Kale 2010-12-11 10:54:43 UTC
Hi All,
I have received sponsorship from "Sikh National Archives" to add the ability in Koha to display book covers scanned and stored locally in the server. "Sikh National Archives" have a lot of old / rare books & manuscripts which do not have ISBN hence this requirement.
  
Here are the customer requirements in brief :
1. Provide facility to upload scanned cover images linked to biblionumber.
2. Create required mysql tables to hold cover images.
3. Provide global system preference to control display of local cover images in OPAC.
4. Create system preference to decide priority of cover images from multiple
providers such as Amazon, Google, Local.
5. Customize OPAC to display only first cover image found as per system preference
priority.
6. Customize OPAC search and detail pages to display local cover images linked to
biblionumber.
7. Provide user rights management to control upload of images.

I have added a RFC to wiki here : http://wiki.koha-community.org/wiki/Local_cover_images_RFC

Requesting for comments and suggestions..
Comment 2 Dobrica Pavlinusic 2011-04-10 10:53:05 UTC
What is current state of this bug? I also have need for local cover images and I wouldn't like to duplicate effort.

Is source for this changes available somewhere, so we can work on same code?
Comment 3 Koustubha Kale 2011-11-06 09:37:14 UTC Comment hidden (obsolete)
Comment 4 Magnus Enger 2011-11-08 11:59:57 UTC
* Patch did not apply, giving this message: "fatal: cannot convert from UTF-8utf-8 to UTF-8". 
Removing 'Content-Type: text/plain; charset="utf-8"' from the patch by hand solved this. 
 
* There are a couple of typos in updatedatabase.pl: 
- "Display local cover mages on OPAC"
- "table to stover local cover images"

* Comment #3 says "Images should be 140x200 pixels. If they are larger they will be auto-resized to comply", but the code in upload-cover-image.pl seems to implement a max size of 200x300 (which is way too big if you ask me): 

$debug and warn "$filename is $width pix X $height pix.";
if ($width > 200 || $height > 300) {    # MAX pixel dims are 200 X 300...
$debug and warn "$filename exceeds the maximum pixel dimensions of 200 X 300. Resizing...";

* A couple of things in the templates:

- opac-detail.tt: Why have alt="No Local Cover Image" when the image is actually displayed? 
[% IF ( OPACLocalCoverImages ) %] <img src="/cgi-bin/koha/opac-coverimage.pl?bibnum=[% biblionumber %]" alt="No Local Cover Image" class="thumbnail" />[% ELSE %]<span class="no-image">No Local cover image available</span>[% END %]

- opac-results.tt: Same problem with the alt text. And there is a missing " after [% SEARCH_RESULT.biblionumber %]. 
[% IF ( OPACLocalCoverImages ) %] <img src="/cgi-bin/koha/opac-coverimage.pl?bibnum=[% SEARCH_RESULT.biblionumber %] alt="No Local Cover Image" class="thumbnail" />[% ELSE %]<span class="no-image">No Local cover image available</span>[% END %]

* Questions/suggestion for enhancement
- The note that says "NOTE: Only PNG, GIF, JPEG, XPM formats are supported." when you are about to upload an image could perhaps also mention the restrictions on file size and dimensions?
- It looks like images on the result list and detail view will be the same size? It would be nice to have small images in the result list and bigger images on the detail view. 
- Maybe the max size could be configurable through systempreferences? 

This will be a great feature, though!
Comment 5 Jared Camins-Esakov 2011-12-10 19:27:24 UTC Comment hidden (obsolete)
Comment 6 Koustubha Kale 2011-12-11 08:07:50 UTC
Hi,
Tried the patch. Applies cleanly except for a few whitespace errors.
But when I try to run the automic update installer/data/mysql/atomicupdate/local_cover_images.pl I got a permission denied. So executable permissions are needed for this file. After I did that the patch works.

Also the uploaded image does not show up in the intranet search page. It does show up in the OPAC search page. In both intranet & OPAC the uploaded images DO show up under the images tab in detail page.


regards
kmkale
Comment 7 Jared Camins-Esakov 2011-12-11 13:16:05 UTC Comment hidden (obsolete)
Comment 8 Jared Camins-Esakov 2011-12-11 13:20:59 UTC
The most recent patch resolves the whitespace errors and gives the updater executable permissions. Cover images are not added to the Intranet search results because I found them to be very aesthetically unappealing. Someone with more design ability could easily add them in a follow-up patch, if s/he wanted.
Comment 9 Jared Camins-Esakov 2011-12-12 01:12:12 UTC Comment hidden (obsolete)
Comment 10 Jared Camins-Esakov 2011-12-13 13:41:24 UTC Comment hidden (obsolete)
Comment 11 Jared Camins-Esakov 2011-12-13 13:41:44 UTC Comment hidden (obsolete)
Comment 12 Jared Camins-Esakov 2011-12-13 13:45:20 UTC Comment hidden (obsolete)
Comment 13 Jared Camins-Esakov 2011-12-13 13:45:37 UTC Comment hidden (obsolete)
Comment 14 Jared Camins-Esakov 2011-12-13 13:47:15 UTC Comment hidden (obsolete)
Comment 15 Jared Camins-Esakov 2011-12-13 13:47:29 UTC Comment hidden (obsolete)
Comment 16 Jared Camins-Esakov 2011-12-13 13:49:03 UTC
Please note that *both* patches are required for the functionality. First apply "Add support for uploading images to Koha" then apply "Display local cover images." Then test as specified in the commit message on the second patch.
Comment 17 Koustubha Kale 2011-12-13 16:25:32 UTC Comment hidden (obsolete)
Comment 18 Koustubha Kale 2011-12-13 16:27:08 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2011-12-13 17:33:25 UTC Comment hidden (obsolete)
Comment 20 Jared Camins-Esakov 2011-12-13 18:30:04 UTC Comment hidden (obsolete)
Comment 21 Owen Leonard 2011-12-13 21:03:56 UTC
I just noticed a problem: The images tab is showing up in the OPAC and staff client for all titles, not just titles with local images.
Comment 22 Jared Camins-Esakov 2011-12-14 02:06:04 UTC Comment hidden (obsolete)
Comment 23 Koustubha Kale 2011-12-14 04:00:14 UTC Comment hidden (obsolete)
Comment 24 Koustubha Kale 2011-12-14 04:01:26 UTC Comment hidden (obsolete)
Comment 25 Chris Nighswonger 2011-12-14 14:55:55 UTC
From an IRC discussion regarding the  RetrieveImage sub:

<magnuse> chris_n: http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6755
* chris_n notes that this patch adds a what appears to be unrelated line to Makefile.PL
<chris_n> yeah, that sub needs to be changed to return a null value if there is a db error
<chris_n> it already logs it
<chris_n> ie. if ($sth->errstr) { log error; return;}
<chris_n> and then let the caller handle it
Comment 26 Jared Camins-Esakov 2011-12-14 15:17:22 UTC
(In reply to comment #25)
> From an IRC discussion regarding the  RetrieveImage sub:
> 
> <magnuse> chris_n:
> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6755
> * chris_n notes that this patch adds a what appears to be unrelated line to
> Makefile.PL

Without the added line to Makefile.PL, production-mode Koha installations fail dramatically because of the Koha/Images.pm file not being installed.

> <chris_n> yeah, that sub needs to be changed to return a null value if there is
> a db error
> <chris_n> it already logs it
> <chris_n> ie. if ($sth->errstr) { log error; return;}
> <chris_n> and then let the caller handle it

I can make that change.
Comment 27 Chris Nighswonger 2011-12-14 15:20:55 UTC
Upon further clarification, the change to Makefile.PL is adding a new directory implementing a previously proposed Koha:: namespace. While I'm not against this in principle, I don't recall any formal discussion and/or decision on this move. There should probably be some or at least some reference to it on the wiki.

If/when we do add this namespace, it should probably be done as a separate commit with an appropriate amount of verbiage explaining the change and referencing said discussion/decision rather than being buried in a large feature change/addition.

My $0.02 worth.
Comment 28 Jared Camins-Esakov 2011-12-14 15:56:11 UTC Comment hidden (obsolete)
Comment 29 Jared Camins-Esakov 2011-12-14 15:56:43 UTC Comment hidden (obsolete)
Comment 30 Jared Camins-Esakov 2011-12-14 15:57:45 UTC Comment hidden (obsolete)
Comment 31 Jared Camins-Esakov 2011-12-14 15:58:05 UTC Comment hidden (obsolete)
Comment 32 Jared Camins-Esakov 2011-12-14 15:59:00 UTC
I have revised the patches as requested, and eliminated the Koha:: namespace. Further comments appreciated.
Comment 33 Owen Leonard 2011-12-14 17:16:42 UTC Comment hidden (obsolete)
Comment 34 Jared Camins-Esakov 2011-12-16 02:36:59 UTC Comment hidden (obsolete)
Comment 35 Magnus Enger 2011-12-30 11:26:01 UTC
I applied all the patches attached, in the order they are listed, so git log gives me this: 

$ git log --pretty=oneline --abbrev-commit
23c90fb Bug 1633 - Interface revisions for the staff client
6a652ec Bug 1633: Don't show image tab when inappropriate
7abbc66 Bug 1633 - Add ability to take book cover images from local img db
57eda8a Bug 1633: Display local cover images
38762dd Bug 1633: Add support for uploading images to Koha
f99601e Bug 7375 - Update minimum version of Perl to 5.10

I'm marking this failed QA because of the following three blockers. The enhancements would be nice to have, but can be left for later. 

*** Blockers

* No image shown when there is just one

LocalCoverImages = On

Uploaded one image and attached it to biblionumber = 1. 

Image shows OK here: /cgi-bin/koha/catalogue/detail.pl?biblionumber=1
But: "There are no images for this record." at /cgi-bin/koha/catalogue/imageviewer.pl?biblionumber=1&imagenumber=1

Changing line 68 of koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/imageviewer.tt from this:
  [% IF ( images.size > 1 ) %]
to this: 
  [% IF ( images.size > 0 ) %]
makes the image display. 

Uploading a second image makes both of them display correctly. 

* AllowMultipleCovers does not stop multiple covers

With AllowMultipleCovers = "Don't", /cgi-bin/koha/tools/upload-cover-image.pl was automatically set to "Replace existing covers", but I had no problem adding multiple covers to one record. 

* Always "Click on an image..."

On records that have no associated images I always see this: "Click on an image to view it in the image viewer", not this: "No images have been uploaded for this bibliographic record yet. Please upload one.". 

Changing line 522 in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt from 
  [% IF ( localimages ) %]
to 
  [% IF ( localimages.0 ) %]
seems to solve this. 

I'm not sure, but I think that when there are no images, localimages is an array where the first element is undef, not an array that is undefined - or something like that...

*** Enhancements

* How can images be deleted? 

* Thumbnails have empty alt

Thumbnails displayed on the right hand side in the image viewer (both OPAC and Intranet) have empty alt attributes: 

<img alt="" src="/cgi-bin/koha/catalogue/image.pl?imagenumber=3&amp;thumbnail=1" id="3" class="thumbnail">

It could perhaps say alt="Thumbnail" or something similar? 

* Still possible to see images when OPACLocalCoverImages is off

OPACLocalCoverImages = Don't display

I can still see the image at /cgi-bin/koha/opac-image.pl?imagenumber=1

* Wrong POD in tools/upload-cover-image.pl?

The POD on line 33 of tools/upload-cover-image.pl says: "Files greater than 100K will be refused. Images should be 140x200 pixels. If they are larger they will be auto-resized to comply." 

I was able to upload an image of 1.3MB without any trouble, and it was resized correctly. 

The last part of this has perhaps not been updated to take into account the 600x800 pixel max size of the "large" view? 

* Images in the modification log? 

When I look at /cgi-bin/koha/tools/viewlog.pl?do_it=1&modules=CATALOGUING&action=MODIFY&object=1 there is no mention of the images there. Perhaps that would be a nice addition, so you could see when images were added, removed etc? 

* Make it possible to select "main" image when there is more than one

Looks like the first image to be uploaded is the one that is displayed in the result list and at the top of a record in detail view. Might be nice if it was possible to select which image should be the "main" one. 

* Make it possible to replace one specific image among several
Comment 36 Jared Camins-Esakov 2011-12-30 13:56:16 UTC
Magnus,

Thanks for testing. I have five revised patches coming up that fix your blockers, and a few of your enhancements. Hot off the keyboard!

(In reply to comment #35)
> * No image shown when there is just one
> 
> LocalCoverImages = On
> 
> Uploaded one image and attached it to biblionumber = 1. 
> 
> Image shows OK here: /cgi-bin/koha/catalogue/detail.pl?biblionumber=1
> But: "There are no images for this record." at
> /cgi-bin/koha/catalogue/imageviewer.pl?biblionumber=1&imagenumber=1
> 
> Changing line 68 of
> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/imageviewer.tt from this:
>   [% IF ( images.size > 1 ) %]
> to this: 
>   [% IF ( images.size > 0 ) %]
> makes the image display. 
> 
> Uploading a second image makes both of them display correctly. 

Fixed

> * AllowMultipleCovers does not stop multiple covers
> 
> With AllowMultipleCovers = "Don't", /cgi-bin/koha/tools/upload-cover-image.pl
> was automatically set to "Replace existing covers", but I had no problem adding
> multiple covers to one record. 

Fixed

> * Always "Click on an image..."
> 
> On records that have no associated images I always see this: "Click on an image
> to view it in the image viewer", not this: "No images have been uploaded for
> this bibliographic record yet. Please upload one.". 
> 
> Changing line 522 in
> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt from 
>   [% IF ( localimages ) %]
> to 
>   [% IF ( localimages.0 ) %]
> seems to solve this. 
> 
> I'm not sure, but I think that when there are no images, localimages is an
> array where the first element is undef, not an array that is undefined - or
> something like that...

Fixed

> *** Enhancements
> 
> * How can images be deleted? 

Good point. It is not yet possible. I will put that on my to-do list.

> * Thumbnails have empty alt
> 
> Thumbnails displayed on the right hand side in the image viewer (both OPAC and
> Intranet) have empty alt attributes: 
> 
> <img alt=""
> src="/cgi-bin/koha/catalogue/image.pl?imagenumber=3&amp;thumbnail=1" id="3"
> class="thumbnail">
> 
> It could perhaps say alt="Thumbnail" or something similar? 

Done!

> * Still possible to see images when OPACLocalCoverImages is off
> 
> OPACLocalCoverImages = Don't display
> 
> I can still see the image at /cgi-bin/koha/opac-image.pl?imagenumber=1

Not anymore. ;)

> * Wrong POD in tools/upload-cover-image.pl?
> 
> The POD on line 33 of tools/upload-cover-image.pl says: "Files greater than
> 100K will be refused. Images should be 140x200 pixels. If they are larger they
> will be auto-resized to comply." 
> 
> I was able to upload an image of 1.3MB without any trouble, and it was resized
> correctly. 
> 
> The last part of this has perhaps not been updated to take into account the
> 600x800 pixel max size of the "large" view? 

There was incorrect POD, but I changed it.
Comment 37 Jared Camins-Esakov 2011-12-30 13:57:32 UTC Comment hidden (obsolete)
Comment 38 Jared Camins-Esakov 2011-12-30 13:57:50 UTC Comment hidden (obsolete)
Comment 39 Jared Camins-Esakov 2011-12-30 13:58:10 UTC Comment hidden (obsolete)
Comment 40 Jared Camins-Esakov 2011-12-30 13:58:26 UTC Comment hidden (obsolete)
Comment 41 Jared Camins-Esakov 2011-12-30 13:58:40 UTC Comment hidden (obsolete)
Comment 42 Magnus Enger 2011-12-30 16:29:19 UTC Comment hidden (obsolete)
Comment 43 Magnus Enger 2011-12-30 16:34:23 UTC Comment hidden (obsolete)
Comment 44 Magnus Enger 2011-12-30 16:37:38 UTC Comment hidden (obsolete)
Comment 45 Magnus Enger 2011-12-30 16:38:10 UTC Comment hidden (obsolete)
Comment 46 Magnus Enger 2011-12-30 16:38:38 UTC Comment hidden (obsolete)
Comment 47 Magnus Enger 2011-12-30 16:39:10 UTC Comment hidden (obsolete)
Comment 48 Magnus Enger 2011-12-30 16:46:08 UTC
I have signed off all the current patches. 

The blockers mentioned in comment 35 have all been fixed, as well as some of the enhancements. 

I have tested uploading large and small jpg and png images to specific records. Upload and display works as expected. When AllowMultipleCovers = "Don't allow" new images for a record with old images automatically overwrite the old ones. 

I have also tested with a zip-file containing 3 images and a comma separated file called datalink.txt. I hve tested attaching all 3 images to the same record, as well as different images to different records. 

We can probably think of improvements to this, but it looks like the base functionality is OK, so I think we should get this in now and then we can enhance it later.
Comment 49 Koustubha Kale 2012-01-14 08:49:19 UTC Comment hidden (obsolete)
Comment 50 Koustubha Kale 2012-01-14 08:53:35 UTC Comment hidden (obsolete)
Comment 51 Koustubha Kale 2012-01-14 08:54:34 UTC Comment hidden (obsolete)
Comment 52 Koustubha Kale 2012-01-14 08:56:31 UTC Comment hidden (obsolete)
Comment 53 Koustubha Kale 2012-01-14 08:57:26 UTC Comment hidden (obsolete)
Comment 54 Koustubha Kale 2012-01-14 08:58:20 UTC Comment hidden (obsolete)
Comment 55 Koustubha Kale 2012-01-14 08:58:59 UTC Comment hidden (obsolete)
Comment 56 Koustubha Kale 2012-01-14 08:59:22 UTC Comment hidden (obsolete)
Comment 57 Koustubha Kale 2012-01-14 09:24:56 UTC
(In reply to comment #56)
> Created attachment 7153 [details] [review]
> Bug 1633 - [SIGNED-OFF] Interface revisions for the staff client
> 
> This patch revises the image viewer page to match other
> biblio detail pages: a toolbar, a sidebar menu, etc.
> 
> Additional data is now pulled by imageviewer.pl in order
> to provide information necessary for the proper functioning
> of the toolbar and menus: item counts, hold availability,
> biblio view options.
> 
> Other changes have been made to match the changes in the
> OPAC interface like styling of the images and no-javascript
> compatibility
> 
> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
> Signed-off-by: Magnus Enger <magnus@enger.priv.no>
> 
> Patch failed to apply because of multiple Content-Type lines in the patch.
> Corrected the same with this new patch.

The attachment still has the double content-type lines. Tried amending the patch after it failed to apply during a git bz apply, but could not get rid of the double content-type lines. Any help in this matter is much appreciated.
Comment 58 Jared Camins-Esakov 2012-01-14 13:28:49 UTC Comment hidden (obsolete)
Comment 59 Jared Camins-Esakov 2012-01-14 13:31:48 UTC
Fixed double content encoding and git-bz's hatred of Unicode subjects. For future reference, the double content encoding appears when there is Unicode in the body or subject of the commit message. In this case, the problem was the subject.
Comment 60 Paul Poulain 2012-01-18 16:36:38 UTC
QA comments:
* C4/Images.pm and C4/UploadedFile.pm have no copyright/ GPL header
* have new scripts be perltidied ?
* some files uses tab instead of 4 spaces for indentation (and in a given file, there is sometimes a mix of both, for example upload-images.tt)
* in UploadCoverImage, the FSF address is wrong, it should be # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
* opac-image.pl has a GPL/FSF header, but no copyright 2011, xxx
Comment 61 Paul Poulain 2012-01-18 16:45:52 UTC
QA comment:
a strange thing: the "upload image" button appear on the staff ONLY if you've set "localCoverImages" syspref, not if you've set "OPACLocalCoverImages".
I thought (and still think) we could decide to have cover images on OPAC and not on staff interface (otherwise, why 2 sysprefs !)

Other than this, it works well !
Comment 62 Jared Camins-Esakov 2012-01-18 21:32:59 UTC Comment hidden (obsolete)
Comment 63 Koustubha Kale 2012-01-21 16:25:39 UTC Comment hidden (obsolete)
Comment 64 Koustubha Kale 2012-01-21 16:34:31 UTC Comment hidden (obsolete)
Comment 65 Koustubha Kale 2012-01-21 16:40:44 UTC
Tested and signed off on QA follow-up patch by Jared Camins-Esakov.
Also rebased the patch for Display local cover images as it was failing to apply for file koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css due to changes pushed in master.
Comment 66 Jared Camins-Esakov 2012-01-22 14:30:25 UTC Comment hidden (obsolete)
Comment 67 Jared Camins-Esakov 2012-01-22 14:30:59 UTC Comment hidden (obsolete)
Comment 68 Jared Camins-Esakov 2012-01-22 14:31:12 UTC Comment hidden (obsolete)
Comment 69 Jared Camins-Esakov 2012-01-22 14:31:37 UTC Comment hidden (obsolete)
Comment 70 Jared Camins-Esakov 2012-01-22 14:31:49 UTC Comment hidden (obsolete)
Comment 71 Jared Camins-Esakov 2012-01-22 14:32:07 UTC Comment hidden (obsolete)
Comment 72 Jared Camins-Esakov 2012-01-22 14:35:19 UTC
The patches were out of order on this bug. I just downloaded and reuploaded the patches to get them in the right order. I did not change any of the patches, and they are at their latest state and apply cleanly to master thanks to kmkale's rebasing on January 21.  Paul's QA comments have been incorporated, and everything has been signed off.
Comment 73 Paul Poulain 2012-01-24 10:59:47 UTC
QA comment :
* I made a lot of tests, everything work:
  - uploading more than 1 image
  - replacing 2 images by 1 image
  - uploading the same image on 2 different biblios, removing one
  - uploading different images on different biblios
  - displaying on OPAC. There is a glitch on the display, but removing a local css fixes the problem. It means local CSS will have to be updated probably.
  - tried to upload a file that is not an image. I got a nice: "Cannot process file as an image. Please ensure you only upload GIF, JPEG, PNG, or XPM images."

All my comment 60 and comment 61 point have been fixed
About comment 61 : I can't see a use case where the library would have OPAClocalcoverimages but not localcoverimages. It would result in something strange, as the librarian could upload his images, but not see the result in the staff. But maybe that will please some libraries, so I'll continue to live with my question ;-)

Passed QA,
Comment 74 Paul Poulain 2012-01-24 11:01:28 UTC
Congrats everybody !
Comment 75 Jared Camins-Esakov 2012-01-24 13:43:08 UTC Comment hidden (obsolete)
Comment 76 Jared Camins-Esakov 2012-01-24 14:23:19 UTC Comment hidden (obsolete)
Comment 77 Jared Camins-Esakov 2012-01-24 15:40:14 UTC Comment hidden (obsolete)
Comment 78 Chris Cormack 2012-01-24 19:30:04 UTC
Tests now pass

 perl xt/permissions.t 
unable to locate Koha configuration file koha-conf.xml at C4/Context.pm line 333.
ok 1 - Open reference user permissions file installer/data/mysql/en/mandatory/userpermissions.sql
ok 2 - Found 48 user permissions
ok 3 - Open translated user permissions file installer/data/mysql/de-DE/mandatory/userpermissions.sql
ok 4 - Found 48 perms
ok 5 - No user permissions to add
ok 6 - No user permissions to delete
ok 7 - Open translated user permissions file installer/data/mysql/it-IT/necessari/userpermissions.sql
ok 8 - Found 48 perms
ok 9 - No user permissions to add
ok 10 - No user permissions to delete
ok 11 - Open translated user permissions file installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql
ok 12 - Found 48 perms
ok 13 - No user permissions to add
ok 14 - No user permissions to delete
ok 15 - Open translated user permissions file installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql
ok 16 - Found 48 perms
ok 17 - No user permissions to add
ok 18 - No user permissions to delete
ok 19 - Open translated user permissions file installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql
ok 20 - Found 48 perms
ok 21 - No user permissions to add
ok 22 - No user permissions to delete
ok 23 - Open translated user permissions file installer/data/mysql/pl-PL/mandatory/userpermissions.sql
ok 24 - Found 48 perms
ok 25 - No user permissions to add
ok 26 - No user permissions to delete
1..26

 perl xt/tt_valid.t 
1..1
ok 1 - TT syntax: not using TT directive within HTML tag
Comment 79 Chris Cormack 2012-01-24 19:30:22 UTC Comment hidden (obsolete)
Comment 80 Chris Cormack 2012-01-24 19:30:48 UTC
Created attachment 7325 [details] [review]
Bug 1633 follow-up: Fix jenkins tests

The previous patches missed adding the upload_local_cover_images permission to
languages other than en, and used a forbidden TT construct, giving jenkins
agitas and indigestion. This patch adds the permission to other languages and
corrects the syntax.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 81 Marcel de Rooy 2012-01-26 08:34:25 UTC Comment hidden (obsolete)
Comment 82 Marcel de Rooy 2012-01-26 08:37:20 UTC
QA: Looks good. Updating status. Chris' patch made my last followup obsolete.
Comment 83 Paul Poulain 2012-03-13 14:58:06 UTC
(forgot to set version 3.8)
Comment 84 Nicole C. Engard 2012-07-09 21:00:05 UTC
I'm getting reports that local cover image aren't showing on the opac search results - is there a bug with this? Or was this not part of the scope of this patch?
Comment 85 Nicole C. Engard 2012-07-09 21:00:19 UTC
Sorry .. reports in 3.8.2
Comment 86 Nicole C. Engard 2012-09-06 21:00:44 UTC
Changing status to see if I can get an answer on my above comments. Is it working for everyone?
Comment 87 Magnus Enger 2013-03-15 10:47:53 UTC
 (In reply to comment #84)
> I'm getting reports that local cover image aren't showing on the opac search
> results - is there a bug with this? Or was this not part of the scope of
> this patch?

For me, local covers show up nicely in the search results on version 3.08.10.000, when OPACLocalCoverImages is activated. 

Nicole: Could you open a new bug if this is still a problem for your reporters?
Comment 88 Manos PETRIDIS 2013-09-02 15:17:59 UTC
The bug you're looking for, the one where local cover images fail to show, is #9737.