Description
paxed
2014-02-03 07:06:55 UTC
It would also be good to have the field names translatable (eg. "zipcode/branchcode" in the output) Created attachment 27936 [details] [review] 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> Created attachment 31425 [details] [review] [Signed-off] 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> (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. 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"; Created attachment 31438 [details] [review] [Follow-up] 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 To test follow test plan from first patch as appropiate. Second patch needs Sign-off 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. Created attachment 31512 [details] [review] 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 To test follow test plan from first patch as appropiate. http://bugs.koha-community.org/show_bug.cgi?id=11668 (Missing use Koha::I18N; added) Created attachment 31513 [details] [review] 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. http://bugs.koha-community.org/show_bug.cgi?id=11668 (Missing use Koha::I18N; added) Still applies on current master: 3.17.00.029 Created attachment 32698 [details] [review] 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> Created attachment 32699 [details] [review] 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> 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> 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> 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. 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(). (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. 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. 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 |