Bug 26145

Summary: Add the ability to attach a cover image at item level
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: CatalogingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bugzilla, clemens.bastian, dcook, heather_hernandez, lisettepalouse+koha, lucas, m.de.rooy
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
If LocalCoverImages is turned on, it will now be possible to attach a local cover images for an item. It can be especially useful for subscriptions. One cover image could be attach per serial number. The cover images will be displayed on the item list table on the bibliographic record detail page.
Version(s) released in:
20.11.00
Bug Depends on:    
Bug Blocks: 27130, 27135, 30234, 30792    
Attachments: Bug 26145: Correctly preserve itemnumber value
Bug 26145: Use Koha::Logger and display the full stacktrace
Bug 26145: DB changes - Add biblioimages.imagenumber
Bug 26145: Add the ability to upload a cover image per item
Bug 26145: Display the cover image at the OPAC
Bug 26145: Refactoring - Add tests
Bug 26145: Refactoring - Rename table biblioimages to cover_images
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages
Bug 26145: Allow multi covers per item
Bug 26145: Fix image tag
Bug 26145: Hide 'Upload image' if no CoverImage pref is set
Bug 26145: Add the "Cover image" column to the table settings
Bug 26145: Correctly preserve itemnumber value
Bug 26145: Compiled CSS
Bug 26145: DBIC schema changes
Bug 26145: Display the cover image at the OPAC
Bug 26145: Refactoring - Add tests
Bug 26145: Refactoring - Rename table biblioimages to cover_images
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages
Bug 26145: Allow multi covers per item
Bug 26145: Fix image tag
Bug 26145: Hide 'Upload image' if no CoverImage pref is set
Bug 26145: Add the "Cover image" column to the table settings
Bug 26145: Correctly preserve itemnumber value
Bug 26145: Compiled CSS
Bug 26145: DBIC schema changes
Bug 26145: (QA follow-up) Add missing filters
Bug 26145: DB changes - Add biblioimages.imagenumber
Bug 26145: Add the ability to upload a cover image per item
Bug 26145: Display the cover image at the OPAC
Bug 26145: Refactoring - Add tests
Bug 26145: Refactoring - Rename table biblioimages to cover_images
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages
Bug 26145: Allow multi covers per item
Bug 26145: Fix image tag
Bug 26145: Hide 'Upload image' if no CoverImage pref is set
Bug 26145: Add the "Cover image" column to the table settings
Bug 26145: Correctly preserve itemnumber value
Bug 26145: DBIC schema changes
Bug 26145: Compiled CSS
Bug 26145: Add the new column to the column settings yml file
Bug 26145: (QA follow-up) Add missing filters
Bug 26145: DB changes - Add biblioimages.imagenumber
Bug 26145: Add the ability to upload a cover image per item
Bug 26145: Display the cover image at the OPAC
Bug 26145: Refactoring - Add tests
Bug 26145: Refactoring - Rename table biblioimages to cover_images
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages
Bug 26145: Allow multi covers per item
Bug 26145: Fix image tag
Bug 26145: Hide 'Upload image' if no CoverImage pref is set
Bug 26145: Add the "Cover image" column to the table settings
Bug 26145: Correctly preserve itemnumber value
Bug 26145: Add the new column to the column settings yml file
Bug 26145: (QA follow-up) Add missing filters

Description Jonathan Druart 2020-08-05 14:36:53 UTC
We can attach one (or more) cover image to bibliographic records, but it would be good in some cases to have the cover image attach to the item.

It can be handy for subscription for instance, where we could imagine having the cover image of each serial number displayed on the bibliographic record detail page.
Comment 1 Jonathan Druart 2020-08-16 16:56:56 UTC
Created attachment 108366 [details]
Bug 26145: DB changes - Add biblioimages.imagenumber

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 2 Jonathan Druart 2020-08-16 16:57:02 UTC
Created attachment 108367 [details]
Bug 26145: Add the ability to upload a cover image per item

This patchset adds the ability to attach cover images at item level.
This commit message will explain the different patches that are
following.

