Bug 20502 - Record URL broken, when there is more than one 856 on the record
Summary: Record URL broken, when there is more than one 856 on the record
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: master
Hardware: All All
: P5 - low normal with 1 vote (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-30 12:42 UTC by Katrin Fischer
Modified: 2021-02-18 10:52 UTC (History)
5 users (show)

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


Attachments
Bug 20502: Split URI on course detail view (2.68 KB, patch)
2020-08-20 08:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20502: Split URI on course detail view (2.74 KB, patch)
2020-08-20 12:52 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 20502: Split URI on course detail view - OPAC (2.63 KB, patch)
2020-08-21 07:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20502: Fix url vs uri (2.81 KB, patch)
2021-02-10 07:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20502: Replace texts with 'Link to resource' (4.30 KB, patch)
2021-02-10 07:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20502: Split URI on course detail view (2.84 KB, patch)
2021-02-12 08:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20502: Split URI on course detail view - OPAC (2.73 KB, patch)
2021-02-12 08:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20502: Fix url vs uri (2.92 KB, patch)
2021-02-12 08:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[NOT_SIGNED] Bug 20502: Replace texts with 'Link to resource' (4.35 KB, patch)
2021-02-12 08:33 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 Katrin Fischer 2018-03-30 12:42:06 UTC
When there is more than one 856 in the record, the Record URL in the course reserves table creates a broken link. Example form the sample data:

http://www.loc.gov/catdir/enhancements/fy1010/2009039494-b.html%20|%20http://www.loc.gov/catdir/enhancements/fy1010/2009039494-d.html
Comment 1 Jonathan Druart 2018-04-02 17:19:53 UTC
We should not use biblioitems.url but GetMarcUrls instead
Comment 2 Katrin Fischer 2019-09-05 15:56:28 UTC
Still valid in 18.11 at least, possibly master as well.
Comment 3 Donna 2020-08-19 20:29:02 UTC
This is still an issue in 19.11.
Comment 4 Jonathan Druart 2020-08-20 08:00:37 UTC
Created attachment 108712 [details] [review]
Bug 20502: Split URI on course detail view

If there are more that one URIs for biblio or item, we should split the
value.
This patch reuses the same code as bug 24697.

Test plan:
Add one item to a course
Add several URI for this item and for its biblio
Show the course detail view
The different URI of the item must be displayed
Remove all URI for the item
Show the course detail view
The different URI of the biblio must be displayed
Comment 5 ByWater Sandboxes 2020-08-20 12:52:17 UTC
Created attachment 108737 [details] [review]
Bug 20502: Split URI on course detail view

If there are more that one URIs for biblio or item, we should split the
value.
This patch reuses the same code as bug 24697.

Test plan:
Add one item to a course
Add several URI for this item and for its biblio
Show the course detail view
The different URI of the item must be displayed
Remove all URI for the item
Show the course detail view
The different URI of the biblio must be displayed

Signed-off-by: Donna Bachowski <donna@bywatersolutions.com>
Comment 6 Katrin Fischer 2020-08-20 21:52:31 UTC
This works for staff, can we have the same change for OPAC please?
Comment 7 Jonathan Druart 2020-08-21 07:01:13 UTC
Created attachment 108775 [details] [review]
Bug 20502: Split URI on course detail view - OPAC
Comment 8 Katrin Fischer 2020-08-21 21:57:05 UTC
I am not sure about the behaviour change here where we display the URL when there is more than one. At the moment these display not very nicely in the table :(

I think the best would be to display "Item URL" for item URLs as it's now and display the link text for 856 or keept the current "Record URL" (first is a wish... second probably doable).

What do you think?

There is also a bug when there is only one URL for a course item:
I think
cr.biblioitem.url.split should be cr.biblioitem.uri.split
Comment 9 Jonathan Druart 2021-02-10 07:46:50 UTC
Created attachment 116636 [details] [review]
Bug 20502: Fix url vs uri
Comment 10 Jonathan Druart 2021-02-10 07:46:54 UTC
Created attachment 116637 [details] [review]
Bug 20502: Replace texts with 'Link to resource'
Comment 11 Jonathan Druart 2021-02-10 07:47:13 UTC
(In reply to Katrin Fischer from comment #8)
> I am not sure about the behaviour change here where we display the URL when
> there is more than one. At the moment these display not very nicely in the
> table :(
> 
> I think the best would be to display "Item URL" for item URLs as it's now
> and display the link text for 856 or keept the current "Record URL" (first
> is a wish... second probably doable).
> 
> What do you think?

I don't understand, can you rephrase? My patch is applying what is done on catalogue/detail.pl, so not sure how it is displaying "not very nicely".
There is an optional follow-up patch that replaces the text with "Link to resource"

> There is also a bug when there is only one URL for a course item:
> I think
> cr.biblioitem.url.split should be cr.biblioitem.uri.split

Fixed now.
Comment 12 Marcel de Rooy 2021-02-12 08:30:10 UTC
The last patch uses the same text in a foreach. Looks very ugly?
I would stick to your original plan to mimic catalogue/detail.
Comment 13 Marcel de Rooy 2021-02-12 08:33:21 UTC
Created attachment 116788 [details] [review]
Bug 20502: Split URI on course detail view

If there are more that one URIs for biblio or item, we should split the
value.
This patch reuses the same code as bug 24697.

Test plan:
Add one item to a course
Add several URI for this item and for its biblio
Show the course detail view
The different URI of the item must be displayed
Remove all URI for the item
Show the course detail view
The different URI of the biblio must be displayed

Signed-off-by: Donna Bachowski <donna@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2021-02-12 08:33:25 UTC
Created attachment 116789 [details] [review]
Bug 20502: Split URI on course detail view - OPAC

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2021-02-12 08:33:28 UTC
Created attachment 116790 [details] [review]
Bug 20502: Fix url vs uri

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2021-02-12 08:33:32 UTC
Created attachment 116791 [details] [review]
[NOT_SIGNED] Bug 20502: Replace texts with 'Link to resource'
Comment 17 Marcel de Rooy 2021-02-12 08:34:53 UTC
I did not signoff on the last one ;)
Comment 18 Jonathan Druart 2021-02-18 10:52:38 UTC
Moved to In discussion for the last patch.