Bug 36966 - Fix links for local cover images for items on staff detail page
Summary: Fix links for local cover images for items on staff detail page
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on: 28179
Blocks:
  Show dependency treegraph
 
Reported: 2024-05-27 03:10 UTC by David Cook
Modified: 2024-07-22 10:44 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the local cover image links for items (staff interface record details holdings table > dropdown link for Edit > Upload image) by removing unnecessary parameters, fixing an invalid link, an uninitialised Template::Toolkit variable. This has no noticeable effect, but is important for avoiding future issues.
Version(s) released in:
24.11.00,24.05.02
Circulation function:


Attachments
Bug 36966: Fix local cover image links for items (2.30 KB, patch)
2024-05-27 03:35 UTC, David Cook
Details | Diff | Splinter Review
Bug 36966: Fix local cover image links for items (2.35 KB, patch)
2024-05-27 20:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 36966: Fix data-link for local cover images (1.71 KB, patch)
2024-05-29 07:27 UTC, David Cook
Details | Diff | Splinter Review
Bug 36966: Fix local cover image links for items (2.35 KB, patch)
2024-05-30 02:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 36966: Fix data-link for local cover images (1.75 KB, patch)
2024-05-30 02:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 36966: Actually fix data-link for local cover images (1.65 KB, patch)
2024-05-30 04:17 UTC, David Cook
Details | Diff | Splinter Review
Bug 36966: Fix local cover image links for items (2.35 KB, patch)
2024-05-30 05:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 36966: Fix data-link for local cover images (1.75 KB, patch)
2024-05-30 05:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 36966: Actually fix data-link for local cover images (1.69 KB, patch)
2024-05-30 05:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 36966: Fix local cover image links for items (2.40 KB, patch)
2024-06-25 17:43 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36966: Fix data-link for local cover images (1.81 KB, patch)
2024-06-25 17:44 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36966: Actually fix data-link for local cover images (1.75 KB, patch)
2024-06-25 17:44 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2024-05-27 03:10:43 UTC
The links for local cover images aren't built correctly on the detail page of the staff interface

While it doesn't cause a problem on current "main", any development that sets the "item" variable in the template at a higher level will generate fatal errors. Owen has bumped into this in his Bootstrap 5 work.
Comment 1 David Cook 2024-05-27 03:13:03 UTC
(Btw as I fix this... I think I notice we're still using & to separate query parameters in URLs when we should be using &. It's a reserved character for that purpose.)
Comment 2 David Cook 2024-05-27 03:35:19 UTC
Created attachment 167187 [details] [review]
Bug 36966: Fix local cover image links for items

This change fixes the local cover image links for items by removing
unnecessary parameters, an invalid link, and an uninitialised
Template::Toolkit variable.

Test plan:
0. Apply patch
1. Enable syspref 'LocalCoverImages'
3. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4. Click on the dropdown next to "Edit" at the item level
5. Upload an image for the item
6. Note that the local cover image appears in the holdings table
7. Click on the image and note that it loads in the light box
8. Success!
Comment 3 Jonathan Druart 2024-05-27 14:53:35 UTC
Should this data-link attr removed from other places?
Comment 4 David Nind 2024-05-27 20:02:35 UTC
Created attachment 167195 [details] [review]
Bug 36966: Fix local cover image links for items

This change fixes the local cover image links for items by removing
unnecessary parameters, an invalid link, and an uninitialised
Template::Toolkit variable.