The main idea is to have cover images for a given item. This is useful
for a bibliographic record linked with a subscription. Each item could
have the cover image for the serial numbers.

In this first patch there is a limitation to allow only 1 cover per
item, but a later patch will remove it. That way we will take advantage
of the recent work done to display nicely cover images (bug 25031), and
reuse it in this development (staff interface only).

In order to use a flexible and robust code, the legacy C4 code
(C4::Images) has been moved to Koha::CoverImages. Also the DB table
biblioimages has been renamed cover_images.

Test plan (for the whole patch set):
0. Turn off AllowMultipleCovers
1. Create a new bibliographic record and items
2. Attach a cover image to the bibliographic record
3. In the item list of the bibliographic detail page, you will notice a
new "Upload image" action. Select it
4. Select an image
=> Notice the new column in the item table
5. Upload another image
=> You cannot upload 2 images, you are going to replace the existing one
6. Turn on AllowMultipleCovers
7. Attach another image to the image
=> Notice the 2 images are displayed nicely, with navigation controls
8. Confirm you can view an image if you click on it and you can delete it
9. Test the OPAC view now
=> Cover image for items are displayed in the table, there is no
navigation controls here however.

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 3 Jonathan Druart 2020-08-16 16:57:09 UTC
Created attachment 108368 [details]
Bug 26145: Display the cover image at the OPAC

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 4 Jonathan Druart 2020-08-16 16:57:16 UTC
Created attachment 108369 [details]
Bug 26145: Refactoring - Add tests

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 5 Jonathan Druart 2020-08-16 16:57:23 UTC
Created attachment 108370 [details]
Bug 26145: Refactoring - Rename table biblioimages to cover_images

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 6 Jonathan Druart 2020-08-16 16:57:31 UTC
Created attachment 108371 [details]
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 7 Jonathan Druart 2020-08-16 16:57:38 UTC
Created attachment 108372 [details]
Bug 26145: DBIC schema changes

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 8 Jonathan Druart 2020-08-16 16:57:46 UTC
Created attachment 108373 [details]
Bug 26145: Allow multi covers per item

In this patch we remove the limitation of 1 cover image per item

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 9 Jonathan Druart 2020-08-16 16:57:51 UTC
Created attachment 108374 [details]
Bug 26145: Compiled CSS

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 10 Jonathan Druart 2020-08-18 12:04:45 UTC
Created attachment 108475 [details]
Bug 26145: DBIC schema changes

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 11 Owen Leonard 2020-08-18 13:07:19 UTC
Overall this works really well. I see one small markup error:

<i class="fa fa-upload">Upload image</i>

Should be:

<i class="fa fa-upload"></i> Upload image

I notice on the detail page in the staff interface it's not possible to use the column visibility menu to hide the cover image column.

I'm concerned about the size of the item cover images in the OPAC. I think they should be constrained to something smaller. I think between 75 and 100 pixels would be good.
Comment 12 Jonathan Druart 2020-08-18 13:36:50 UTC
(In reply to Owen Leonard from comment #11)
> Overall this works really well. I see one small markup error:
> 
> <i class="fa fa-upload">Upload image</i>
> 
> Should be:
> 
> <i class="fa fa-upload"></i> Upload image

Will do

> I notice on the detail page in the staff interface it's not possible to use
> the column visibility menu to hide the cover image column.

I forgot than, good catch!

> I'm concerned about the size of the item cover images in the OPAC. I think
> they should be constrained to something smaller. I think between 75 and 100
> pixels would be good.

It's the size of the thumbnail, do you mean we should limit the thumbnail size when we display it? Why only OPAC, because staff has the slider?
Comment 13 Jonathan Druart 2020-08-18 13:37:23 UTC
> I forgot than, good catch!
I forgot that, good catch!
Comment 14 Jonathan Druart 2020-08-18 14:18:53 UTC
Created attachment 108517 [details]
Bug 26145: Fix image tag
Comment 15 Jonathan Druart 2020-08-18 14:18:56 UTC
Created attachment 108518 [details]
Bug 26145: Hide 'Upload image' if no CoverImage pref is set

If LocalCoverImages AND OPACLocalCoverImages are disabled, we should not
let the ability to upload an image for the items.

We should also hide the column "cover image" in the tables
Comment 16 Jonathan Druart 2020-08-18 14:19:00 UTC
Created attachment 108519 [details]
Bug 26145: Add the "Cover image" column to the table settings

The new column can now be hidden by the table settings
Comment 17 Clemens Bastian 2020-08-21 15:37:10 UTC
My tests resulted in the following two failed scenarios:

1) Description of the test case
1. Turn "AllowMultipleCovers" on Allow, LocalCoverImages "Display", OPACLocalCoverImages "Display"
2. Create bib record ("record 1")
3. Attach one (or more, does not matter) image to "record 1"
4. Create item ("item 1")
5. Attach one (or more, does not matter) to the "item 1"
6. Click on the image of "item 1" and delete it (or any if multiple images were added)
7.You will now see the image(s) which were added to "record 1" not images which were added to "item 1". This is a confusing in my opinion, I would expect to see the remaining images of "item 1"

