|
Description
Jonathan Druart
2020-08-05 14:36:53 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) 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) 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) Created attachment 108369 [details] Bug 26145: Refactoring - Add tests Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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) 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) Created attachment 108372 [details] Bug 26145: DBIC schema changes Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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) Created attachment 108374 [details] Bug 26145: Compiled CSS Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) Created attachment 108475 [details] Bug 26145: DBIC schema changes Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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. (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? > I forgot than, good catch!
I forgot that, good catch!
Created attachment 108517 [details] Bug 26145: Fix image tag 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 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 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
Created attachment 108837 [details] Bug 26145: DB changes - Add biblioimages.imagenumber Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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) 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) Created attachment 108840 [details] Bug 26145: Refactoring - Add tests Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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) 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) 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) Created attachment 108844 [details] Bug 26145: Compiled CSS Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) Created attachment 108845 [details] Bug 26145: DBIC schema changes Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) Created attachment 108846 [details] Bug 26145: Fix image tag 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 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 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. 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. (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. 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. Created attachment 109226 [details] [review] Bug 26145: Use Koha::Logger and display the full stacktrace 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) 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) 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) Created attachment 110345 [details] [review] Bug 26145: Refactoring - Add tests Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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) 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) 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) Created attachment 110349 [details] [review] Bug 26145: Fix image tag 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 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 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. Created attachment 110353 [details] [review] Bug 26145: Compiled CSS Created attachment 110354 [details] [review] Bug 26145: DBIC schema changes This is ready for testing again. Patches have been rebased against master (several conflicts caused by bug 20168) 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> 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> 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> 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> 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> Created attachment 110680 [details] [review] Bug 26145: Fix image tag Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov> 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> 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> 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> Created attachment 110684 [details] [review] Bug 26145: Compiled CSS Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov> Created attachment 110685 [details] [review] Bug 26145: DBIC schema changes Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov> Going through the steps of the test plan, everything went exactly as it should! Yea! 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. Created attachment 110800 [details] [review] Bug 26145: (QA follow-up) Add missing filters 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) 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) 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) Created attachment 110864 [details] [review] Bug 26145: Refactoring - Add tests Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch) 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) 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) 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) Created attachment 110868 [details] [review] Bug 26145: Fix image tag 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 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 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. Created attachment 110872 [details] [review] Bug 26145: DBIC schema changes Created attachment 110873 [details] [review] Bug 26145: Compiled CSS Created attachment 110874 [details] [review] Bug 26145: Add the new column to the column settings yml file Created attachment 110875 [details] [review] Bug 26145: (QA follow-up) Add missing filters (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). 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> 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> 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> 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> 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> 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> 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> Created attachment 111453 [details] [review] Bug 26145: Fix image tag Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> 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> 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> 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> 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> Created attachment 111458 [details] [review] Bug 26145: (QA follow-up) Add missing filters Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> 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. Pushed to master for 20.11, thanks to everybody involved! (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? (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. nice enhancement but will not be backporting to 20.05.x (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). No, I don't feel strongly and your argument is good too. We can leave that as is I think. |