Description
Katrin Fischer
2018-03-30 12:42:06 UTC
We should not use biblioitems.url but GetMarcUrls instead Still valid in 18.11 at least, possibly master as well. This is still an issue in 19.11. 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 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> This works for staff, can we have the same change for OPAC please? Created attachment 108775 [details] [review] Bug 20502: Split URI on course detail view - OPAC 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 Created attachment 116636 [details] [review] Bug 20502: Fix url vs uri Created attachment 116637 [details] [review] Bug 20502: Replace texts with 'Link to resource' (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. The last patch uses the same text in a foreach. Looks very ugly? I would stick to your original plan to mimic catalogue/detail. 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> 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> Created attachment 116790 [details] [review] Bug 20502: Fix url vs uri Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 116791 [details] [review] [NOT_SIGNED] Bug 20502: Replace texts with 'Link to resource' I did not signoff on the last one ;) Moved to In discussion for the last patch. This is stuck, how do we move forward? *** Bug 18599 has been marked as a duplicate of this bug. *** |