2) Similar to 1) 
1. Turn "AllowMultipleCovers" on Allow, LocalCoverImages "Display", OPACLocalCoverImages "Display"  
2. Create bib record ("record 1")
3. Create item ("item 1")
4. Attach one (or more, does not matter) to the "item 1"
5. Click on the image of "item 1" and delete it (or any if multiple images were added)
6. Click on the image of "item 1" (as if you like to remove it)
7. Click on "Upload an image file" below the shown image of "item 1"
8. When you now upload an image here it will be inserted at "record 1" not on "item 1" as how I would expect it
Comment 18 Jonathan Druart 2020-08-21 16:16:45 UTC
Created attachment 108837 [details]
Bug 26145: DB changes - Add biblioimages.imagenumber

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 19 Jonathan Druart 2020-08-21 16:16:49 UTC
Created attachment 108838 [details]
Bug 26145: Add the ability to upload a cover image per item

This patchset adds the ability to attach cover images at item level.
This commit message will explain the different patches that are
following.

The main idea is to have cover images for a given item. This is useful
for a bibliographic record linked with a subscription. Each item could
have the cover image for the serial numbers.

In this first patch there is a limitation to allow only 1 cover per
item, but a later patch will remove it. That way we will take advantage
of the recent work done to display nicely cover images (bug 25031), and
reuse it in this development (staff interface only).

In order to use a flexible and robust code, the legacy C4 code
(C4::Images) has been moved to Koha::CoverImages. Also the DB table
biblioimages has been renamed cover_images.

Test plan (for the whole patch set):
0. Turn off AllowMultipleCovers
1. Create a new bibliographic record and items
2. Attach a cover image to the bibliographic record
3. In the item list of the bibliographic detail page, you will notice a
new "Upload image" action. Select it
4. Select an image
=> Notice the new column in the item table
5. Upload another image
=> You cannot upload 2 images, you are going to replace the existing one
6. Turn on AllowMultipleCovers
7. Attach another image to the image
=> Notice the 2 images are displayed nicely, with navigation controls
8. Confirm you can view an image if you click on it and you can delete it
9. Test the OPAC view now
=> Cover image for items are displayed in the table, there is no
navigation controls here however.

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 20 Jonathan Druart 2020-08-21 16:16:52 UTC
Created attachment 108839 [details]
Bug 26145: Display the cover image at the OPAC

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 21 Jonathan Druart 2020-08-21 16:16:56 UTC
Created attachment 108840 [details]
Bug 26145: Refactoring - Add tests

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 22 Jonathan Druart 2020-08-21 16:17:00 UTC
Created attachment 108841 [details]
Bug 26145: Refactoring - Rename table biblioimages to cover_images

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 23 Jonathan Druart 2020-08-21 16:17:04 UTC
Created attachment 108842 [details]
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 24 Jonathan Druart 2020-08-21 16:17:08 UTC
Created attachment 108843 [details]
Bug 26145: Allow multi covers per item

