Bug 8995 - Show OpenURL links in OPAC search results
Summary: Show OpenURL links in OPAC search results
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-31 13:48 UTC by Julian Maurice
Modified: 2019-05-18 09:27 UTC (History)
19 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
This new feature allows you to link documents in your catalogue to an OpenURL resolver and possibly enable your patrons to get full texts in digital form.
Version(s) released in:
19.05.00


Attachments
Bug 8995: Show OpenURL links in OPAC search results (20.07 KB, patch)
2012-10-31 13:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (35.08 KB, patch)
2013-11-06 15:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (35.04 KB, patch)
2013-12-18 11:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Use uri_escape_utf8 instead of uri_escape (709 bytes, patch)
2013-12-18 11:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in bootstrap theme (5.23 KB, patch)
2013-12-18 11:02 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (35.07 KB, patch)
2014-01-09 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Use uri_escape_utf8 instead of uri_escape (709 bytes, patch)
2014-01-09 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in bootstrap theme (5.23 KB, patch)
2014-01-09 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (35.05 KB, patch)
2014-04-08 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Use uri_escape_utf8 instead of uri_escape (709 bytes, patch)
2014-04-08 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in bootstrap theme (5.06 KB, patch)
2014-04-08 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (35.21 KB, patch)
2014-06-30 10:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Use uri_escape_utf8 instead of uri_escape (707 bytes, patch)
2014-06-30 10:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in bootstrap theme (5.06 KB, patch)
2014-06-30 10:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (35.21 KB, patch)
2014-07-31 09:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Use uri_escape_utf8 instead of uri_escape (709 bytes, patch)
2014-07-31 09:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in bootstrap theme (5.06 KB, patch)
2014-07-31 09:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Add OpenURL link to OPAC bootstrap XSLT files (11.99 KB, patch)
2014-07-31 09:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Change meaning and name of syspref OPACHideOpenURLForItemTypes (5.93 KB, patch)
2014-07-31 09:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Consider OPACURLOpenInNewWindow for OpenURL links (6.45 KB, patch)
2014-07-31 10:07 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (37.91 KB, patch)
2015-02-09 12:22 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED OFF] Bug 8995: Show OpenURL links in OPAC search results (38.86 KB, patch)
2015-02-13 16:39 UTC, Paola Rossi
Details | Diff | Splinter Review
[SIGNED OFF] Bug 8995: Show OpenURL links in OPAC search results (121.91 KB, patch)
2015-06-26 07:41 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED OFF] Bug 8995: Show OpenURL links in OPAC search results (121.90 KB, patch)
2015-11-09 10:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (31.87 KB, patch)
2016-02-25 15:18 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Update opac.css (80.82 KB, patch)
2016-02-25 15:18 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (31.90 KB, patch)
2016-09-02 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Update opac.css (80.75 KB, patch)
2016-09-02 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Remove useless code (2.02 KB, patch)
2016-09-02 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Fix URL generation when the URL in syspref has a query part (8.49 KB, patch)
2016-09-02 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Tell about the new sysprefs in COinSinOPACResults description (1.29 KB, patch)
2016-09-02 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Fix and simplify usage of syspref OpenURLImageLocation (7.92 KB, patch)
2016-09-02 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.09 KB, patch)
2016-09-28 12:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (31.98 KB, patch)
2017-06-22 07:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Update opac.css (81.87 KB, patch)
2017-06-22 07:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Remove useless code (2.03 KB, patch)
2017-06-22 07:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Fix URL generation when the URL in syspref has a query part (8.53 KB, patch)
2017-06-22 07:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Tell about the new sysprefs in COinSinOPACResults description (1.29 KB, patch)
2017-06-22 07:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Fix and simplify usage of syspref OpenURLImageLocation (7.95 KB, patch)
2017-06-22 07:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.10 KB, patch)
2017-06-22 07:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (31.96 KB, patch)
2017-06-22 09:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Update opac.css (81.91 KB, patch)
2017-06-22 09:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Remove useless code (2.07 KB, patch)
2017-06-22 09:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Fix URL generation when the URL in syspref has a query part (8.54 KB, patch)
2017-06-22 09:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Tell about the new sysprefs in COinSinOPACResults description (1.34 KB, patch)
2017-06-22 09:02 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Fix and simplify usage of syspref OpenURLImageLocation (7.97 KB, patch)
2017-06-22 09:02 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.14 KB, patch)
2017-06-22 09:02 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (32.20 KB, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Remove useless code (2.07 KB, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Fix URL generation when the URL in syspref has a query part (8.54 KB, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Tell about the new sysprefs in COinSinOPACResults description (1.34 KB, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Fix and simplify usage of syspref OpenURLImageLocation (7.97 KB, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.14 KB, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Adapt to current codebase (911 bytes, patch)
2019-03-11 21:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (28.94 KB, patch)
2019-03-11 21:41 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.14 KB, patch)
2019-03-11 21:41 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Adapt to current codebase (911 bytes, patch)
2019-03-11 21:41 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Added test (1.85 KB, patch)
2019-03-12 13:50 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL (738 bytes, patch)
2019-03-12 13:50 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl (3.95 KB, patch)
2019-03-13 08:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins (22.36 KB, patch)
2019-03-13 08:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add one more test fot get_openurl (1.29 KB, patch)
2019-03-13 08:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Tak system preference item-level_itypes into account (1.46 KB, patch)
2019-03-14 13:24 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (29.00 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.15 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Adapt to current codebase (969 bytes, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Added test (1.90 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL (789 bytes, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl (4.00 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins (22.41 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add one more test fot get_openurl (1.34 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: (follow-up) Tak system preference item-level_itypes into account (1.72 KB, patch)
2019-03-14 13:37 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (29.01 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.15 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Adapt to current codebase (969 bytes, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Added test (1.72 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL (789 bytes, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl (4.00 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins (22.43 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add one more test fot get_openurl (1.34 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Tak system preference item-level_itypes into account (1.72 KB, patch)
2019-04-24 22:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (29.07 KB, patch)
2019-04-25 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.21 KB, patch)
2019-04-25 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Adapt to current codebase (1.01 KB, patch)
2019-04-25 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Added test (1.78 KB, patch)
2019-04-25 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL (854 bytes, patch)
2019-04-25 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl (4.07 KB, patch)
2019-04-25 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins (22.54 KB, patch)
2019-04-25 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add one more test fot get_openurl (1.40 KB, patch)
2019-04-25 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (follow-up) Take system preference item-level_itypes into account (1.78 KB, patch)
2019-04-25 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: (QA follow-up) Convert db update to perl (2.58 KB, patch)
2019-04-25 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8995: Show OpenURL links in OPAC search results (29.13 KB, patch)
2019-04-29 14:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: Add IGNORE to the INSERT statement (1.21 KB, patch)
2019-04-29 14:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (follow-up) Add tests, move open_url/coins routines to Koha namespace (23.41 KB, patch)
2019-04-29 14:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (QA follow-up) Convert db update to perl (2.59 KB, patch)
2019-04-29 14:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8995: (QA follow-up) Mock required sysprefs in the tests (1.64 KB, patch)
2019-04-30 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2012-10-31 13:48:48 UTC

    
Comment 1 Julian Maurice 2012-10-31 13:49:48 UTC Comment hidden (obsolete)
Comment 2 claire.hernandez@biblibre.com 2012-11-07 13:31:46 UTC
We are currently working on this patch, please do not consider this submission for the moment
Comment 3 Julian Maurice 2013-11-06 15:05:23 UTC Comment hidden (obsolete)
Comment 4 mathieu saby 2013-11-17 14:47:47 UTC
Sponsored by Rennes 2 university.
This patch also clean the date taken from 210 field, because openurl standard does not accept dates like "impr. 2010" or "cop 2009". It needs only 2010 or 2009.

I'm testing it.

M. Saby
Comment 5 mathieu saby 2013-11-17 15:07:26 UTC
Sorry, Software error:

Can't escape \x{0152}, try uri_escape_utf8() instead at /home/msaby/kohamaster/src/C4/Biblio.pm line 1452.

even with new syspref disabled. I suppose it is linked to the refactoring of COins.


2 questions : 
- now that we have a bootstrap opac defined as new default opac, does this patch needs a followup to work with bootstrap theme as well as with prog theme?

- as the changes made on building COins could potentially have effects even if the new syspref is not activated (cf my software error...), I think we need a little regression test plan, to be sure Coins are still build correctly for unimarc and marc21 (I've seen things about 773 field, but I don't know what is it for). I'm not sure of the best way to do that : I suppose some FF extensions like OpenURLReferrer could be usefull for that, but I've not tested it recently...

Mathieu
Comment 6 Julian Maurice 2013-12-18 11:01:44 UTC Comment hidden (obsolete)
Comment 7 Julian Maurice 2013-12-18 11:01:53 UTC Comment hidden (obsolete)
Comment 8 Julian Maurice 2013-12-18 11:02:03 UTC Comment hidden (obsolete)
Comment 9 Julian Maurice 2013-12-18 11:03:45 UTC
1st patch rebased on master
2nd patch fix the "Can't escape \x{0152}" error
3rd patch update bootstrap theme to display OpenURL links

All patches needs a signoff
Comment 10 Jonathan Druart 2014-01-09 14:08:18 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2014-01-09 14:08:26 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-01-09 14:08:33 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2014-01-09 14:08:48 UTC
rebased patches.
Comment 14 Holger Meißner 2014-03-31 08:34:24 UTC
Applying: Bug 8995: Show OpenURL links in OPAC search results
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-search.pl
Auto-merging opac/opac-detail.pl
Auto-merging koha-tmpl/opac-tmpl/prog/en/xslt/UNIMARCslim2OPACResults.xsl
Auto-merging koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACResults.xsl
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/css/opac.css
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Biblio.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 8995: Show OpenURL links in OPAC search results
Comment 15 Jonathan Druart 2014-04-08 14:12:14 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2014-04-08 14:12:20 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2014-04-08 14:12:26 UTC Comment hidden (obsolete)
Comment 18 David Cook 2014-05-20 05:20:32 UTC
I like this idea, but I imagine it might make more sense to have a system preference called "OPACOpenURLForItemTypes" rather than "OPACHideOpenURLForItemTypes". 

That is, I imagine there are probably only going to be maybe one or two item types (i.e. eJournals and maybe eBooks) where you want to provide a OpenURL. So rather than listing all the ones you don't, it would be easier and less ongoing maintenance, to just list the ones that you do want to include.

--

It might also be an idea to change most of references to COinS to OpenURL in the code, since it really is OpenURL data being generated. The only time there really are COinS is when you stick the OpenURL into an HTML span, right?
Comment 19 Koha Team Lyon 3 2014-06-18 14:29:55 UTC
Some problems occurred applying patches from bug 8995:
<h1>Something went wrong !</h1>Applying: Bug 8995: Show OpenURL links in OPAC search results Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging C4/Biblio.pm
Auto-merging C4/XSLT.pm
CONFLICT (content): Merge conflict in C4/XSLT.pm Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging koha-tmpl/opac-tmpl/prog/en/css/opac.css
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl
Auto-merging koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACResults.xsl
Auto-merging opac/opac-detail.pl
CONFLICT (content): Merge conflict in opac/opac-detail.pl Auto-merging opac/opac-search.pl Failed to merge in the changes.
Patch failed at 0001 Bug 8995: Show OpenURL links in OPAC search results When you have
Comment 20 Koha Team Lyon 3 2014-06-27 14:11:29 UTC
I'm really interested in this functionality. Are you still working on it ?
Comment 21 Jonathan Druart 2014-06-30 10:16:08 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2014-06-30 10:16:14 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2014-06-30 10:16:21 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2014-06-30 10:16:50 UTC
I just fixed the conflicts.
Comment 25 Koha Team Lyon 3 2014-07-04 13:55:05 UTC
I'm disappointed, because I made a lot of tests and everything is OK except I can't see the openurl link when I Set sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay to "default".
It works very well when this sysprefs are empty (tested with two different open resolvers with electronic journals and ebooks.)

Two more remarks :
I agree with  David Cook (Comment 18)  : we usually use open url for 2 item types, it would be easier to list only those two and not all the others.

It is possible to open the link in a new tab (I activate the syspref OPACOpenInNewWindow, but there is no difference)
Comment 26 Koha Team Lyon 3 2014-07-04 13:56:49 UTC
One detail : I have tested only with the bootstrap theme
Comment 27 Julian Maurice 2014-07-31 09:31:28 UTC Comment hidden (obsolete)
Comment 28 Julian Maurice 2014-07-31 09:31:39 UTC Comment hidden (obsolete)
Comment 29 Julian Maurice 2014-07-31 09:31:49 UTC Comment hidden (obsolete)
Comment 30 Julian Maurice 2014-07-31 09:31:55 UTC Comment hidden (obsolete)
Comment 31 Julian Maurice 2014-07-31 09:32:01 UTC Comment hidden (obsolete)
Comment 32 Julian Maurice 2014-07-31 09:43:01 UTC
(In reply to Koha Team Lyon 3 from comment #25)
> I'm disappointed, because I made a lot of tests and everything is OK except
> I can't see the openurl link when I Set sysprefs OPACXSLTDetailsDisplay and
> OPACXSLTResultsDisplay to "default".
> It works very well when this sysprefs are empty (tested with two different
> open resolvers with electronic journals and ebooks.)

Fixed by attachment 30388 [details] [review]

> Two more remarks :
> I agree with  David Cook (Comment 18)  : we usually use open url for 2 item
> types, it would be easier to list only those two and not all the others.

Fixed by attachment 30389 [details] [review]
Comment 33 Julian Maurice 2014-07-31 09:43:56 UTC
> It is possible to open the link in a new tab (I activate the syspref
> OPACOpenInNewWindow, but there is no difference)

I wasn't aware of OpacOpenInNewWindow syspref, I will take a look to see what I can do.
Comment 34 Julian Maurice 2014-07-31 10:07:58 UTC Comment hidden (obsolete)
Comment 35 Paola Rossi 2014-11-17 16:57:44 UTC
I've tried to apply the patches aginst master 3.17.00.055

Applying: Bug 8995: Show OpenURL links in OPAC search results
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-search.pl
CONFLICT (modify/delete): koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt deleted in HEAD and modified in Bug 8995: Show OpenURL links in OPAC search results. Version Bug 8995: Show OpenURL links in OPAC search results of koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt left in tree.
CONFLICT (modify/delete): koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt deleted in HEAD and modified in Bug 8995: Show OpenURL links in OPAC search results. Version Bug 8995: Show OpenURL links in OPAC search results of koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt left in tree.
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/UNIMARCslim2OPACResults.xsl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/UNIMARCslim2OPACDetail.xsl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/css/opac-old.css
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/XSLT.pm
CONFLICT (content): Merge conflict in C4/XSLT.pm
Auto-merging C4/Biblio.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 8995: Show OpenURL links in OPAC search results

So I pass the patch to "Patch doesn't apply" status.
Comment 36 Julian Maurice 2015-02-09 12:22:59 UTC Comment hidden (obsolete)
Comment 37 Julian Maurice 2015-02-09 12:23:26 UTC
All patches rebased and squashed
Comment 38 Paola Rossi 2015-02-13 16:39:41 UTC Comment hidden (obsolete)
Comment 39 Katrin Fischer 2015-03-16 06:52:28 UTC
This patch conflicts in a since removed CSS file:
CONFLICT (modify/delete): koha-tmpl/opac-tmpl/bootstrap/css/opac-old.css

Looking at the patch I can see that the .less file for bootstrap has been modified, but there is no updated generated CSS file, but only the change to the outdated file. Can you check please?
Comment 40 Julian Maurice 2015-06-26 07:41:49 UTC Comment hidden (obsolete)
Comment 41 Katrin Fischer 2015-10-18 22:05:11 UTC
There are some conflicts - especially in the CSS files. Please fix!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 8995: Show OpenURL links in OPAC search results
Using index info to reconstruct a base tree...
M	C4/Biblio.pm
M	installer/data/mysql/sysprefs.sql
M	installer/data/mysql/updatedatabase.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
M	koha-tmpl/opac-tmpl/bootstrap/css/opac.css
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl
M	koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl
M	koha-tmpl/opac-tmpl/bootstrap/less/opac.less
M	opac/opac-detail.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-detail.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/less/opac.less
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/less/opac.less
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/css/opac.css
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/css/opac.css
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Biblio.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 8995: Show OpenURL links in OPAC search results
The copy of the patch that failed is found in:
   /home/katrin/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/SIGNED-OFF-Bug-8995-Show-OpenURL-links-in-OPAC-sea-AHeoX5.patch
Comment 42 Julian Maurice 2015-11-09 10:59:23 UTC Comment hidden (obsolete)
Comment 43 Katrin Fischer 2015-11-30 23:10:30 UTC
Comment on attachment 44639 [details] [review]
[SIGNED OFF] Bug 8995: Show OpenURL links in OPAC search results

Review of attachment 44639 [details] [review]:
-----------------------------------------------------------------

Hi, 

I have started with a code review here:

1) Please remove the changes to opac-detail.tt and opac-results.tt. (see: http://wiki.koha-community.org/wiki/Coding_Guidelines#DEPR1:_non-XSLT_search_results_and_bib_display_templates_deprecated)

::: C4/XSLT.pm
@@ +215,5 @@
>          $sysxml .= "<syspref name=\"$syspref\">$sp</syspref>\n";
>      }
>      $sysxml .= "</sysprefs>\n";
> +
> +    $variables ||= {};

2) Please chose a better variable name reflecting that this is about the OpenUrl feature.

::: installer/data/mysql/sysprefs.sql
@@ +510,1 @@
>  ;

2.5) Please do the librarian in me a favor... and keep the alphabetic order in this file :)

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
@@ +324,4 @@
>                    both: "both top and footer"
>                    top: "top"
>                    footer: "only footer"
> +        -

3) Please create a new heading for the OpenURL preferences, so that they are listed in one section together.

@@ +324,5 @@
>                    both: "both top and footer"
>                    top: "top"
>                    footer: "only footer"
> +        -
> +            - 'Complete URL of OpenURL resolver (starting with <tt>http://</tt> or <tt>https://</tt>):'

4) Please remove the <tt> tags (see: https://www.w3.org/wiki/HTML/Elements/tt). I think we are using <code> in some other preferences.

@@ +349,5 @@
> +        -
> +            - 'List of item type codes (separated by spaces) for those you want to show the OpenURL link:'
> +            - pref: OPACOpenURLItemTypes
> +            - '<br />'
> +            - 'It uses biblioitems.itemtype field, so if you map a MARC field to it, and link this MARC field to a list of authorised values (for example CCODE), you can use these values for system preference value.'

5) Sorry, but I think this is bad advice. One shouldn't redefine the biblioitems.itemtype field like that - or you will cause yourself trouble in a lot of other places. For example some of the circulation code falls back to the itemtype defined on biblio level now.

What happens when pref is left empty? Can the OpenUrl be displayed for records with empty biblioitems.itemtype?
Comment 44 Marcel de Rooy 2016-01-24 16:13:05 UTC
Are there still plans to get this moving again?

Additional question: How is this related to the COinS html Koha already generates? Why do we exactly need this, if that span can be easily converted to an OpenURL with standard solutions (browser extensions etc.) ?

http://ocoins.info/latent.html
Comment 45 Julian Maurice 2016-02-25 15:18:30 UTC Comment hidden (obsolete)
Comment 46 Julian Maurice 2016-02-25 15:18:35 UTC Comment hidden (obsolete)
Comment 47 Julian Maurice 2016-02-25 15:30:03 UTC
(In reply to Katrin Fischer from comment #43)
> 1) Please remove the changes to opac-detail.tt and opac-results.tt. (see:
> http://wiki.koha-community.org/wiki/Coding_Guidelines#DEPR1:_non-
> XSLT_search_results_and_bib_display_templates_deprecated)

Changes removed

> 2) Please chose a better variable name reflecting that this is about the
> OpenUrl feature.

I am not sure of what you ask here, "COinS" is related to OpenURL, this name makes perfect sense to me.

> 2.5) Please do the librarian in me a favor... and keep the alphabetic order
> in this file :)

Done ;)

> 3) Please create a new heading for the OpenURL preferences, so that they are
> listed in one section together.

Done
 
> 4) Please remove the <tt> tags (see:
> https://www.w3.org/wiki/HTML/Elements/tt). I think we are using <code> in
> some other preferences.

Done

> 5) Sorry, but I think this is bad advice. One shouldn't redefine the
> biblioitems.itemtype field like that - or you will cause yourself trouble in
> a lot of other places. For example some of the circulation code falls back
> to the itemtype defined on biblio level now.

Advice removed

> What happens when pref is left empty? Can the OpenUrl be displayed for
> records with empty biblioitems.itemtype?

If the preference is empty, the feature is disabled. It can be changed if needed.


I took the opportunity to put opac.css changes in a separate patch as recommended by guidelines.
Comment 48 Katrin Fischer 2016-03-23 06:52:01 UTC
Hi Julian, coming back to this soon - could you point me to an example config mayb to see this in action?
Comment 49 Katrin Fischer 2016-03-28 19:36:39 UTC
Some notes:

1) a & in the URL breaks the detail page. How can we handle this? I tried testing the feature with a service that requires you to pass on a library ID and some other parameters in order to use the service.

2) We already have a system preference mentioning OpenURL: [Don't include|Include] COinSinOPACResults COinS / OpenURL / Z39.88 in OPAC search results. Warning: Enabling this feature will slow OPAC search response times. It has no effect on the OpenURL feature added by this patch. I'd suggest rephrasing it, but not sure about a suggestion. Maybe just an addition that the OpenURL feature is separate?

3) Further tests with the service I tried using showed that a lot of the attributes were invalid (after researching a bit I think we might provide 1.0 valid, while the service is a lower OpenURL version) - so I can't really use that for testing :(
I'd still much appreciate a 'real life example' to test this with.

4) Please take a look at comment#44

... still needs testing with UNIMARC.
Comment 50 Julian Maurice 2016-03-29 07:32:56 UTC
1) In the XSLT, a '?' is appended to the syspref, followed by other params, so it's a bug. Changing status to Failed QA

2) Telling about the other sysprefs here would be nice I think

3) I don't know any free service that could be used for testing. I'll try to find a way to make this testable

