Bug 18694

Created attachment 63836 [details] [review]
Bug 18694: Translatability: Get rid of exposing [%% FOREACH in csv/cash_register_stats.tt

Code exposed is:
%s %s %s [%%- FOREACH field IN row; field IF !loop.last; ItemTypes.GetDescription(field) IF loop.last; sep IF !loop.last; END %%] %s TOTAL [%%- FOREACH field IN total; field; sep IF !loop.last; END %%]

Similar for: csv/orders_by_budget.tt

To test:
- Apply patch
- In staff client, verify that Cash register statistics work as before
  (Home > Reports > Cash register statistics)
- Verify the same for Home > Reports > Orders by fund (triggers orders_by_budget.tt)
- Bonus test: Create a "language" aa-AA (perl translate create aa-AA
  from folder misc/translator, verify that lines mentioned above do
  no longer appear in aa-AA-staff-prog.po )
- Run QA tools (newest version with test for newlines in tt directives)

It does not work, the funds are all displayed on the same line.

(In reply to Jonathan Druart from comment #3)
> It does not work, the funds are all displayed on the same line.

Oh, that is a problem for translations, because in the target language the result of the translation process is the same one-liner as in my patch.

Setting importance to normal because it is not only a string change, but functionality in target languages seems to be broken.

(In reply to Marc Véron from comment #4)
> (In reply to Jonathan Druart from comment #3)
> > It does not work, the funds are all displayed on the same line.
> 
> Oh, that is a problem for translations, because in the target language the
> result of the translation process is the same one-liner as in my patch.

I only tested on the non-translated interface.

(In reply to Jonathan Druart from comment #3)
> It does not work, the funds are all displayed on the same line.

Hi Jonathan

I took some time to re-test on Home > Reports > Orders by fund, but I can not recreate the issue.

The funds display properly if I choose Output > To screen
The funds are properly imported from csv to LibreOffice Calc if I choose Ooutput > To a file.

Where do you see the funds in one line? Can you pls. provide steps to reproduce and/or a screen shot?

Thanks
Marc

Hi Marc, 

make sure the string is not 'fuzzy' but actually translated. I have filed a bug a bit ago for the CSV export on item search being broken again (all in one line) once the header line is actually translated. When we tested the new fix, I think that was missed.

(In reply to Katrin Fischer from comment #7)
> Hi Marc, 
> 
> make sure the string is not 'fuzzy' but actually translated. I have filed a
> bug a bit ago for the CSV export on item search being broken again (all in
> one line) once the header line is actually translated. When we tested the
> new fix, I think that was missed.

The problem Jonathan reports is in the untranslated (English) version, see comment #5, but I can not reproduce.

Ah sorry, missed that, it sounded so familiar. - Might still be worth a test.

(In reply to Katrin Fischer from comment #7)
> Hi Marc, 
> 
> make sure the string is not 'fuzzy' but actually translated
(...)
Hi Katrin
If you do for a 'language' aa-AA:
perl translate create aa-AA
...you won't get strings marked as fuzzy

But will they not still be untranslated? My point was that if you don't properly translate, sometimes the results are not what you would expect. Running the install step will not create the same file output for an untranslated vs a translated file.

(In reply to Katrin Fischer from comment #11)
> But will they not still be untranslated? My point was that if you don't
> properly translate, sometimes the results are not what you would expect.
> Running the install step will not create the same file output for an
> untranslated vs a translated file.

Maybe there is a misunderstandig about the intention of:
perl translate create aa-AA

It is simply a test to verify that unwanted strings no longer are picked by the translation process (no longer appear in the related aa-AA*.po file).

It makes sense, I was just trying to explain something I found out recently that explains why sometimes the CSV exports tests ok, when indeed they are still broken.

(In reply to Jonathan Druart from comment #3)
> It does not work, the funds are all displayed on the same line.

Hi Jonathan

I'd like to move forward this bug, but I still can not reproduce this issue (see my comment #6 )

Can you provide steps to reproduce and/or a screenshot? 

Thanks a lot
Marc

The patch does no longer apply, I cannot retest.

(In reply to Jonathan Druart from comment #15)
> The patch does no longer apply, I cannot retest.

Patch applies cleanly here on current master

I did
git pull

HEAD is at: a38ffe2 Bug 18656 - Require confirmation of deletion of files from patron record

git bz apply 18694

63836 - Bug 18694: Translatability: Get rid of exposing [%% FOREACH in csv/cash_register_stats.tt

Yes -> applies cleanly

(In reply to Marc Véron from comment #16)
> (In reply to Jonathan Druart from comment #15)
> Yes -> applies cleanly

No it does not, this bug report is marked as dependent on bug 18734 and both patches conflict.

Created attachment 64209 [details] [review]
Bug 18694: Translatability: Get rid of exposing [%% FOREACH in csv/cash_register_stats.tt

Code exposed is:
%s %s %s [%%- FOREACH field IN row; field IF !loop.last; ItemTypes.GetDescription(field) IF loop.last; sep IF !loop.last; END %%] %s TOTAL [%%- FOREACH field IN total; field; sep IF !loop.last; END %%]

Similar for: csv/orders_by_budget.tt

To test:
- Apply patch
- In staff client, verify that Cash register statistics work as before
  (Home > Reports > Cash register statistics)
- Verify the same for Home > Reports > Orders by fund (triggers orders_by_budget.tt)
- Bonus test: Create a "language" aa-AA (perl translate create aa-AA
  from folder misc/translator, verify that lines mentioned above do
  no longer appear in aa-AA-staff-prog.po )
- Run QA tools (newest version with test for newlines in tt directives)

Rebased on top of Bug 18734 2017-12-06 / mv

Created attachment 64231 [details] [review]
Bug 18694: Translatability: Get rid of exposing [%% FOREACH in csv/cash_register_stats.tt

Code exposed is:
%s %s %s [%%- FOREACH field IN row; field IF !loop.last; ItemTypes.GetDescription(field) IF loop.last; sep IF !loop.last; END %%] %s TOTAL [%%- FOREACH field IN total; field; sep IF !loop.last; END %%]

Similar for: csv/orders_by_budget.tt

To test:
- Apply patch
- In staff client, verify that Cash register statistics work as before
  (Home > Reports > Cash register statistics)
- Verify the same for Home > Reports > Orders by fund (triggers orders_by_budget.tt)
- Bonus test: Create a "language" aa-AA (perl translate create aa-AA
  from folder misc/translator, verify that lines mentioned above do
  no longer appear in aa-AA-staff-prog.po )
- Run QA tools (newest version with test for newlines in tt directives)

Rebased on top of Bug 18734 2017-12-06 / mv

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Sorry, it is actually broken for translated language, with and without your patch.
Skipping QA.

Pushed to master for 17.11, thanks to everybody involved!

Pushed to 17.05.x, will be in 17.05.01

This patch has been pushed to 16.11.x and will be in 16.11.09.

depends on bug 6934, 

Blocked by Enhancement, skipping for 16.05.x branch