In this patch we remove the limitation of 1 cover image per item

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 25 Jonathan Druart 2020-08-21 16:17:12 UTC
Created attachment 108844 [details]
Bug 26145: Compiled CSS

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 26 Jonathan Druart 2020-08-21 16:17:16 UTC
Created attachment 108845 [details]
Bug 26145: DBIC schema changes

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 27 Jonathan Druart 2020-08-21 16:17:20 UTC
Created attachment 108846 [details]
Bug 26145: Fix image tag
Comment 28 Jonathan Druart 2020-08-21 16:17:24 UTC
Created attachment 108847 [details]
Bug 26145: Hide 'Upload image' if no CoverImage pref is set

If LocalCoverImages AND OPACLocalCoverImages are disabled, we should not
let the ability to upload an image for the items.

We should also hide the column "cover image" in the tables
Comment 29 Jonathan Druart 2020-08-21 16:17:28 UTC
Created attachment 108848 [details]
Bug 26145: Add the "Cover image" column to the table settings

The new column can now be hidden by the table settings
Comment 30 Jonathan Druart 2020-08-21 16:17:32 UTC
Created attachment 108849 [details]
Bug 26145: Correctly preserver itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.
Comment 31 Jonathan Druart 2020-08-21 16:18:11 UTC
Created attachment 108850 [details]
Bug 26145: Correctly preserve itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.
Comment 32 Jonathan Druart 2020-08-21 16:20:54 UTC
(In reply to Clemens Bastian from comment #17)

Thanks for testing! The last patch will fix the issues you found.

The whole patch set has been rebased against master.
Comment 33 Jonathan Druart 2020-08-27 13:26:53 UTC
Created attachment 109225 [details] [review]
Bug 26145: Correctly preserve itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.
Comment 34 Jonathan Druart 2020-08-27 13:26:57 UTC
Created attachment 109226 [details] [review]
Bug 26145: Use Koha::Logger and display the full stacktrace
Comment 35 Jonathan Druart 2020-09-18 10:22:35 UTC
Created attachment 110342 [details] [review]
Bug 26145: DB changes - Add biblioimages.imagenumber

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 36 Jonathan Druart 2020-09-18 10:22:41 UTC
Created attachment 110343 [details] [review]
Bug 26145: Add the ability to upload a cover image per item

This patchset adds the ability to attach cover images at item level.
This commit message will explain the different patches that are
following.

The main idea is to have cover images for a given item. This is useful
for a bibliographic record linked with a subscription. Each item could
have the cover image for the serial numbers.

In this first patch there is a limitation to allow only 1 cover per
item, but a later patch will remove it. That way we will take advantage
of the recent work done to display nicely cover images (bug 25031), and
reuse it in this development (staff interface only).

In order to use a flexible and robust code, the legacy C4 code
(C4::Images) has been moved to Koha::CoverImages. Also the DB table
biblioimages has been renamed cover_images.

Test plan (for the whole patch set):
0. Turn off AllowMultipleCovers
1. Create a new bibliographic record and items
2. Attach a cover image to the bibliographic record
3. In the item list of the bibliographic detail page, you will notice a
new "Upload image" action. Select it
4. Select an image
=> Notice the new column in the item table
5. Upload another image
=> You cannot upload 2 images, you are going to replace the existing one
6. Turn on AllowMultipleCovers
7. Attach another image to the image
=> Notice the 2 images are displayed nicely, with navigation controls
8. Confirm you can view an image if you click on it and you can delete it
9. Test the OPAC view now
=> Cover image for items are displayed in the table, there is no
navigation controls here however.

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 37 Jonathan Druart 2020-09-18 10:22:47 UTC
Created attachment 110344 [details] [review]
Bug 26145: Display the cover image at the OPAC

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 38 Jonathan Druart 2020-09-18 10:22:54 UTC
Created attachment 110345 [details] [review]
Bug 26145: Refactoring - Add tests

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 39 Jonathan Druart 2020-09-18 10:23:03 UTC
Created attachment 110346 [details] [review]
Bug 26145: Refactoring - Rename table biblioimages to cover_images

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 40 Jonathan Druart 2020-09-18 10:23:09 UTC
Created attachment 110347 [details] [review]
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 41 Jonathan Druart 2020-09-18 10:23:15 UTC
Created attachment 110348 [details] [review]
Bug 26145: Allow multi covers per item

In this patch we remove the limitation of 1 cover image per item

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 42 Jonathan Druart 2020-09-18 10:23:21 UTC
Created attachment 110349 [details] [review]
Bug 26145: Fix image tag
Comment 43 Jonathan Druart 2020-09-18 10:23:30 UTC
Created attachment 110350 [details] [review]
Bug 26145: Hide 'Upload image' if no CoverImage pref is set

If LocalCoverImages AND OPACLocalCoverImages are disabled, we should not
let the ability to upload an image for the items.

We should also hide the column "cover image" in the tables
Comment 44 Jonathan Druart 2020-09-18 10:23:38 UTC
Created attachment 110351 [details] [review]
Bug 26145: Add the "Cover image" column to the table settings

The new column can now be hidden by the table settings
Comment 45 Jonathan Druart 2020-09-18 10:23:43 UTC
Created attachment 110352 [details] [review]
Bug 26145: Correctly preserve itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.
Comment 46 Jonathan Druart 2020-09-18 10:23:55 UTC
Created attachment 110353 [details] [review]
Bug 26145: Compiled CSS
Comment 47 Jonathan Druart 2020-09-18 10:24:04 UTC
Created attachment 110354 [details] [review]
Bug 26145: DBIC schema changes
Comment 48 Jonathan Druart 2020-09-18 10:24:42 UTC
This is ready for testing again.

Patches have been rebased against master (several conflicts caused by bug 20168)
Comment 49 ByWater Sandboxes 2020-09-24 17:02:34 UTC
Created attachment 110675 [details] [review]
Bug 26145: Display the cover image at the OPAC

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 50 ByWater Sandboxes 2020-09-24 17:02:38 UTC
Created attachment 110676 [details] [review]
Bug 26145: Refactoring - Add tests

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 51 ByWater Sandboxes 2020-09-24 17:02:43 UTC
Created attachment 110677 [details] [review]
Bug 26145: Refactoring - Rename table biblioimages to cover_images

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 52 ByWater Sandboxes 2020-09-24 17:02:49 UTC
Created attachment 110678 [details] [review]
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 53 ByWater Sandboxes 2020-09-24 17:02:53 UTC
Created attachment 110679 [details] [review]
Bug 26145: Allow multi covers per item

In this patch we remove the limitation of 1 cover image per item

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 54 ByWater Sandboxes 2020-09-24 17:02:56 UTC
Created attachment 110680 [details] [review]
Bug 26145: Fix image tag

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 55 ByWater Sandboxes 2020-09-24 17:03:01 UTC
Created attachment 110681 [details] [review]
Bug 26145: Hide 'Upload image' if no CoverImage pref is set

If LocalCoverImages AND OPACLocalCoverImages are disabled, we should not
let the ability to upload an image for the items.

We should also hide the column "cover image" in the tables

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 56 ByWater Sandboxes 2020-09-24 17:03:05 UTC
Created attachment 110682 [details] [review]
Bug 26145: Add the "Cover image" column to the table settings

The new column can now be hidden by the table settings

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 57 ByWater Sandboxes 2020-09-24 17:03:11 UTC
Created attachment 110683 [details] [review]
Bug 26145: Correctly preserve itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 58 ByWater Sandboxes 2020-09-24 17:03:16 UTC
Created attachment 110684 [details] [review]
Bug 26145: Compiled CSS

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 59 ByWater Sandboxes 2020-09-24 17:03:21 UTC
Created attachment 110685 [details] [review]
Bug 26145: DBIC schema changes

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 60 Heather 2020-09-24 17:06:36 UTC
Going through the steps of the test plan, everything went exactly as it should!  Yea!
Comment 61 Katrin Fischer 2020-09-26 21:28:26 UTC
1) Fixed some missing filters - posting follow-up.