4) An OpenURL resolver can provide additional resources, and we can't expect all OPAC users to have installed a browser extension such as Zotero
Comment 51 Magnus Enger 2016-04-19 08:22:45 UTC
(In reply to Julian Maurice from comment #50)
> 1) In the XSLT, a '?' is appended to the syspref, followed by other params,
> so it's a bug. Changing status to Failed QA
> 
> 2) Telling about the other sysprefs here would be nice I think

I would love to see this in 16.05. Any chance these two issues can be adressed in time?
Comment 52 Julian Maurice 2016-09-02 14:01:27 UTC Comment hidden (obsolete)
Comment 53 Julian Maurice 2016-09-02 14:01:33 UTC Comment hidden (obsolete)
Comment 54 Julian Maurice 2016-09-02 14:01:38 UTC Comment hidden (obsolete)
Comment 55 Julian Maurice 2016-09-02 14:01:42 UTC Comment hidden (obsolete)
Comment 56 Julian Maurice 2016-09-02 14:01:47 UTC Comment hidden (obsolete)
Comment 57 Julian Maurice 2016-09-02 14:01:51 UTC Comment hidden (obsolete)
Comment 58 Julian Maurice 2016-09-02 14:03:35 UTC
First 2 patches rebased on master
4 new patches added to fix some issues
Comment 59 Jonathan Druart 2016-09-28 12:17:40 UTC
How can we test this feature correctly?
Comment 60 Jonathan Druart 2016-09-28 12:20:34 UTC Comment hidden (obsolete)
Comment 61 Magnus Enger 2016-09-29 19:39:43 UTC
(In reply to Jonathan Druart from comment #59)
> How can we test this feature correctly?

The trouble is finding a link resolver that can be used. One of my customers is very interested in this patch, so I have set up a gitified, temporary instance for them with the patches on this bug. They have configured it to use their existing SFX lin resolver. Here is an example:
http://stami.demo.bibkat.no/cgi-bin/koha/opac-detail.pl?biblionumber=9810

They have tested it and found it satisfactory. Their testing was before the 5 followup patches, but the instance is currently running with all the patches on this bug.
Comment 62 M. Tompsett 2017-03-03 19:50:51 UTC
If one fixes the opac.less (easy to fix), and recompiles the opac.css (easy to do in kohadevbox -- mine isn't booted), this patch set could be rescued.
Comment 63 Magnus Enger 2017-06-21 09:11:06 UTC
Any chance of another rebase? I'll sign off quickley this time.
Comment 64 Magnus Enger 2017-06-21 12:36:09 UTC
If anyone needs an OpenURL linkresolver to test these patches against, I have a customer that has one that can be used. Please contact me for details.
Comment 65 Julian Maurice 2017-06-22 07:55:41 UTC Comment hidden (obsolete)
Comment 66 Julian Maurice 2017-06-22 07:55:48 UTC Comment hidden (obsolete)
Comment 67 Julian Maurice 2017-06-22 07:55:53 UTC Comment hidden (obsolete)
Comment 68 Julian Maurice 2017-06-22 07:55:59 UTC Comment hidden (obsolete)
Comment 69 Julian Maurice 2017-06-22 07:56:04 UTC Comment hidden (obsolete)
Comment 70 Julian Maurice 2017-06-22 07:56:10 UTC Comment hidden (obsolete)
Comment 71 Julian Maurice 2017-06-22 07:56:15 UTC Comment hidden (obsolete)
Comment 72 Julian Maurice 2017-06-22 07:56:42 UTC
Patches rebased
Comment 73 Magnus Enger 2017-06-22 09:01:12 UTC Comment hidden (obsolete)
Comment 74 Magnus Enger 2017-06-22 09:01:23 UTC Comment hidden (obsolete)
Comment 75 Magnus Enger 2017-06-22 09:01:31 UTC Comment hidden (obsolete)
Comment 76 Magnus Enger 2017-06-22 09:01:56 UTC Comment hidden (obsolete)
Comment 77 Magnus Enger 2017-06-22 09:02:05 UTC Comment hidden (obsolete)
Comment 78 Magnus Enger 2017-06-22 09:02:26 UTC Comment hidden (obsolete)
Comment 79 Magnus Enger 2017-06-22 09:02:35 UTC Comment hidden (obsolete)
Comment 80 Magnus Enger 2017-06-22 09:03:39 UTC
One of my customers have tested this, and are very happy with the result. This should be a very nice addition for many academic libraries.
Comment 81 Katrin Fischer 2017-06-22 19:34:16 UTC
Hi Magnuse, as you have a good test case for it - could I get you to write something for the manual once this goes in?
Comment 82 Magnus Enger 2017-06-23 06:20:23 UTC
(In reply to Katrin Fischer from comment #81)
> Hi Magnuse, as you have a good test case for it - could I get you to write
> something for the manual once this goes in?

Sure! Can you remind me about it?
Comment 83 Katrin Fischer 2017-06-23 06:21:48 UTC
I will! ;)
Comment 84 Josef Moravec 2018-02-20 14:09:27 UTC
Patch doesn't apply, please, rebase.

Also, we will need pod and test for GetOpenURLResolverURL in C4::Biblio
Comment 85 Josef Moravec 2019-03-11 21:30:03 UTC Comment hidden (obsolete)
Comment 86 Josef Moravec 2019-03-11 21:30:11 UTC Comment hidden (obsolete)
Comment 87 Josef Moravec 2019-03-11 21:30:17 UTC Comment hidden (obsolete)
Comment 88 Josef Moravec 2019-03-11 21:30:24 UTC Comment hidden (obsolete)
Comment 89 Josef Moravec 2019-03-11 21:30:32 UTC Comment hidden (obsolete)
Comment 90 Josef Moravec 2019-03-11 21:30:38 UTC Comment hidden (obsolete)
Comment 91 Josef Moravec 2019-03-11 21:30:46 UTC Comment hidden (obsolete)
Comment 92 Josef Moravec 2019-03-11 21:41:41 UTC Comment hidden (obsolete)
Comment 93 Josef Moravec 2019-03-11 21:41:50 UTC Comment hidden (obsolete)
Comment 94 Josef Moravec 2019-03-11 21:41:56 UTC Comment hidden (obsolete)
Comment 95 Josef Moravec 2019-03-11 21:46:02 UTC
I rebased and squashed the patches a bit. 
To pass QA there are at least missing tests for GetOpenURLResolverURL.
Comment 96 Josef Moravec 2019-03-12 13:50:29 UTC Comment hidden (obsolete)
Comment 97 Josef Moravec 2019-03-12 13:50:34 UTC Comment hidden (obsolete)
Comment 98 Josef Moravec 2019-03-13 08:13:09 UTC Comment hidden (obsolete)
Comment 99 Josef Moravec 2019-03-13 08:13:14 UTC Comment hidden (obsolete)
Comment 100 Josef Moravec 2019-03-13 08:31:22 UTC Comment hidden (obsolete)
Comment 101 Josef Moravec 2019-03-14 13:24:53 UTC Comment hidden (obsolete)
Comment 102 Magnus Enger 2019-03-14 13:37:04 UTC
Created attachment 86602 [details] [review]
Bug 8995: Show OpenURL links in OPAC search results

This patch use GetCOinS sub to provide an OpenURL link in OPAC search
results. It uses 4 new system preferences:
  - OpenURLinOPACResults: enable or disable this feature
  - OpenURLResolverURL: url of the openURL resolver
  - OpenURLText: text of the link
  - OpenURLImageLocation: image of the link

Link is displayed as an image if OpenURLImageLocation is defined, and as
text otherwise.
It works both with and without XSLT enabled.

Changes made to GetCOinSBiblio:

For 'journal':
- Title should be in rft.jtitle instead of rft.title
- rft.date, rft.aulast, rft.aufirst, rft.au, rft.pub and rft.pages have
  no meaning for a subscription, so they are simply removed from URL

This patch refactors GetCOinSBiblio, so the construction of URL is done
only at the end. This way we do not have ugly
  $var .= "&amp;$value"
in the function body.

Also use URI::Escape instead of custom regexps.

This development consider the value of syspref OPACURLOpenInNewWindow
when building the link.

Test plan:
1/ Enable syspref OPACShowOpenURL and put your OpenURL resolver URL in
OpenURLResolverURL syspref (if you don't have one, just fill it with
some fake URL, you'll have to check if OpenURL links are correct)
2/ If you want, set the other sysprefs OpenURLImageLocation and
OpenURLText
3/ Fill syspref OPACOpenURLItemTypes with some (not all) of your
item types.
4/ Empty sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay
5/ Go to OPAC and launch a search.
6/ Check you have in the results (near the title) the OpenURL link (only
for itemtypes that are in OPACOpenURLItemTypes).
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
7/ Go to the detail page of one of those and check you have the OpenURL
link too. (Above tags)
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
8/ Set sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay to
"default" and repeat steps 5 to 7

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 103 Magnus Enger 2019-03-14 13:37:10 UTC
Created attachment 86603 [details] [review]
Bug 8995: Add IGNORE to the INSERT statement

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 104 Magnus Enger 2019-03-14 13:37:16 UTC
Created attachment 86604 [details] [review]
Bug 8995: (follow-up) Adapt to current codebase

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 105 Magnus Enger 2019-03-14 13:37:23 UTC
Created attachment 86605 [details] [review]
Bug 8995: (follow-up) Added test

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 106 Magnus Enger 2019-03-14 13:37:29 UTC
Created attachment 86606 [details] [review]
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 107 Magnus Enger 2019-03-14 13:37:35 UTC
Created attachment 86607 [details] [review]
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 108 Magnus Enger 2019-03-14 13:37:41 UTC
Created attachment 86608 [details] [review]
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins

Test plan:
1) Ensure the COinS span tag is still included on this pages. You need
to look into html source and search for span tag with class 'Z3988',
   which has COinS string in title.
   Staff client:
       catalogue -> ISBDdetail
       catalogue -> MARCdetail
       catalogue -> detail
       virtualshelves -> shelves
    OPAC (you should have COinSinOPACResults system preference enabled):
        opac detail
        opac search
        opac shelves
2) Run tests:
prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 109 Magnus Enger 2019-03-14 13:37:47 UTC
Created attachment 86609 [details] [review]
Bug 8995: (follow-up) Add one more test fot get_openurl

Test plan:
prove t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 110 Magnus Enger 2019-03-14 13:37:54 UTC
Created attachment 86610 [details] [review]
Bug 8995: (follow-up) Tak system preference item-level_itypes into account

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested with all 9 current patches. Works as advertised, including
OPACURLOpenInNewWindow. If a record has no items, no OpenURL link
is displayed. All the suggested tests pass. I did not test with
XSLT turned off.
Comment 111 Josef Moravec 2019-04-24 22:11:02 UTC
Created attachment 88665 [details] [review]
Bug 8995: Show OpenURL links in OPAC search results

This patch use GetCOinS sub to provide an OpenURL link in OPAC search
results. It uses 4 new system preferences:
  - OpenURLinOPACResults: enable or disable this feature
  - OpenURLResolverURL: url of the openURL resolver
  - OpenURLText: text of the link
  - OpenURLImageLocation: image of the link

Link is displayed as an image if OpenURLImageLocation is defined, and as
text otherwise.
It works both with and without XSLT enabled.

Changes made to GetCOinSBiblio:

For 'journal':
- Title should be in rft.jtitle instead of rft.title
- rft.date, rft.aulast, rft.aufirst, rft.au, rft.pub and rft.pages have
  no meaning for a subscription, so they are simply removed from URL

This patch refactors GetCOinSBiblio, so the construction of URL is done
only at the end. This way we do not have ugly
  $var .= "&amp;$value"
in the function body.

Also use URI::Escape instead of custom regexps.

This development consider the value of syspref OPACURLOpenInNewWindow
when building the link.

Test plan:
1/ Enable syspref OPACShowOpenURL and put your OpenURL resolver URL in
OpenURLResolverURL syspref (if you don't have one, just fill it with
some fake URL, you'll have to check if OpenURL links are correct)
2/ If you want, set the other sysprefs OpenURLImageLocation and
OpenURLText
3/ Fill syspref OPACOpenURLItemTypes with some (not all) of your
item types.
4/ Empty sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay
5/ Go to OPAC and launch a search.
6/ Check you have in the results (near the title) the OpenURL link (only
for itemtypes that are in OPACOpenURLItemTypes).
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
7/ Go to the detail page of one of those and check you have the OpenURL
link too. (Above tags)
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
8/ Set sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay to
"default" and repeat steps 5 to 7

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 112 Josef Moravec 2019-04-24 22:11:07 UTC
Created attachment 88666 [details] [review]
Bug 8995: Add IGNORE to the INSERT statement

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 113 Josef Moravec 2019-04-24 22:11:12 UTC
Created attachment 88667 [details] [review]
Bug 8995: (follow-up) Adapt to current codebase

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 114 Josef Moravec 2019-04-24 22:11:17 UTC
Created attachment 88668 [details] [review]
Bug 8995: (follow-up) Added test

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 115 Josef Moravec 2019-04-24 22:11:22 UTC
Created attachment 88669 [details] [review]
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 116 Josef Moravec 2019-04-24 22:11:27 UTC
Created attachment 88670 [details] [review]
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 117 Josef Moravec 2019-04-24 22:11:33 UTC
Created attachment 88671 [details] [review]
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins

Signed-off-by: Magnus Enger <magnus@libriotech.no>

Test plan:
1) Ensure the COinS span tag is still included on this pages. You need
to look into html source and search for span tag with class 'Z3988',
   which has COinS string in title.
   Staff client:
       catalogue -> ISBDdetail
       catalogue -> MARCdetail
       catalogue -> detail
       virtualshelves -> shelves
    OPAC (you should have COinSinOPACResults system preference enabled):
        opac detail
        opac search
        opac shelves
2) Run tests:
prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t
Comment 118 Josef Moravec 2019-04-24 22:11:38 UTC
Created attachment 88672 [details] [review]
Bug 8995: (follow-up) Add one more test fot get_openurl

