Created attachment 23365 [details] [review] Add a Z39.50 search page to the OPAC to let users search for records on a remote Koha instance. This patch adds the OPACZ3950 preference which activates a "Z39.50 Search" page in the OPAC. This page can be used to let members search for biblio records on remote Koha instances. If the "Show" link is clicked in the results page, the user is then redirected to the remote Koha's OPAC to see the record's details. This feature can be useful if you have multiple libraries with their own Koha instance and you want your users to be able to search for books in your other libraries. Please note that this will only work with Koha instances. The links in the results page won't work with other normal Z39.50 servers. Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Hm not sure, but this might overlap with bug 10486.
I think I would prefer to pursue bug 10486 as it should work with any Z39.50 target. Also, this patch appears to duplicate a lot of Z39.50 searching code. Seeing that I already made a copy of a lot of the code with the Z39.50 authority search, I think we should be looking to minimize boilerplate/duplicated code.
This feature may be provided as a new feature called federated search via z39.5/SRU servers. The function may be same as at the time of data entry via z39.5 only the format of result display needs to be changes. With this facility libraries may search the remote z39.5/SRU enabled OPAC as well as they can configure their Institutional repositories also like in DSpace and eprints.
Created attachment 60950 [details] [review] Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances This patch adds the OPACZ3950 preference which activates a "Z39.50 Search" page in the OPAC. This page can be used to let members search for biblio records on remote Koha instances. If the "Show" link is clicked in the results page, the user is then redirected to the remote Koha's OPAC to see the record's details. This feature can be useful if you have multiple libraries with their own Koha instance and you want your users to be able to search for books in your other libraries. Please note that this will only work with Koha instances. The links in the results page won't work with other normal Z39.50 servers. Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Created attachment 60951 [details] [review] Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances This patch adds the OPACZ3950 preference which activates a "Z39.50 Search" page in the OPAC. This page can be used to let members search for biblio records on remote Koha instances. If the "Show" link is clicked in the results page, the user is then redirected to the remote Koha's OPAC to see the record's details. This feature can be useful if you have multiple libraries with their own Koha instance and you want your users to be able to search for books in your other libraries. Please note that this will only work with Koha instances. The links in the results page won't work with other normal Z39.50 servers. Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Created attachment 60952 [details] [review] Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances This patch adds the OPACZ3950 preference which activates a "Z39.50 Search" page in the OPAC. This page can be used to let members search for biblio records on remote Koha instances. If the "Show" link is clicked in the results page, the user is then redirected to the remote Koha's OPAC to see the record's details. This feature can be useful if you have multiple libraries with their own Koha instance and you want your users to be able to search for books in your other libraries. Please note that this will only work with Koha instances. The links in the results page won't work with other normal Z39.50 servers. Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Hi all, I rebased all patches and squash all in one good patch. It apply correcly under the master. I think, its the alternative of 10486, because it not progress. Thanks.
Comment on attachment 60952 [details] [review] Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances Review of attachment 60952 [details] [review]: ----------------------------------------------------------------- Hi Blou, did a first code review. Additionally to the comments I made I noticed that the changes to kohastructure.sql are misssing for the column. ::: C4/Auth.pm @@ +523,4 @@ > OPACURLOpenInNewWindow => "" . C4::Context->preference("OPACURLOpenInNewWindow"), > OPACUserCSS => "" . C4::Context->preference("OPACUserCSS"), > OpacAuthorities => C4::Context->preference("OpacAuthorities"), > + OpacZ3950 => C4::Context->preference("OpacZ3950"), It's better to use the newer TT plugin method to check system preferences. I think this is done below, so this can possibly be reoved. ::: admin/z3950servers.pl @@ +66,4 @@ > $id = 0; > } elsif ( $op eq 'add_validated' ) { > my @fields=qw/host port db userid password rank syntax encoding timeout > + recordtype checked servername servertype sru_options sru_fields attributes opac attributes is added by another patch on bug 11297, so should be marked as a dependency or removed here. ::: installer/data/mysql/sysprefs.sql @@ +569,5 @@ > ('XSLTListsDisplay','default','','Enable XSLT stylesheet control over lists pages display on intranet','Free'), > ('XSLTResultsDisplay','default','','Enable XSL stylesheet control over results page display on intranet','Free'), > ('z3950AuthorAuthFields','701,702,700',NULL,'Define the MARC biblio fields for Personal Name Authorities to fill biblio.author','free'), > +('z3950NormalizeAuthor','0','','If ON, Personal Name Authorities will replace authors in biblio.author','YesNo'), > +('OPACZ3950','0','','Allow patrons to search for bibliographic records in other libraries via Z39.50.','YesNo') The list in sysprefs.sql should be sorted alphabetically to minimize conflicts. ::: installer/data/mysql/updatedatabase.pl @@ +13959,4 @@ > print "Upgrade to $DBversion done (Bug 16530 - Add a circ sidebar navigation menu)\n"; > } > > +$DBversion = "XXX"; This should be an atomicupdate in a separate file now instead of an entry in updatedatabase. @@ +13960,5 @@ > } > > +$DBversion = "XXX"; > +if( CheckVersion( $DBversion ) ) { > + $dbh->do("ALTER TABLE z3950servers ADD COLUMN opac TINYINT(1);"); Please specify where the new column should be added using AFTER/BEFORE column. @@ +13962,5 @@ > +$DBversion = "XXX"; > +if( CheckVersion( $DBversion ) ) { > + $dbh->do("ALTER TABLE z3950servers ADD COLUMN opac TINYINT(1);"); > + $dbh->do("INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES('OPACZ3950','0','','Allow patrons to search for bibliographic records in other libraries via Z39.50.','YesNo')"); > + print "Add opac column to z3950servers table.\nAdd OPACZ3950 preference.\n"; I am not sure if the \n work nicely in the HTML output of the web installer, maybe remove them. ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt @@ +235,4 @@ > You searched for [% searchfield %] > [% END %] > <table id="serverst"> > + <thead><tr><th>Target</th><th>Hostname/Port</th><th>Database</th><th>Userid</th><th>Password</th><th>Preselected</th><th>Rank</th><th>Syntax</th><th>Encoding</th><th>Timeout</th><th>Record type</th><th>Attributes</th><th>OPAC</th><th>Options</th> Attributes are from bug 11297 - see above. ::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc @@ +273,4 @@ > [% IF ( Koha.Preference( 'UseCourseReserves' ) == 1 ) %]<li><a href="/cgi-bin/koha/opac-course-reserves.pl">Course reserves</a></li>[% END %] > [% IF Koha.Preference( 'OpacBrowser' ) == 1 %]<li><a href="/cgi-bin/koha/opac-browser.pl">Browse by hierarchy</a></li>[% END %] > [% IF Koha.Preference( 'OpacAuthorities' ) == 1 %]<li><a href="/cgi-bin/koha/opac-authorities-home.pl">Authority search</a></li>[% END %] > + [% IF Koha.Preference( 'OpacZ3950' ) %]<li><span id="z3950searchOpac"><a target="_blank" href="#" >Z39.50 Search</a></span></li>[% END %] Not sure patrons will know what Z39.50 means, maybe we could find another label - "search other catalogs" maybe? ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-z3950-search.tt @@ +52,5 @@ > + { label => "Keyword (any)", prop => 'srchany' }, > + { label => "LC call number", prop => 'lccall' }, > + { label => "Control no.", prop => 'controlnumber' }, > + { label => "Dewey", prop => 'dewey' }, > + { label => "Standard ID", prop => 'stdid' } Not sure if this syntax is translatable - needs testing. @@ +64,5 @@ > + <input type="hidden" name="op" id="op" value="do_search" /> > + <input type="hidden" name="biblionumber" value="[% biblionumber %]" /> > + <input type="hidden" name="frameworkcode" value="[% frameworkcode %]" /> > + <div id="search_params" class="span6 form-horizontal"> > + <h2>Search Parameters</h2> Should be: Search parameters (only first word capitalized in Koha) @@ +180,5 @@ > + <div class="row-fluid"> > + <div class="span4"> > + <div class="input-prepend input-append"> > + [% IF ( show_prevbutton ) %] > + <button class="btn" type="button" name="changepage_prev" onclick="$('#current_page').val([% current_page %]-1);$('#page_form').submit();">Previous Page</button> see above (Previous page) @@ +184,5 @@ > + <button class="btn" type="button" name="changepage_prev" onclick="$('#current_page').val([% current_page %]-1);$('#page_form').submit();">Previous Page</button> > + [% END %] > + <span class="add-on">Page [% current_page %] / [% total_pages %]</span> > + [% IF ( show_nextbutton ) %] > + <button class="btn" type="button" name="changepage_next" onclick="$('#current_page').val([% current_page %]+1);$('#page_form').submit();">Next Page</button> see above (Next page) ::: opac/opac-z3950-search.pl @@ +18,5 @@ > +# with Koha; if not, write to the Free Software Foundation, Inc., > +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + > +use strict; > +use warnings; Should be: use Modern::Perl @@ +51,5 @@ > +my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ > + template_name => "opac-z3950-search.tt", > + query => $input, > + type => "opac", > + authnotrequired => 1, All pages should require auth if OpacPublic is set to Disable: authnotrequired => ( C4::Context->preference("OpacPublic") ? 1 : 0 ),
Created attachment 72812 [details] Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances ::: C4/Auth.pm @@ +523,4 @@ > OPACURLOpenInNewWindow => "" . C4::Context->preference("OPACURLOpenInNewWindow"), > OPACUserCSS => "" . C4::Context->preference("OPACUserCSS"), > OpacAuthorities => C4::Context->preference("OpacAuthorities"), > + OpacZ3950 => C4::Context->preference("OpacZ3950"), It's better to use the newer TT plugin method to check system preferences. I think this is done below, so this can possibly be reoved. ::: installer/data/mysql/sysprefs.sql @@ +569,5 @@ > ('XSLTListsDisplay','default','','Enable XSLT stylesheet control over lists pages display on intranet','Free'), > ('XSLTResultsDisplay','default','','Enable XSL stylesheet control over results page display on intranet','Free'), > ('z3950AuthorAuthFields','701,702,700',NULL,'Define the MARC biblio fields for Personal Name Authorities to fill biblio.author','free'), > +('z3950NormalizeAuthor','0','','If ON, Personal Name Authorities will replace authors in biblio.author','YesNo'), > +('OPACZ3950','0','','Allow patrons to search for bibliographic records in other libraries via Z39.50.','YesNo') The list in sysprefs.sql should be sorted alphabetically to minimize conflicts. ::: installer/data/mysql/updatedatabase.pl @@ +13959,4 @@ > print "Upgrade to $DBversion done (Bug 16530 - Add a circ sidebar navigation menu)\n"; > } > > +$DBversion = "XXX"; This should be an atomicupdate in a separate file now instead of an entry in updatedatabase. @@ +13960,5 @@ > } > > +$DBversion = "XXX"; > +if( CheckVersion( $DBversion ) ) { > + $dbh->do("ALTER TABLE z3950servers ADD COLUMN opac TINYINT(1);"); Please specify where the new column should be added using AFTER/BEFORE column. @@ +13962,5 @@ > +$DBversion = "XXX"; > +if( CheckVersion( $DBversion ) ) { > + $dbh->do("ALTER TABLE z3950servers ADD COLUMN opac TINYINT(1);"); > + $dbh->do("INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES('OPACZ3950','0','','Allow patrons to search for bibliographic records in other libraries via Z39.50.','YesNo')"); > + print "Add opac column to z3950servers table.\nAdd OPACZ3950 preference.\n"; I am not sure if the \n work nicely in the HTML output of the web installer, maybe remove them. ::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc @@ +273,4 @@ > [% IF ( Koha.Preference( 'UseCourseReserves' ) == 1 ) %]<li><a href="/cgi-bin/koha/opac-course-reserves.pl">Course reserves</a></li>[% END %] > [% IF Koha.Preference( 'OpacBrowser' ) == 1 %]<li><a href="/cgi-bin/koha/opac-browser.pl">Browse by hierarchy</a></li>[% END %] > [% IF Koha.Preference( 'OpacAuthorities' ) == 1 %]<li><a href="/cgi-bin/koha/opac-authorities-home.pl">Authority search</a></li>[% END %] > + [% IF Koha.Preference( 'OpacZ3950' ) %]<li><span id="z3950searchOpac"><a target="_blank" href="#" >Z39.50 Search</a></span></li>[% END %] Not sure patrons will know what Z39.50 means, maybe we could find another label - "search other catalogs" maybe? @@ +64,5 @@ > + <input type="hidden" name="op" id="op" value="do_search" /> > + <input type="hidden" name="biblionumber" value="[% biblionumber %]" /> > + <input type="hidden" name="frameworkcode" value="[% frameworkcode %]" /> > + <div id="search_params" class="span6 form-horizontal"> > + <h2>Search Parameters</h2> Should be: Search parameters (only first word capitalized in Koha) @@ +180,5 @@ > + <div class="row-fluid"> > + <div class="span4"> > + <div class="input-prepend input-append"> > + [% IF ( show_prevbutton ) %] > + <button class="btn" type="button" name="changepage_prev" onclick="$('#current_page').val([% current_page %]-1);$('#page_form').submit();">Previous Page</button> see above (Previous page) @@ +184,5 @@ > + <button class="btn" type="button" name="changepage_prev" onclick="$('#current_page').val([% current_page %]-1);$('#page_form').submit();">Previous Page</button> > + [% END %] > + <span class="add-on">Page [% current_page %] / [% total_pages %]</span> > + [% IF ( show_nextbutton ) %] > + <button class="btn" type="button" name="changepage_next" onclick="$('#current_page').val([% current_page %]+1);$('#page_form').submit();">Next Page</button> see above (Next page) ::: opac/opac-z3950-search.pl @@ +18,5 @@ > +# with Koha; if not, write to the Free Software Foundation, Inc., > +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + > +use strict; > +use warnings; Should be: use Modern::Perl ::: opac/opac-z3950-search.pl @@ +51,5 @@ > +my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ > + template_name => "opac-z3950-search.tt", > + query => $input, > + type => "opac", > + authnotrequired => 1, All pages should require auth if OpacPublic is set to Disable: authnotrequired => ( C4::Context->preference("OpacPublic") ? 1 : 0 ), These are the review points left to be corrected. ::: admin/z3950servers.pl @@ +66,4 @@ > $id = 0; > } elsif ( $op eq 'add_validated' ) { > my @fields=qw/host port db userid password rank syntax encoding timeout > + recordtype checked servername servertype sru_options sru_fields attributes opac attributes is added by another patch on bug 11297, so should be marked as a dependency or removed here. ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-z3950-search.tt @@ +52,5 @@ > + { label => "Keyword (any)", prop => 'srchany' }, > + { label => "LC call number", prop => 'lccall' }, > + { label => "Control no.", prop => 'controlnumber' }, > + { label => "Dewey", prop => 'dewey' }, > + { label => "Standard ID", prop => 'stdid' } Not sure if this syntax is translatable - needs testing. ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt @@ +235,4 @@ > You searched for [% searchfield %] > [% END %] > <table id="serverst"> > + <thead><tr><th>Target</th><th>Hostname/Port</th><th>Database</th><th>Userid</th><th>Password</th><th>Preselected</th><th>Rank</th><th>Syntax</th><th>Encoding</th><th>Timeout</th><th>Record type</th><th>Attributes</th><th>OPAC</th><th>Options</th> Attributes are from bug 11297 - see above. Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Ignore comment 9, sorry about that.
Created attachment 72813 [details] [review] Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances Made changes wanted in the code review. members search for records on remote Koha instances
Try to test on sandbox 01 (https://wiki.koha-community.org/wiki/Sandboxes) Some problems occurred applying patches from bug 11361: <h1>Something went wrong !</h1>Applying: Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances fatal: sha1 information is lacking or useless (C4/Auth.pm). error: could not build fake ancestor Patch failed at 0001 Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run git bz apply --continue.
Checked in my devbox too: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances fatal: sha1 information is lacking or useless (C4/Auth.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 11361 - Add a Z39.50 search page in the OPAC to let members search for records on remote Koha instances The copy of the patch that failed is found in: /home/vagrant/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/Bug-11361---Add-a-Z3950-search-page-in-the-OPAC-to-mVnpFd.patch
It looks like the attached patch is not complete - also there might be a rebase mistake in Auth.pm.
Created attachment 95808 [details] [review] Bug11361 - opac z3950 search bug11361 - atomicupdate bug
I rebased the patch and it should apply correctly, here is a test plan: - apply patch - update the database - set "OpacZ3950" to "Allow" in OPAC preferences - go in Z39.50/SRU servers administration - edit one server line - check the "OPAC (Shown in OPAC Z39.50 search)" checkbox - go in the opac - click the "Z39.50 Search" link on top - check the z3950 server you want and make a search - verify that you have results If you need a Z3950 server for testing purpose, here is the library of congress online catalog: hostname: lx2.loc.gov port: 210 database: LCDB syntax: MARC21/USMARC encoding: utf8
I get this error when applying the database update: root@2fa601aba3df:koha(bz11361)$ perl installer/data/mysql/updatedatabase.pl DEV atomic update: bug11361.perl DBD::mysql::db do failed: Unknown column 'attributes' in 'z3950servers' [for Statement "ALTER TABLE z3950servers ADD COLUMN opac TINYINT(1) AFTER attributes;"] at (eval 1404) line 3. Add opac column to z3950servers table.Add OPACZ3950 preference.
Created attachment 100077 [details] [review] Bug 11361 - Fixing database and merge conflicts
Should now pass the database update, also solved some new merge conflicts
Thanjs for fixing the database update. Two things from testing: 1) After applying the patch this text appears on the Z39.50/SRU servers administration page, and the header row for the table is repeated: Z39.50/SRU servers administration <<<<<<< f7630d746947b82b40098be4cbac266e4a494553 ======= >>>>>>> Bug11361 - opac z3950 search <<<<<<< f7630d746947b82b40098be4cbac266e4a494553 ======= >>>>>>> Bug11361 - opac z3950 search <<<<<<< f7630d746947b82b40098be4cbac266e4a494553 ======= >>>>>>> Bug11361 - opac z3950 search <<<<<<< f7630d746947b82b40098be4cbac266e4a494553 ======= >>>>>>> Bug11361 - opac z3950 search <<<<<<< f7630d746947b82b40098be4cbac266e4a494553 ======= >>>>>>> Bug11361 - opac z3950 search <<<<<<< f7630d746947b82b40098be4cbac266e4a494553 ======= >>>>>>> Bug11361 - opac z3950 search 2) On the search results when you click on the "Show" link it comes up as "this page cannot be reached" in the main browser window for the URL http://cgi-bin/koha/opac-detail.pl?biblionumber= (MARC and Card links work fine as pop-ups in the Z39.50 search results window).
Created attachment 100463 [details] [review] Bug 11361 - Fixing z3950servers.tt problems
Created attachment 100470 [details] [review] Bug 11361 - Fixing database and merge conflicts Bug 11361 - Fixing z3950servers.tt problems Bug 11361 - Re-adding the 'Attributes' column Bug 11361 - Fixing z3950servers.tt problems
Thanks for testing ! Solved the first problem The "Show" button is here if we do a Z3950 search in our own database. We could remove it or make it invisible if it leads to nothing.
Hi Arthur. I can confirm that issue 1 is now solved! For issue 2, where should the 'Show' link go to? For me, the 'Show' link is trying to link to http://cgi-bin/koha/opac-detail.pl?biblionumber= Is it supposed to link to the record in the remote Koha instance?
Created attachment 113334 [details] [review] Bug 11361 - Fixing database and merge conflicts Bug 11361 - Fixing z3950servers.tt problems Bug 11361 - Re-adding the 'Attributes' column Bug 11361 - Fixing z3950servers.tt problems
Fixing conflicts. I am not able to fix the broken link ... When we post the form to start the search, we send a hidden input : <pre> <input type="hidden" name ="biblionumber" value="0"> </pre> However, we use this value to put it into the broken link...
(In reply to Frédérick Capovilla from comment #0) > This patch adds the OPACZ3950 preference which activates a "Z39.50 Search" > page in the OPAC. > That sounds like a misleading preference name which could easily confuse people. Perhaps a better one would be something like SearchRemoteKoha. > This page can be used to let members search for biblio records on remote > Koha instances. If the "Show" link is clicked in the results page, the user > is then redirected to the remote Koha's OPAC to see the record's details. > > This feature can be useful if you have multiple libraries with their own > Koha instance and you want your users to be able to search for books in your > other libraries. > > Please note that this will only work with Koha instances. The links in the > results page won't work with other normal Z39.50 servers. > It's an interesting idea. In the past, we put together a Koha union catalogue that harvested from individual Koha instances (we injected the Koha URLs into the records as they're harvested). We didn't use the union catalogue in the end, but that seems a more standards based way of doing things.
Created attachment 119535 [details] [review] Bug 11361 - Fixing database and merge conflicts Bug 11361 - Fixing z3950servers.tt problems Bug 11361 - Re-adding the 'Attributes' column Bug 11361 - Fixing z3950servers.tt problems
Created attachment 119536 [details] [review] Bug 11361 - (QA follow-up) Add filters and change forbidden patterns
Created attachment 119928 [details] Bug 11361 - Screenshots of pop-up window display I've quickly tested again. Screenshots attached from using Firefox Developer Edition on Ubuntu 18.04. The pop-up window for the display for searching doesn't look right. This may be because of the Bootstrap update changes. I'm assuming that if it is setup to search another Koha catalog, then the MARC and Card links will work (instead of page not found errors) and that the Show link will work as well (currently links to http://cgi-bin/koha/opac-detail.pl?biblionumber=). To test properly, does anyone have the details for a Koha catalog which would work?