2) Hm, the 2 separate updatedatabase statements are a bit confusing. Please merge them.

3) New column is not added to the column configuration for the items table in the OPAC. I've tried to fix it, but just adding it to the .yml didn't have the intended effect. Can you please have a look?

The fail is for 3, overall this works well already :)

Note: I think it would have been nice to have this split into several bugs, starting with the rewrite of C4::Images into the Koha namespace for the existing functionality.
Comment 62 Katrin Fischer 2020-09-26 21:28:57 UTC
Created attachment 110800 [details] [review]
Bug 26145: (QA follow-up) Add missing filters
Comment 63 Jonathan Druart 2020-09-28 15:02:17 UTC
Created attachment 110861 [details] [review]
Bug 26145: DB changes - Add biblioimages.imagenumber

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 64 Jonathan Druart 2020-09-28 15:02:24 UTC
Created attachment 110862 [details] [review]
Bug 26145: Add the ability to upload a cover image per item

This patchset adds the ability to attach cover images at item level.
This commit message will explain the different patches that are
following.

The main idea is to have cover images for a given item. This is useful
for a bibliographic record linked with a subscription. Each item could
have the cover image for the serial numbers.

In this first patch there is a limitation to allow only 1 cover per
item, but a later patch will remove it. That way we will take advantage
of the recent work done to display nicely cover images (bug 25031), and
reuse it in this development (staff interface only).