Test plan:
prove t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 119 Josef Moravec 2019-04-24 22:11:43 UTC
Created attachment 88673 [details] [review]
Bug 8995: (follow-up) Tak system preference item-level_itypes into account

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested with all 9 current patches. Works as advertised, including
OPACURLOpenInNewWindow. If a record has no items, no OpenURL link
is displayed. All the suggested tests pass. I did not test with
XSLT turned off.
Comment 120 Josef Moravec 2019-04-24 22:12:04 UTC
Just rebased on master
Comment 121 Martin Renvoize 2019-04-25 09:45:44 UTC
Created attachment 88735 [details] [review]
Bug 8995: Show OpenURL links in OPAC search results

This patch use GetCOinS sub to provide an OpenURL link in OPAC search
results. It uses 4 new system preferences:
  - OpenURLinOPACResults: enable or disable this feature
  - OpenURLResolverURL: url of the openURL resolver
  - OpenURLText: text of the link
  - OpenURLImageLocation: image of the link

Link is displayed as an image if OpenURLImageLocation is defined, and as
text otherwise.
It works both with and without XSLT enabled.

Changes made to GetCOinSBiblio:

For 'journal':
- Title should be in rft.jtitle instead of rft.title
- rft.date, rft.aulast, rft.aufirst, rft.au, rft.pub and rft.pages have
  no meaning for a subscription, so they are simply removed from URL

