Bug 16914 - Export csv in item search, exports all items in one line
Summary: Export csv in item search, exports all items in one line
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Josef Moravec
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 17795 (view as bug list)
Depends on: 14264
Blocks: 18331
  Show dependency treegraph
 
Reported: 2016-07-13 12:48 UTC by Hugo Agud
Modified: 2017-03-24 13:27 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 16914: Insert end of line in CSV export of item search in translated templates (2.39 KB, patch)
2016-11-03 09:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 16914 - Export csv in item search, exports all items in one line - alternate (1.26 KB, patch)
2016-11-09 13:39 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 16914 - Export csv in item search, exports all items in one line - alternate-alternate (1.08 KB, patch)
2016-12-22 12:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16914: Rely on TT for newlines (5.23 KB, patch)
2016-12-22 15:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16914: Remove unused empty_lines.inc file (649 bytes, patch)
2016-12-22 15:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16914: Rely on TT for newlines (5.28 KB, patch)
2016-12-22 18:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16914: Remove unused empty_lines.inc file (706 bytes, patch)
2016-12-22 18:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16914: Rely on TT for newlines (5.34 KB, patch)
2016-12-22 18:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16914: Remove unused empty_lines.inc file (763 bytes, patch)
2016-12-22 18:22 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Hugo Agud 2016-07-13 12:48:17 UTC
We have discovered thant when you try to export in csv (koha 3.22.7) in translated interface (es-ES and ca-ES) all items are located in just one file

This is because a problem with a file itemsearch_item.csv that it doesn't include the order to break line for each item (at least is how we have fixed)

For reproduce the error
1. Go to staff 
2. Go to advance search -- item search
3. do whatever search and select csv to export
4. open with any textedior

sudo diff /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/es-ES/includes/catalogue/itemsearch_item.csv.inc /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/es-ES/includes/catalogue/itemsearch_item.csv.inc-original

< [%- USE Branches -%]
< [%- USE Koha -%]
< [%- biblio = item.biblio -%]
< [%- biblioitem = item.biblioitem -%]
< "[% biblio.title |html %] [% IF ( Koha.Preference( 'marcflavour' ) == 'UNIMARC' && biblio.author ) %]by [% END %][% biblio.author |html %]", "[% (biblioitem.publicationyear || biblio.copyrightdate) |html %]", "[% biblioitem.publishercode |html %]", "[% biblioitem.collectiontitle |html %]", "[% item.barcode |html %]", "[% item.itemcallnumber |html %]", "[% Branches.GetName(item.homebranch) |html %]", "[% Branches.GetName(item.holdingbranch) |html %]", "[% item.location |html %]", "[% item.stocknumber |html %]", "[% item.status |html %]", "[% (item.issues || 0) |html %]"[% INCLUDE 'empty_line.inc' %]
---
> [%- USE Branches -%] [%- USE Koha -%] [%- biblio = item.biblio -%] [%- biblioitem = item.biblioitem -%] "[% biblio.title |html %] [% IF ( Koha.Preference( 'marcflavour' ) == 'UNIMARC' && biblio.author ) %]por [% END %][% biblio.author |html %]", "[% (biblioitem.publicationyear || biblio.copyrightdate) |html %]", "[% biblioitem.publishercode |html %]", "[% biblioitem.collectiontitle |html %]", "[% item.barcode |html %]", "[% item.itemcallnumber |html %]", "[% Branches.GetName(item.homebranch) |html %]", "[% Branches.GetName(item.holdingbranch) |html %]", "[% item.location |html %]", "[% item.stocknumber |html %]", "[% item.status |html %]", "[% (item.issues || 0) |html %]"[% INCLUDE 'empty_line.inc' %] 
\ No newline at end of file
Comment 1 Josef Moravec 2016-11-03 09:07:32 UTC
Created attachment 57120 [details] [review]
Bug 16914: Insert end of line in CSV export of item search in translated templates

Test plan:
0) Do not apply the patch
1) Install any translation
2) Go to item search and choose Output to be CSV
3) Search for at least 2 items in result set, open CSV in text editor
    -> in english, everything is ok
    -> in other languages are all data on one line
