[Koha-bugs] [Bug 10486] Allow external Z39.50 targets to be searched from the OPAC

[bugzilla-daemon at bugs.koha-community.org]

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

--- Comment #33 from Katrin Fischer <katrin.fischer at 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) + '"><< 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.