This patch refactors GetCOinSBiblio, so the construction of URL is done
only at the end. This way we do not have ugly
  $var .= "&amp;$value"
in the function body.

Also use URI::Escape instead of custom regexps.

This development consider the value of syspref OPACURLOpenInNewWindow
when building the link.

Test plan:
1/ Enable syspref OPACShowOpenURL and put your OpenURL resolver URL in
OpenURLResolverURL syspref (if you don't have one, just fill it with
some fake URL, you'll have to check if OpenURL links are correct)
2/ If you want, set the other sysprefs OpenURLImageLocation and
OpenURLText
3/ Fill syspref OPACOpenURLItemTypes with some (not all) of your
item types.
4/ Empty sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay
5/ Go to OPAC and launch a search.
6/ Check you have in the results (near the title) the OpenURL link (only
for itemtypes that are in OPACOpenURLItemTypes).
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
7/ Go to the detail page of one of those and check you have the OpenURL
link too. (Above tags)
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
8/ Set sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay to
"default" and repeat steps 5 to 7

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 122 Martin Renvoize 2019-04-25 09:45:49 UTC
Created attachment 88736 [details] [review]
Bug 8995: Add IGNORE to the INSERT statement

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 123 Martin Renvoize 2019-04-25 09:45:54 UTC
Created attachment 88737 [details] [review]
Bug 8995: (follow-up) Adapt to current codebase

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 124 Martin Renvoize 2019-04-25 09:45:58 UTC
Created attachment 88738 [details] [review]
Bug 8995: (follow-up) Added test

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 125 Martin Renvoize 2019-04-25 09:46:04 UTC
Created attachment 88739 [details] [review]
Bug 8995: (follow-up) Add POD for GetOpenURLResolverURL

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 126 Martin Renvoize 2019-04-25 09:46:08 UTC
Created attachment 88740 [details] [review]
Bug 8995: (follow-up) Move GetOpenURLResolverURL to Koha::Biblio->get_openurl

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 127 Martin Renvoize 2019-04-25 09:46:14 UTC
Created attachment 88741 [details] [review]
Bug 8995: (follow-up) Move GetCOinSBiblio to Koha::Biblio->get_coins

Signed-off-by: Magnus Enger <magnus@libriotech.no>

Test plan:
1) Ensure the COinS span tag is still included on this pages. You need
to look into html source and search for span tag with class 'Z3988',
   which has COinS string in title.
   Staff client:
       catalogue -> ISBDdetail
       catalogue -> MARCdetail
       catalogue -> detail
       virtualshelves -> shelves
    OPAC (you should have COinSinOPACResults system preference enabled):
        opac detail
        opac search
        opac shelves
