http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10486

--- Comment #33 from Katrin Fischer <katrin.fisc...@bsz-bw.de> ---
Comment on attachment 24206
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24206
Bug 10486 - Allow external Z39.50 targets to be searched from the OPAC

Review of attachment 24206:
 --> 
(http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10486&attachment=24206)
-----------------------------------------------------------------

Hi Jesse, I got curious and took a look at the code. Looks mostly good (as far
as I can tell), but I spotted some translation issues in the templates. Could
you take a look please?

::: C4/Search.pm
@@ +72,4 @@
>    &GetDistinctValues
>    &enabled_staff_search_views
>    &PurgeSearchHistory
> +  &GetExternalSearchTargets

New routine - being super strict I would expect some unit tests :)

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/external-targets.tt
@@ +27,5 @@
> +    $( '#targets tr[data-targetid=[% saved_id %]]' ).addClass( 'updated' );
> +    [% END %]
> +
> +    $( '#targets .delete' ).click( function() {
> +        return confirm( _( 'Are you sure you wish to delete this target?' ) 
> );

Please use "" else a translation using ' can break the Javascript code.

::: koha-tmpl/opac-tmpl/prog/en/modules/opac-external-search.tt
@@ +67,5 @@
> +    } );
> +}
> +
> +function search( offset, reset_search ) {
> +    $( '#pazpar2-status' ).html( 'Searching external targets... <img 
> class="throbber" 
> src="/opac-tmpl/lib/jquery/plugins/themes/classic/throbber.gif" /></span>' );

'Searching external targets' is untranslatable. Marking all spotted translation
problems with 'T'.

@@ +77,5 @@
> +    }
> +
> +    function callback( data ) {
> +        if ( data.error ) {
> +            $( '#pazpar2-status' ).html( '<strong class="unavailable">Error 
> searching external targets.</strong>' );

T

@@ +82,5 @@
> +            return;
> +        }
> +
> +        if ( !data.total ) {
> +            $( '#pazpar2-status' ).html( '<strong>No results found in the 
> external targets.</strong>' );

T

@@ +88,5 @@
> +        }
> +
> +        $( '#results tbody' ).empty();
> +
> +        $( '#pazpar2-status' ).html( '<strong>' + _( 'Found ' ) + data.total 
> + _( ' results in ' ) + $( '#targets-facet input:checked' ).length + _( ' 
> external targets.' ) + '</strong>' );

T (single quotes). Also a bit hard to translate, as this will be split up in
the translation file.

@@ +104,5 @@
> +
> +            if ( resultRenderCache[ hit.recid[0] ] ) {
> +                results.push( resultRenderCache[ hit.recid[0] ] );
> +            } else {
> +                results.push( hit['md-work-title'] ? hit['md-work-title'][0] 
> : 'Loading...' );

T

@@ +131,5 @@
> +        var cur_page = data.start / results_per_page;
> +        var max_page = Math.floor( data.total / results_per_page );
> +
> +        if ( cur_page != 0 ) {
> +            pages.push( '<a class="nav" href="#" data-offset="' + (offset - 
> results_per_page) + '">&lt;&lt; Previous</a>' );

T

@@ +143,5 @@
> +            }
> +        }
> +
> +        if ( cur_page < max_page ) {
> +            pages.push( ' <a class="nav" href="#" data-offset="' + (offset + 
> results_per_page) + '">Next >></a>' );

T

::: koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
@@ +319,4 @@
>              }
> +
> +            first_succeeded = true;
> +            FinishExternalSearch( 'pazpar2', _("Found __LINK__ in ") + 
> num_targets + _(" external targets"), data.total, 
> '/cgi-bin/koha/opac-external-search.pl?q=' + escape( querystring ) );

This looks interesting. Maybe a replacement like __LINK__ could make
translating the sentences easier.

::: opac/opac-external-search.pl
@@ +17,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;

Could be using Modern Perl instead.

::: opac/opac-search.pl
@@ +884,5 @@
>  
> +$template->{VARS}->{OPACSearchExternalTargets} = 
> C4::Context->preference('OPACSearchExternalTargets');
> +$template->{VARS}->{external_search_targets} = GetExternalSearchTargets( 
> C4::Context->userenv ? C4::Context->userenv->{branch} : '' );
> +
> +$template->{VARS}->{OverDriveLibraryID} = 
> C4::Context->preference('OverDriveLibraryID');

I think some of those lines could now be replaced using the TT plugin for
looking up the system preferences from the template. Just noting, it's ok to
leave it.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to