In order to use a flexible and robust code, the legacy C4 code
(C4::Images) has been moved to Koha::CoverImages. Also the DB table
biblioimages has been renamed cover_images.

Test plan (for the whole patch set):
0. Turn off AllowMultipleCovers
1. Create a new bibliographic record and items
2. Attach a cover image to the bibliographic record
3. In the item list of the bibliographic detail page, you will notice a
new "Upload image" action. Select it
4. Select an image
=> Notice the new column in the item table
5. Upload another image
=> You cannot upload 2 images, you are going to replace the existing one
6. Turn on AllowMultipleCovers
7. Attach another image to the image
=> Notice the 2 images are displayed nicely, with navigation controls
8. Confirm you can view an image if you click on it and you can delete it
9. Test the OPAC view now
=> Cover image for items are displayed in the table, there is no
navigation controls here however.

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 65 Jonathan Druart 2020-09-28 15:02:28 UTC
Created attachment 110863 [details] [review]
Bug 26145: Display the cover image at the OPAC

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 66 Jonathan Druart 2020-09-28 15:02:34 UTC
Created attachment 110864 [details] [review]
Bug 26145: Refactoring - Add tests

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 67 Jonathan Druart 2020-09-28 15:02:40 UTC
Created attachment 110865 [details] [review]
Bug 26145: Refactoring - Rename table biblioimages to cover_images

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 68 Jonathan Druart 2020-09-28 15:02:48 UTC
Created attachment 110866 [details] [review]
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 69 Jonathan Druart 2020-09-28 15:02:54 UTC
Created attachment 110867 [details] [review]
Bug 26145: Allow multi covers per item

In this patch we remove the limitation of 1 cover image per item

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Comment 70 Jonathan Druart 2020-09-28 15:03:00 UTC
Created attachment 110868 [details] [review]
Bug 26145: Fix image tag
Comment 71 Jonathan Druart 2020-09-28 15:03:05 UTC
Created attachment 110869 [details] [review]
Bug 26145: Hide 'Upload image' if no CoverImage pref is set

If LocalCoverImages AND OPACLocalCoverImages are disabled, we should not
let the ability to upload an image for the items.

We should also hide the column "cover image" in the tables
Comment 72 Jonathan Druart 2020-09-28 15:03:11 UTC
Created attachment 110870 [details] [review]
Bug 26145: Add the "Cover image" column to the table settings