2) Run tests:
prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 128 Martin Renvoize 2019-04-25 09:46:18 UTC
Created attachment 88742 [details] [review]
Bug 8995: (follow-up) Add one more test fot get_openurl

Test plan:
prove t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 129 Martin Renvoize 2019-04-25 09:46:23 UTC
Created attachment 88743 [details] [review]
Bug 8995: (follow-up) Take system preference item-level_itypes into account

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested with all 9 current patches. Works as advertised, including
OPACURLOpenInNewWindow. If a record has no items, no OpenURL link
is displayed. All the suggested tests pass. I did not test with
XSLT turned off.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 130 Martin Renvoize 2019-04-25 09:46:28 UTC
Created attachment 88744 [details] [review]
Bug 8995: (QA follow-up) Convert db update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 131 Martin Renvoize 2019-04-25 09:48:27 UTC
Good work team, well shepherded to completion.

This works well for me and I'm unable to find any regressions.. QA scripts all pass (with the exception of the git manipulation issue where I converted the .sql file to a .perl one as per the current RM's preference). 

Passing QA.. any chance of something for the release notes?
Comment 132 Nick Clemens 2019-04-26 12:34:29 UTC
I like this one a lot - can we get a squashed/clean version of these patches just for readability sake?
Comment 133 Josef Moravec 2019-04-29 14:31:23 UTC
Created attachment 89012 [details] [review]
Bug 8995: Show OpenURL links in OPAC search results