Test plan:
0. Apply patch
1. Enable syspref 'LocalCoverImages'
3. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4. Click on the dropdown next to "Edit" at the item level
5. Upload an image for the item
6. Note that the local cover image appears in the holdings table
7. Click on the image and note that it loads in the light box
8. Success!

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Cook 2024-05-27 23:04:21 UTC
(In reply to Jonathan Druart from comment #3)
> Should this data-link attr removed from other places?

Actually, I think I shouldn't have removed it here.

I couldn't see anywhere it was used, but looking again today I can see that at the very bottom of light box page it should say "Local cover image (edit)" and that has a link to the editor. 

I do have a way of making that work. I'll take a look...
Comment 6 David Cook 2024-05-29 07:18:31 UTC
Yeah nah I definitely broke it in a different way..
Comment 7 David Cook 2024-05-29 07:27:18 UTC
Created attachment 167236 [details] [review]
Bug 36966: Fix data-link for local cover images

This patch fixes the data-link so that the "edit" link at the
bottom of the lightbox viewer correctly takes the user to
the editor interface for the local cover image
Comment 8 David Nind 2024-05-30 02:35:25 UTC
Created attachment 167254 [details] [review]
Bug 36966: Fix local cover image links for items

This change fixes the local cover image links for items by removing
unnecessary parameters, an invalid link, and an uninitialised
Template::Toolkit variable.

Test plan:
0. Apply patch
1. Enable syspref 'LocalCoverImages'
3. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4. Click on the dropdown next to "Edit" at the item level
5. Upload an image for the item
6. Note that the local cover image appears in the holdings table
7. Click on the image and note that it loads in the light box
8. Success!

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2024-05-30 02:35:27 UTC
Created attachment 167255 [details] [review]
Bug 36966: Fix data-link for local cover images

This patch fixes the data-link so that the "edit" link at the
bottom of the lightbox viewer correctly takes the user to
the editor interface for the local cover image

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2024-05-30 03:09:51 UTC
I think this works... but it seems the only way to replace the item-level image is via Edit > Upload image.

Before patch applied:

0. Enable LocalCoverImages system preference

1. Upload a record-level image:
   1.1 Upload a record-level image: Images tab > Upload
   1.2 Upload an item-level image: Holding stab > Edit > Upload image > [use a different image from the record-level one)
   ==> Result: Different record-level and item-level images

2. Replace the item-level image (Edit > Upload image):
   2.1 Replace the item-level cover image: Holdings tab > Edit > Upload image > [use a different image from the record-level image]
   ==> Result: item-level image replaced (as expected)

3. Replace the item-level image (click on cover image in holdings table):
   3.1 Attempt to replace the item-level image: click on the item-level cover under the cover image column > click edit in the light-box viewer > get error trace:
    Can't call method "biblionumber" on an undefined value at /kohadevbox/koha/catalogue/imageviewer.pl line 42

After the patches are applied:

1. Repeat 3.1: displays all existing images (record and item-level images), and can "Drop files here or click to select a file". Drag and drop a replacement image: 
   ==> Result: 
       . Record-level image replaced
       . For search results, item-level image displayed

Hope I haven't confused things!
Comment 11 David Cook 2024-05-30 04:12:36 UTC
(In reply to David Nind from comment #10)
> Hope I haven't confused things!

Nah, I think it's a good catch. I think I've just confused myself a bit previously. Too much multitasking...
Comment 12 David Cook 2024-05-30 04:17:35 UTC
Created attachment 167259 [details] [review]
Bug 36966: Actually fix data-link for local cover images
Comment 13 David Cook 2024-05-30 04:17:53 UTC
Ok... "now" it should work haha.
Comment 14 David Nind 2024-05-30 05:45:30 UTC
(In reply to David Cook from comment #13)
> Ok... "now" it should work haha.

Thanks David. Sign off on the way.

Thinks I learnt with this big, you can have item-level cover images.

David
Comment 15 David Nind 2024-05-30 05:46:20 UTC
Created attachment 167263 [details] [review]
Bug 36966: Fix local cover image links for items

This change fixes the local cover image links for items by removing
unnecessary parameters, an invalid link, and an uninitialised
Template::Toolkit variable.

Test plan:
0. Apply patch
1. Enable syspref 'LocalCoverImages'
3. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4. Click on the dropdown next to "Edit" at the item level
5. Upload an image for the item
6. Note that the local cover image appears in the holdings table
7. Click on the image and note that it loads in the light box
8. Success!

Signed-off-by: David Nind <david@davidnind.com>
Comment 16 David Nind 2024-05-30 05:46:23 UTC
Created attachment 167264 [details] [review]
Bug 36966: Fix data-link for local cover images

This patch fixes the data-link so that the "edit" link at the
bottom of the lightbox viewer correctly takes the user to
the editor interface for the local cover image

Signed-off-by: David Nind <david@davidnind.com>
Comment 17 David Nind 2024-05-30 05:46:26 UTC
Created attachment 167265 [details] [review]
Bug 36966: Actually fix data-link for local cover images

Signed-off-by: David Nind <david@davidnind.com>
Comment 18 Nick Clemens (kidclamp) 2024-06-25 17:43:59 UTC
Created attachment 168090 [details] [review]
Bug 36966: Fix local cover image links for items

This change fixes the local cover image links for items by removing
unnecessary parameters, an invalid link, and an uninitialised
Template::Toolkit variable.

Test plan:
0. Apply patch
1. Enable syspref 'LocalCoverImages'
3. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4. Click on the dropdown next to "Edit" at the item level
5. Upload an image for the item
6. Note that the local cover image appears in the holdings table
7. Click on the image and note that it loads in the light box
8. Success!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens (kidclamp) 2024-06-25 17:44:01 UTC
Created attachment 168091 [details] [review]
Bug 36966: Fix data-link for local cover images

This patch fixes the data-link so that the "edit" link at the
bottom of the lightbox viewer correctly takes the user to
the editor interface for the local cover image

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens (kidclamp) 2024-06-25 17:44:03 UTC
Created attachment 168092 [details] [review]
Bug 36966: Actually fix data-link for local cover images

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Katrin Fischer 2024-06-27 09:52:41 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 22 Lucas Gass 2024-07-19 15:16:55 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 23 Fridolin Somers 2024-07-22 09:45:01 UTC
Does not apply on 23.11.x
Looks like this depends on Bug 33568 right ?