The new column can now be hidden by the table settings
Comment 73 Jonathan Druart 2020-09-28 15:03:16 UTC
Created attachment 110871 [details] [review]
Bug 26145: Correctly preserve itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.
Comment 74 Jonathan Druart 2020-09-28 15:03:23 UTC
Created attachment 110872 [details] [review]
Bug 26145: DBIC schema changes
Comment 75 Jonathan Druart 2020-09-28 15:03:31 UTC
Created attachment 110873 [details] [review]
Bug 26145: Compiled CSS
Comment 76 Jonathan Druart 2020-09-28 15:03:43 UTC
Created attachment 110874 [details] [review]
Bug 26145: Add the new column to the column settings yml file
Comment 77 Jonathan Druart 2020-09-28 15:03:50 UTC
Created attachment 110875 [details] [review]
Bug 26145: (QA follow-up) Add missing filters
Comment 78 Jonathan Druart 2020-09-28 15:06:10 UTC
(In reply to Katrin Fischer from comment #61)
> 1) Fixed some missing filters - posting follow-up.

Thanks!

> 2) Hm, the 2 separate updatedatabase statements are a bit confusing. Please
> merge them.

Ok, I will merge them before pushing them.
They are 2 different things, so I wanted to make it clear in the output.

> 3) New column is not added to the column configuration for the items table
> in the OPAC. I've tried to fix it, but just adding it to the .yml didn't
> have the intended effect. Can you please have a look?

Done.

> Note: I think it would have been nice to have this split into several bugs,
> starting with the rewrite of C4::Images into the Koha namespace for the
> existing functionality.

I took advantage of being in this part of the code to move legacy C4 code to the Koha namespace. I did not plan to do that before writing the first patches (hence it is not done at the beginning of the patch stack).
The only thing I could have separated is the "allow multiple covers", but I thought it was good to have it from scratch (as it is the current behaviour for biblio records).
Comment 79 Katrin Fischer 2020-10-11 12:50:24 UTC
Created attachment 111446 [details] [review]
Bug 26145: DB changes - Add biblioimages.imagenumber

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 80 Katrin Fischer 2020-10-11 12:50:33 UTC
Created attachment 111447 [details] [review]
Bug 26145: Add the ability to upload a cover image per item

This patchset adds the ability to attach cover images at item level.
This commit message will explain the different patches that are
following.

The main idea is to have cover images for a given item. This is useful
for a bibliographic record linked with a subscription. Each item could
have the cover image for the serial numbers.

In this first patch there is a limitation to allow only 1 cover per
item, but a later patch will remove it. That way we will take advantage
of the recent work done to display nicely cover images (bug 25031), and
reuse it in this development (staff interface only).

In order to use a flexible and robust code, the legacy C4 code
(C4::Images) has been moved to Koha::CoverImages. Also the DB table
biblioimages has been renamed cover_images.

Test plan (for the whole patch set):
0. Turn off AllowMultipleCovers
1. Create a new bibliographic record and items
2. Attach a cover image to the bibliographic record
3. In the item list of the bibliographic detail page, you will notice a
new "Upload image" action. Select it
4. Select an image
=> Notice the new column in the item table
5. Upload another image
=> You cannot upload 2 images, you are going to replace the existing one
6. Turn on AllowMultipleCovers
7. Attach another image to the image
=> Notice the 2 images are displayed nicely, with navigation controls
8. Confirm you can view an image if you click on it and you can delete it
9. Test the OPAC view now
=> Cover image for items are displayed in the table, there is no
navigation controls here however.

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 81 Katrin Fischer 2020-10-11 12:50:39 UTC
Created attachment 111448 [details] [review]
Bug 26145: Display the cover image at the OPAC

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 82 Katrin Fischer 2020-10-11 12:50:46 UTC
Created attachment 111449 [details] [review]
Bug 26145: Refactoring - Add tests

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 83 Katrin Fischer 2020-10-11 12:50:53 UTC
Created attachment 111450 [details] [review]
Bug 26145: Refactoring - Rename table biblioimages to cover_images

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 84 Katrin Fischer 2020-10-11 12:51:00 UTC
Created attachment 111451 [details] [review]
Bug 26145: Refactoring - Move C4::Images to Koha::CoverImages

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 85 Katrin Fischer 2020-10-11 12:51:10 UTC
Created attachment 111452 [details] [review]
Bug 26145: Allow multi covers per item

