https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11361 Maksim Sen <maksim.sen@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60952|0 |1 is obsolete| | --- Comment #9 from Maksim Sen <maksim.sen@inlibro.com> --- Created attachment 72812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72812&action=edit These are the points in the code review that I corrected in this patch. ::: 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 ) -- You are receiving this mail because: You are watching all bug changes.