4) Apply the patch
5) Repeat steps 2 and 3, CSV should look OK either in english and other
languages
Comment 2 Josef Moravec 2016-11-03 09:08:48 UTC
I 've tried a quick patch
Comment 3 Fridolin SOMERS 2016-11-09 12:57:27 UTC
This is a bug, not enhancement.
I also found it.
Comment 4 Fridolin SOMERS 2016-11-09 13:26:06 UTC
Bug 14264 managed export of late orders
Comment 5 Fridolin SOMERS 2016-11-09 13:37:39 UTC
(In reply to Josef Moravec from comment #1)
> Created attachment 57120 [details] [review] [review]
> Bug 16914: Insert end of line in CSV export of item search in translated
> templates
> 
> Test plan:
> 0) Do not apply the patch
> 1) Install any translation
> 2) Go to item search and choose Output to be CSV
> 3) Search for at least 2 items in result set, open CSV in text editor
>     -> in english, everything is ok
>     -> in other languages are all data on one line
> 4) Apply the patch
> 5) Repeat steps 2 and 3, CSV should look OK either in english and other
> languages

Nice try but this does not correct the bug.
It only works because when you change the "en" file itemsearch_item.csv.inc, it will not be translated.
I've tryed to add translation update and install after 4) and the bug is still there.

I think problem comes from the empty_line.inc :
wc -l koha-tmpl/intranet-tmpl/prog/en/includes/empty_line.inc
0 koha-tmpl/intranet-tmpl/prog/en/includes/empty_line.inc
It is really empty, it should contain a line feed.
Comment 6 Fridolin SOMERS 2016-11-09 13:39:06 UTC
Created attachment 57388 [details] [review]
Bug 16914 - Export csv in item search, exports all items in one line - alternate

Translation sets all TT vars on one line.
That is why empty_line.inc whas introduced.
But this file does not contain a line feed, it is realy empty.

This patch adds a line feed in empty_line.inc.

Test plan:
0) Do not apply the patch
1) Install any translation
2) Go to item search and choose Output to be CSV
3) Search for at least 2 items in result set, open CSV in text editor
    -> in english, everything is ok
    -> in other languages are all data on one line
4) Apply the patch
5) Repeat steps 1, 2 and 3, CSV should look OK either in english and other
languages
6) Check it does not break Bug 14264 correction
Comment 7 Fridolin SOMERS 2016-11-09 13:44:10 UTC
Here is my alternate patch.
Note to run also Bug 14264 test plan.

To view the file empty_line.inc, do not use VIM, it will show no difference between empty and only a line feed.
Use word count : 
wc -l koha-tmpl/intranet-tmpl/prog/en/includes/empty_line.inc
1 koha-tmpl/intranet-tmpl/prog/en/includes/empty_line.inc

@Josef Moravec :
If you could test this it would be great.
Comment 8 Josef Moravec 2016-11-14 20:03:08 UTC
I tried to test: this approach works for me, you were right... but I have problem with git-bz - the downloaded and applied patch doesn't make the desired change... 

What do you think about the solution similar to ICS export in OPAC - see bug 17322?
Comment 9 Katrin Fischer 2016-12-08 07:34:29 UTC
This is a bad bug, the export is an important feature of the item search.
Comment 10 M. Tompsett 2016-12-20 17:21:58 UTC
I attempted to apply this on the kohadevbox and it didn't change the file size, even though this is supposed to add a line feed at least.
Comment 11 Katrin Fischer 2016-12-22 12:17:52 UTC
Git applies the patch, so there should be an existing change. But it doesn't fix the problem - maybe we need to apply this with some special setting?
Comment 12 Katrin Fischer 2016-12-22 12:43:36 UTC
Created attachment 58380 [details] [review]
Bug 16914 - Export csv in item search, exports all items in  one line - alternate-alternate

Test plan:
0) Do not apply the patch
1) Install any translation
2) Go to item search and choose Output to be CSV
3) Search for at least 2 items in result set, open CSV in text editor
    -> in english, everything is ok
    -> in other languages are all data on one line
4) Apply the patch
5) Repeat steps 1, 2 and 3, CSV should look OK either in english and other
languages
6) Check it does not break Bug 14264 correction
Comment 13 Katrin Fischer 2016-12-22 12:44:26 UTC
With this patch German and English exporting of late orders AND item search results appears to be ok. (see also bug 17795)
Comment 14 Tomás Cohen Arazi 2016-12-22 15:55:44 UTC
Created attachment 58396 [details] [review]
Bug 16914: Rely on TT for newlines

The translation scripts have an historical tendency to chomp newlines
and we introduced an empty_line.inc file to force newlines when building
CSV output out of our templates (in item search and late orders).

This patch makes the mentioned templates use TT ability to force newlines
plus some misuses of the 'minus' sign.

Test plan:
- Apply the patch
- Do an item search that returns more than one result
- Export as CSV
=> SUCCESS: The CSV file is correctly formed.
- Install any translation:
  $ sudo koha-shell kohadev
 k$ cd kohaclone/misc/translator
 k$ perl translate install <chosen language>