In this patch we remove the limitation of 1 cover image per item

Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 86 Katrin Fischer 2020-10-11 12:51:26 UTC
Created attachment 111453 [details] [review]
Bug 26145: Fix image tag

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 87 Katrin Fischer 2020-10-11 12:51:36 UTC
Created attachment 111454 [details] [review]
Bug 26145: Hide 'Upload image' if no CoverImage pref is set

If LocalCoverImages AND OPACLocalCoverImages are disabled, we should not
let the ability to upload an image for the items.

We should also hide the column "cover image" in the tables

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 88 Katrin Fischer 2020-10-11 12:51:43 UTC
Created attachment 111455 [details] [review]
Bug 26145: Add the "Cover image" column to the table settings

The new column can now be hidden by the table settings

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 89 Katrin Fischer 2020-10-11 12:51:49 UTC
Created attachment 111456 [details] [review]
Bug 26145: Correctly preserve itemnumber value

In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 90 Katrin Fischer 2020-10-11 12:51:55 UTC
Created attachment 111457 [details] [review]
Bug 26145: Add the new column to the column settings yml file

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 91 Katrin Fischer 2020-10-11 12:52:00 UTC
Created attachment 111458 [details] [review]
Bug 26145: (QA follow-up) Add missing filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 92 Katrin Fischer 2020-10-11 12:55:38 UTC
I had to skip over the compiled CSS and DBIC patches when applying the patch set and ran those locally.

One last idea: As we rename a table with a nice name here, we could add another database statement to replace the table name (old to new) in any reports that libraries might have written.
Comment 93 Jonathan Druart 2020-10-12 09:30:46 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 94 Jonathan Druart 2020-10-12 09:37:46 UTC
(In reply to Katrin Fischer from comment #92)
> One last idea: As we rename a table with a nice name here, we could add
> another database statement to replace the table name (old to new) in any
> reports that libraries might have written.

We could do that but are you aware of people using this table in reports?
Comment 95 Katrin Fischer 2020-10-12 09:39:39 UTC
(In reply to Jonathan Druart from comment #94)
> (In reply to Katrin Fischer from comment #92)
> > One last idea: As we rename a table with a nice name here, we could add
> > another database statement to replace the table name (old to new) in any
> > reports that libraries might have written.
> 
> We could do that but are you aware of people using this table in reports?

I am not sure, could it be used in plugins for cover display? Or someone looking for records without added covers yet. It was not a blocker to me, but it might be an easy one in this case that could make things go smoother.
Comment 96 Lucas Gass 2020-11-10 22:46:47 UTC
nice enhancement but will not be backporting to 20.05.x
Comment 97 Jonathan Druart 2020-11-11 09:41:51 UTC
(In reply to Katrin Fischer from comment #95)
> (In reply to Jonathan Druart from comment #94)
> > (In reply to Katrin Fischer from comment #92)
> > > One last idea: As we rename a table with a nice name here, we could add
> > > another database statement to replace the table name (old to new) in any
> > > reports that libraries might have written.
> > 
> > We could do that but are you aware of people using this table in reports?
> 
> I am not sure, could it be used in plugins for cover display? Or someone
> looking for records without added covers yet. It was not a blocker to me,
> but it might be an easy one in this case that could make things go smoother.

Sorry, I forgot about that.
I think I disagreed and that's why I did not answer at the time.

And I still don't think it's a good idea. We could replace "biblioimages" with "cover_images" in the reports, but we could might introduce regressions, with bad side-effects.
The error "table does not exist" will be trivial to fix for anyone knowing how to write SQL code.
However I am inclined to do it if you strongly feel it's now needed (but it's not what you thought apparently).
Comment 98 Katrin Fischer 2020-11-11 10:02:25 UTC
No, I don't feel strongly and your argument is good too. We can leave that as is I think.