Bug 11668

Summary: Untranslatable "Total" in borrowers stats and other reports
Product: Koha Reporter: paxed <pasi.kallinen>
Component: I18N/L10NAssignee: Bugs List <koha-bugs>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: bgkriegel, f.demians, katrin.fischer, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11387
Change sponsored?: --- Patch complexity: String patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 11848    
Bug Blocks:    
Attachments: Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable
[Signed-off] Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable
[Follow-up] Fix untranslatable "Total" in more files
Fix untranslatable "Total" in more files
Fix untranslatable "Total" in more files
Bug 11668: Fix untranslatable "Total" in more files
Bug 11668: Fix untranslatable "Total" in more files
[PASSED QA] Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable
[PASSED QA] Bug 11668: Fix untranslatable "Total" in more files
Bug 11668: Follow-up - fix POD
Bug 11668: another approach

Description paxed 2014-02-03 07:06:55 UTC
When saving borrowers_stats output into CSV, there will be hard-coded, untranslatable, "Total" and "TOTAL" texts.

reports/borrowers_stats.pl:                 print "Total\n";
Comment 1 paxed 2014-02-03 07:16:25 UTC
It would also be good to have the field names translatable (eg. "zipcode/branchcode" in the output)
Comment 2 Galen Charlton 2014-05-05 04:55:48 UTC Comment hidden (obsolete)
Comment 3 Marc Véron 2014-09-06 06:26:05 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2014-09-06 06:32:05 UTC
(In reply to paxed from comment #1)
> It would also be good to have the field names translatable (eg.
> "zipcode/branchcode" in the output)

I propose to move this request to a new bug.
Comment 5 Marc Véron 2014-09-07 07:45:23 UTC
Fixed by first patch (Signed-off):

reports/borrowers_stats.pl:             print "Total\n";
reports/borrowers_stats.pl:             print "TOTAL";


There are more hardcoded "Total" in reports (git grep 'print "T'):

reports/acquisitions_stats.pl:        print "Total\n";
reports/acquisitions_stats.pl:        print "TOTAL";
reports/bor_issues_top.pl:        print "TOTAL";
reports/borrowers_out.pl:        print "Total\n";
reports/borrowers_out.pl:        print "TOTAL";
reports/cat_issues_top.pl:        print "Total\n";
reports/cat_issues_top.pl:        print "TOTAL";
reports/catalogue_stats.pl:             print "Total\n";
reports/catalogue_stats.pl:             print "TOTAL";
reports/issues_avg_stats.pl:        print "Total\n";
reports/issues_avg_stats.pl:        print "TOTAL";
reports/issues_stats.pl:                print "Total\n";
reports/issues_stats.pl:        print "TOTAL";
reports/reserves_stats.pl:              print "Total\n";
reports/reserves_stats.pl:        print "TOTAL";
Comment 6 Marc Véron 2014-09-07 19:11:50 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2014-09-07 19:13:41 UTC
Second patch needs Sign-off
Comment 8 Owen Leonard 2014-09-10 15:18:01 UTC
I tried testing this with reports/acquisitions_stats.pl and got an error instead of a CSV file:

Undefined subroutine main::gettext called at kohaclone/reports/acquisitions_stats.pl line 97.

I see that the first patch adds "use Koha::I18N;" and the follow-ups don't. Looks like it might be required.
Comment 9 Marc Véron 2014-09-10 15:41:39 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2014-09-10 15:47:41 UTC Comment hidden (obsolete)
Comment 11 Marc Véron 2014-10-20 13:13:24 UTC
Still applies on current master: 3.17.00.029
Comment 12 Chris Cormack 2014-10-25 03:12:27 UTC Comment hidden (obsolete)
Comment 13 Chris Cormack 2014-10-25 03:13:43 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2014-10-25 10:17:50 UTC
Created attachment 32711 [details] [review]
[PASSED QA] Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable

To test:

[1] Generate the PO files for the language of your choice.
[2] Supply translations for the "Total" and "TOTAL" strings
    in the $LANG-messages.op file.
[3] Install the translation.
[4] Change the staff interface to the chosen language.
[5] Run the borrowers statistics report and export the
    results to CSV.
[5] Verfiy that the strings "Total" and "TOTAL" are
    replaced with the translation made in step 2.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

Patch behaves as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2014-10-25 10:17:59 UTC
Created attachment 32712 [details] [review]
[PASSED QA] Bug 11668: Fix untranslatable "Total" in more files

This patch fixes untranslatable "Total" in following files:

reports/acquisitions_stats.pl
reports/bor_issues_top.pl
reports/borrowers_out.pl
reports/cat_issues_top.pl
reports/catalogue_stats.pl
reports/issues_avg_stats.pl
reports/issues_stats.pl
reports/reserves_stats.pl

To test follow test plan from first patch as appropiate.

(Missing use Koha::I18N; added)

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2014-10-25 10:18:09 UTC
Created attachment 32713 [details] [review]
Bug 11668: Follow-up - fix POD

The QA script complained about some POD errors. I fixed those.

I left the tabs for now, as the files are all tabs.
Comment 17 Katrin Fischer 2014-10-25 10:22:13 UTC
I am not sure about this patch.

It works as described, but it introduces a new po file (po/de-DE-messages.po) that the translation manager will have to add to Pootle. Adding Bernardo for his opinion.

Also, it looks like we have a bit of a mix, some TOTAL are in the normal po file and some are in the new file.

On the other hand, we introduced this new technique and so it seems valid to use it :) Maybe we need to think about when we want to insist on adding strings to the template and when we prefer the new gettext().
Comment 18 Bernardo Gonzalez Kriegel 2014-10-25 12:24:21 UTC
(In reply to Katrin Fischer from comment #17)
> I am not sure about this patch.
> 

I'm also not sure. It does not work in the same way as normal translations, but until we have a better way it's better than nothing.
Comment 19 Tomás Cohen Arazi 2015-01-10 19:10:06 UTC
I'm not sure about the approach. We've been making an effort to remove the whole strings from the code, and there's no current workflow for including this .po on the translation processs...
I'll mark it as In-Discussion, and probably try to post a counter-patch that avoids Koha::i18n. I'm open to discuss it on a dev meeting soon.
Comment 20 Bernardo Gonzalez Kriegel 2015-03-11 14:15:52 UTC
Created attachment 36794 [details] [review]
Bug 11668: another approach

In this patch I pass the messages to be printed
as hidden parameters that are set using JQuery,
and translated in the usual way.

To test:
1) Apply the patch
2) Update and install de-DE (works with this lang)
3) Go to Reports > Patron statistics
4) Select Patron category in row, Library as column
(just an example), export as CSV
5) Switch to German and repeat 4
6) Check translated 'Total/TOTAL' in german CSV

Just a idea, if you like it, can be extended to
the other cases