- Enable <chosen language> (e.g. es-ES)
- Repeat the item search
- Export as CSV
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Have more than one late orders (bummer)
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed.
- Switch language
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2016-12-22 15:55:51 UTC
Created attachment 58397 [details] [review]
Bug 16914: Remove unused empty_lines.inc file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 M. Tompsett 2016-12-22 17:04:43 UTC
While the concept is good. I'm suspect of the missing -'s and added +'s in all the right spots. And my kohadevbox hates me, so testing is going horribly.
Comment 17 M. Tompsett 2016-12-22 17:23:34 UTC
(In reply to M. Tompsett from comment #16)
> While the concept is good. I'm suspect of the missing -'s and added +'s in
> all the right spots. And my kohadevbox hates me, so testing is going
> horribly.

I couldn't get my kohadevbox nor my non-kohadevbox to output fr-FR search results on a new line in the CSV.
Comment 18 M. Tompsett 2016-12-22 17:28:51 UTC
(In reply to M. Tompsett from comment #17)
> (In reply to M. Tompsett from comment #16)
> > While the concept is good. I'm suspect of the missing -'s and added +'s in
> > all the right spots. And my kohadevbox hates me, so testing is going
> > horribly.
> 
> I couldn't get my kohadevbox nor my non-kohadevbox to output fr-FR search
> results on a new line in the CSV.

DOH! Translation issues!
Comment 19 Katrin Fischer 2016-12-22 18:18:08 UTC
Created attachment 58400 [details] [review]
Bug 16914: Rely on TT for newlines

The translation scripts have an historical tendency to chomp newlines
and we introduced an empty_line.inc file to force newlines when building
CSV output out of our templates (in item search and late orders).

This patch makes the mentioned templates use TT ability to force newlines
plus some misuses of the 'minus' sign.

Test plan:
- Apply the patch
- Do an item search that returns more than one result
- Export as CSV
=> SUCCESS: The CSV file is correctly formed.
- Install any translation:
  $ sudo koha-shell kohadev
 k$ cd kohaclone/misc/translator
 k$ perl translate install <chosen language>
- Enable <chosen language> (e.g. es-ES)
- Repeat the item search
- Export as CSV
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Have more than one late orders (bummer)
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed.
- Switch language
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2016-12-22 18:20:15 UTC
Created attachment 58401 [details] [review]
Bug 16914: Remove unused empty_lines.inc file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Nick Clemens 2016-12-22 18:22:02 UTC
Created attachment 58402 [details] [review]
Bug 16914: Rely on TT for newlines

The translation scripts have an historical tendency to chomp newlines
and we introduced an empty_line.inc file to force newlines when building
CSV output out of our templates (in item search and late orders).

This patch makes the mentioned templates use TT ability to force newlines
plus some misuses of the 'minus' sign.

Test plan:
- Apply the patch
- Do an item search that returns more than one result
- Export as CSV
=> SUCCESS: The CSV file is correctly formed.
- Install any translation:
  $ sudo koha-shell kohadev
 k$ cd kohaclone/misc/translator
 k$ perl translate install <chosen language>
- Enable <chosen language> (e.g. es-ES)
- Repeat the item search
- Export as CSV
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Have more than one late orders (bummer)
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed.
- Switch language
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens 2016-12-22 18:22:10 UTC
Created attachment 58403 [details] [review]
Bug 16914: Remove unused empty_lines.inc file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Brendan Gallagher 2016-12-22 20:28:17 UTC
Pushed to Master - Should be in the 17.05 release - Thanks!
Comment 24 Katrin Fischer 2016-12-22 21:38:57 UTC
These patches have been pushed to 16.11.x, will be in 16.11.01.
Comment 25 Julian Maurice 2017-01-02 12:21:56 UTC
Pushed to 3.22.x for 3.22.14
Comment 26 Katrin Fischer 2017-01-04 10:12:14 UTC
*** Bug 17795 has been marked as a duplicate of this bug. ***
Comment 27 Mason James 2017-01-24 23:35:03 UTC
Pushed to 16.05.x, for 16.05.06 release
Comment 28 Katrin Fischer 2017-03-24 12:55:28 UTC
It appears that if you actually translate the file by having a translated version of the string 

%s %s %s %s "%s %svon %s%s", "%s", "%s", "%s", "%s", "%s", "%s", "%s", "%s", "%s", "%s", "%s" 

then the csv export is broken again because the line breaks in itemsearch_item.csv.inc get destroyed.
Comment 29 Katrin Fischer 2017-03-24 13:03:18 UTC
Easiest way to test: Install German, look for the string (search for the file name), mark it as 'fuzzy' - Export works, remove 'fuzzy' - export is broken.
Comment 30 Katrin Fischer 2017-03-24 13:27:53 UTC
Filed bug 18331 for this.