This patch use GetCOinS sub to provide an OpenURL link in OPAC search
results. It uses 4 new system preferences:
  - OpenURLinOPACResults: enable or disable this feature
  - OpenURLResolverURL: url of the openURL resolver
  - OpenURLText: text of the link
  - OpenURLImageLocation: image of the link

Link is displayed as an image if OpenURLImageLocation is defined, and as
text otherwise.
It works both with and without XSLT enabled.

Changes made to GetCOinSBiblio:

For 'journal':
- Title should be in rft.jtitle instead of rft.title
- rft.date, rft.aulast, rft.aufirst, rft.au, rft.pub and rft.pages have
  no meaning for a subscription, so they are simply removed from URL

This patch refactors GetCOinSBiblio, so the construction of URL is done
only at the end. This way we do not have ugly
  $var .= "&amp;$value"
in the function body.

Also use URI::Escape instead of custom regexps.

This development consider the value of syspref OPACURLOpenInNewWindow
when building the link.

Test plan:
1/ Enable syspref OPACShowOpenURL and put your OpenURL resolver URL in
OpenURLResolverURL syspref (if you don't have one, just fill it with
some fake URL, you'll have to check if OpenURL links are correct)
2/ If you want, set the other sysprefs OpenURLImageLocation and
OpenURLText
3/ Fill syspref OPACOpenURLItemTypes with some (not all) of your
item types.
4/ Empty sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay
5/ Go to OPAC and launch a search.
6/ Check you have in the results (near the title) the OpenURL link (only
for itemtypes that are in OPACOpenURLItemTypes).
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
7/ Go to the detail page of one of those and check you have the OpenURL
link too. (Above tags)
Toggle OPACURLOpenInNewWindow syspref and check that the behaviour of
the link is correct.
8/ Set sysprefs OPACXSLTDetailsDisplay and OPACXSLTResultsDisplay to
"default" and repeat steps 5 to 7

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 134 Josef Moravec 2019-04-29 14:31:29 UTC
Created attachment 89013 [details] [review]
Bug 8995: Add IGNORE to the INSERT statement

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 135 Josef Moravec 2019-04-29 14:31:34 UTC
Created attachment 89014 [details] [review]
Bug 8995: (follow-up) Add tests, move open_url/coins routines to Koha namespace

Test plan:
1) Ensure the COinS span tag is still included on this pages. You need
to look into html source and search for span tag with class 'Z3988',
   which has COinS string in title.
   Staff client:
       catalogue -> ISBDdetail
       catalogue -> MARCdetail
       catalogue -> detail
       virtualshelves -> shelves
    OPAC (you should have COinSinOPACResults system preference enabled):
        opac detail
        opac search
        opac shelves
