Bug 11668 - Untranslatable "Total" in borrowers stats and other reports
Summary: Untranslatable "Total" in borrowers stats and other reports
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 11848
Blocks:
  Show dependency treegraph
 
Reported: 2014-02-03 07:06 UTC by paxed
Modified: 2015-09-02 11:54 UTC (History)
5 users (show)

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


Attachments
Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable (1.55 KB, patch)
2014-05-05 04:55 UTC, Galen Charlton
Details | Diff | Splinter Review
[Signed-off] Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable (1.72 KB, patch)
2014-09-06 06:26 UTC, Marc Véron
Details | Diff | Splinter Review
[Follow-up] Fix untranslatable "Total" in more files (6.48 KB, patch)
2014-09-07 19:11 UTC, Marc Véron
Details | Diff | Splinter Review
Fix untranslatable "Total" in more files (7.56 KB, patch)
2014-09-10 15:41 UTC, Marc Véron
Details | Diff | Splinter Review
Fix untranslatable "Total" in more files (7.74 KB, patch)
2014-09-10 15:47 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 11668: Fix untranslatable "Total" in more files (7.73 KB, patch)
2014-10-25 03:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11668: Fix untranslatable "Total" in more files (7.73 KB, patch)
2014-10-25 03:13 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 11668: make hardcoded strings in reports/borrowers_stats.pl translatable (1.77 KB, patch)
2014-10-25 10:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11668: Fix untranslatable "Total" in more files (7.80 KB, patch)
2014-10-25 10:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11668: Follow-up - fix POD (1.93 KB, patch)
2014-10-25 10:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11668: another approach (3.06 KB, patch)
2015-03-11 14:15 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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