Bug 9141 - Untranslatable strings on "pay an amount..." page
Summary: Untranslatable strings on "pay an amount..." page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Owen Leonard
QA Contact: Paul Poulain
URL: /cgi-bin/koha/members/paycollect.pl
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-25 22:59 UTC by Katrin Fischer
Modified: 2014-05-26 21:04 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
[PATCH] Bug 9141 - Untranslatable strings on "pay an amount..." page (1.86 KB, patch)
2012-11-26 01:36 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9141 - Untranslatable strings on "pay an amount..." page (1.92 KB, patch)
2012-12-03 19:42 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 9141 - Untranslatable strings on "pay an amount..." page (1.95 KB, patch)
2013-01-04 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2012-11-25 22:59:43 UTC
paycollect.pl
100  error => sprintf( 'You must pay a value less than or equal to %f.2',  $total_due )

Message is shown when you pay a fine and enter a value that is bigger than the totel due:

You must pay a value less than or equal to 5.000000.2

Message is not translatable. Also the fine amount is formatted incorrectly.
Comment 1 Owen Leonard 2012-11-26 01:36:13 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2012-12-03 19:42:04 UTC Comment hidden (obsolete)
Comment 3 Paul Poulain 2012-12-11 19:50:15 UTC
QA comment: tiny patch, but I think it could be even simpler. Instead of using a error_over => 1, why not use total_due =
-[% IF ( error ) %]
+[% IF (total_due > 0 ) %]

should achieve the same result, isn't it ?
In this case the patch would just be:
-[% IF ( error ) %]
+[% IF (total_due > 0 ) %]
-    [% error %]
+    You must pay a value less than or equal to [% total_due | format('%.2f') %].

-            error => sprintf( 'You must pay a value less than or equal to %f.2',
-                $total_due )
+            total_due => $total_due

Your opinion ?
Comment 4 Paul Poulain 2013-01-03 16:56:39 UTC
(In reply to comment #3)
> Your opinion ?

ping...
Comment 5 Owen Leonard 2013-01-03 19:44:46 UTC
(In reply to comment #3)
> Your opinion ?

Your version is simpler, but I think what we lose is the readability of the template. Seeing that an error message is displayed if total_due > 0 doesn't mean anything unless you examine the script and see under what condition the variable is being set. The more verbose version makes it clear that the script sets an error variable.

I don't think it's a big deal, but I prefer my version.
Comment 6 Marc Véron 2013-01-04 12:54:10 UTC
I agree with Owen.
Comment 7 Jonathan Druart 2013-01-04 14:16:27 UTC
QA Comments:

With the agreement of Paul, I pass qa on this patch.
Works as expected.
Comment 8 Jonathan Druart 2013-01-04 14:16:44 UTC
Created attachment 14418 [details] [review]
Bug 9141 - Untranslatable strings on "pay an amount..." page

Moving text of an error message from the script into the
template. To test, submit an amount which is greater
than the amount due. You should see an error message
displayed correctly with a properly-formatted currency
amount.

Signed-off-by: Marc Veron <veron@veron.ch>

Patch behaves as expected.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 9 Jared Camins-Esakov 2013-01-08 01:48:57 UTC
This patch has been pushed to master.
Comment 10 Chris Cormack 2013-01-12 19:21:09 UTC
Pushed to 3.10.x and 3.8.x