2) Run tests:
prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested with all 9 current patches. Works as advertised, including
OPACURLOpenInNewWindow. If a record has no items, no OpenURL link
is displayed. All the suggested tests pass. I did not test with
XSLT turned off.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 136 Josef Moravec 2019-04-29 14:31:39 UTC
Created attachment 89015 [details] [review]
Bug 8995: (QA follow-up) Convert db update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 137 Josef Moravec 2019-04-29 14:32:32 UTC
(In reply to Nick Clemens from comment #132)
> I like this one a lot - can we get a squashed/clean version of these patches
> just for readability sake?

Is this rebase OK for you Nick?
Comment 138 Nick Clemens 2019-04-29 16:22:26 UTC
(In reply to Josef Moravec from comment #137)
> (In reply to Nick Clemens from comment #132)
> > I like this one a lot - can we get a squashed/clean version of these patches
> > just for readability sake?
> 
> Is this rebase OK for you Nick?

Indeed, thanks Josef!.  I was worried that changes don't work without XSLT, but if I understand earlier comments they were remove as that view is deprecated.
Comment 139 Nick Clemens 2019-04-29 16:23:09 UTC
Awesome work all!

Pushed to master for 19.05
Comment 140 Katrin Fischer 2019-04-29 16:42:21 UTC
Maybe we should update to new feature?
Comment 141 Tomás Cohen Arazi 2019-04-30 14:38:44 UTC
Created attachment 89120 [details] [review]
Bug 8995: (QA follow-up) Mock required sysprefs in the tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 142 Martin Renvoize 2019-05-08 14:08:16 UTC
New feature will not be backported to 18.11.x series.