Bug 28773 - Aquisitions from external source not working for non english language
Summary: Aquisitions from external source not working for non english language
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords: rel_21_05_candidate
Depends on: 23302
Blocks:
  Show dependency treegraph
 
Reported: 2021-07-28 09:46 UTC by Mark Hofstetter
Modified: 2021-08-11 20:04 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.03


Attachments
Bug 28773: (bug 23302 follow-up) Fix add order from z3950 search results (4.94 KB, patch)
2021-07-28 10:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28773: (bug 23302 follow-up) Fix add order from z3950 search results (5.00 KB, patch)
2021-07-29 06:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28773: (bug 23302 follow-up) Fix add order from z3950 search results (5.09 KB, patch)
2021-07-30 07:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28773: (QA follow-up) Additional changes (8.85 KB, patch)
2021-07-30 09:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hofstetter 2021-07-28 09:46:42 UTC
if you try to order a book via an external source (SRU search) the order button is not working if you are using an non-english language

z3950_search_21.0501003.js:133 Uncaught TypeError: Cannot read property 'document' of null
    at HTMLAnchorElement.<anonymous> (z3950_search_21.0501003.js:133)
    at HTMLTableElement.dispatch (jquery-2.2.3.min_21.0501003.js:3)
    at HTMLTableElement.r.handle (jquery-2.2.3.min_21.0501003.js:3)

bug confirmed by 
(11:44:25) Joubu: It's caused by 23302, link it with that bug number when you have opened the new one
Comment 1 Jonathan Druart 2021-07-28 10:06:24 UTC
Created attachment 123244 [details] [review]
Bug 28773: (bug 23302 follow-up) Fix add order from z3950 search results

For translated interface.

Using 'Order' (the button text) was not a good idea as the interface can
be translated :)
Comment 2 Jonathan Druart 2021-07-28 10:07:05 UTC
This is my try but I am not happy with it.

Letting Mark having his try!
Comment 3 Mark Hofstetter 2021-07-28 13:33:58 UTC
Honestly - I wouldn't have done it differently, and if so only worse.

But I tested it and it works now. On my behalf I'd sign it off, thx a lot
Comment 4 Jonathan Druart 2021-07-29 06:34:30 UTC
Created attachment 123277 [details] [review]
Bug 28773: (bug 23302 follow-up) Fix add order from z3950 search results

For translated interface.

Using 'Order' (the button text) was not a good idea as the interface can
be translated :)

Signed-off-by: Mark Hofstetter <koha@trust-box.at>
Comment 5 Jonathan Druart 2021-07-29 06:35:01 UTC
Marcel, can you QA this one please?
Comment 6 Marcel de Rooy 2021-07-29 07:03:30 UTC
(In reply to Jonathan Druart from comment #5)
> Marcel, can you QA this one please?

Sure
Comment 7 Marcel de Rooy 2021-07-30 07:03:46 UTC
(In reply to Jonathan Druart from comment #1)
> For translated interface.
> 
> Using 'Order' (the button text) was not a good idea as the interface can
> be translated :)

Hmm. The problem is not the button text, but the title attribute.

Some attributes are also translated:

 <li><a class="chosen" data-action="order" href="/cgi-bin/koha/acqui/neworderempty.pl?frameworkcode=[% frameworkcode | uri %]&breedingid=[% breeding_loo.breedingid | uri %]&booksellerid=[% booksellerid | uri %]&basketno=[% basketno | uri %]" title="Bestellung"><i class="fa fa-plus"></i> [% tp('verb', 'Order') | html %]</a></li>

xgettext.pl contains:

if ( $a =~ /title|value|alt|content|placeholder|aria-label/ ) {

But your patch resolves the bug with another (untranslated) attribute.
Comment 8 Marcel de Rooy 2021-07-30 07:05:07 UTC
We only have an existing inconsistency where Order is translated via tp but MARC and Card are going via the regular translation path.
Comment 9 Marcel de Rooy 2021-07-30 07:13:17 UTC
WARNING: OUT OF SCOPE

Just strolling into I18N, I am seeing this:

sub _expand {
    my ($text, %vars) = @_;

    my $re = join '|', map { quotemeta $_ } keys %vars;
    $text =~ s/\{($re)\}/defined $vars{$1} ? $vars{$1} : "{$1}"/ge;

Can this be true?
Comment 10 Marcel de Rooy 2021-07-30 07:15:47 UTC
Created attachment 123291 [details] [review]
Bug 28773: (bug 23302 follow-up) Fix add order from z3950 search results

For translated interface.

Using 'Order' (the button text) was not a good idea as the interface can
be translated :)

Signed-off-by: Mark Hofstetter <koha@trust-box.at>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2021-07-30 07:23:59 UTC
Just wait
Comment 12 Marcel de Rooy 2021-07-30 09:18:08 UTC
Created attachment 123292 [details] [review]
Bug 28773: (QA follow-up) Additional changes

The work of the first patch was not yet finished.

[1] Similar changes to template for Cataloging and Authorities
[2] Made a better distinction now between short title on button
    and long title on menu and modal
[3] The short title parameter removes need for previewed var
[4] To keep related code closer, moved the dataPreview hidden code

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2021-07-30 09:25:13 UTC
Julian, Could you have a look at comment9 ?
How do you get the result with defined $x evaluated ?
Comment 14 Jonathan Druart 2021-08-04 12:06:24 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 15 Kyle M Hall 2021-08-06 18:15:00 UTC
Pushed to 21.05.x for 21.05.03
Comment 16 Fridolin Somers 2021-08-11 20:04:08 UTC
Depends on Bug 23302 not in 20.11.x