Bug 19910 - Download report as 'Comma separated' is misleading
Summary: Download report as 'Comma separated' is misleading
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 18497
Blocks:
  Show dependency treegraph
 
Reported: 2018-01-03 14:07 UTC by Nick Clemens
Modified: 2018-06-07 08:21 UTC (History)
5 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 19910 - Use the delimiter syspref to name download link (2.75 KB, patch)
2018-01-03 14:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19910 - Use the delimiter syspref to name download link (2.80 KB, patch)
2018-01-17 20:24 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19910 - Use the delimiter syspref to name download link (3.01 KB, patch)
2018-01-26 20:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19910 - Use the delimiter syspref to name download link (3.06 KB, patch)
2018-03-30 15:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19910 - Use the delimiter syspref to name download link (3.12 KB, patch)
2018-04-01 15:55 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 19910 - Use the delimiter syspref to name download link (3.18 KB, patch)
2018-04-03 21:26 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19910: Use span to make translators happy (1.40 KB, patch)
2018-04-04 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-01-03 14:07:00 UTC
The system preference 'delimiter' is actually used for determining how the report will be formatted, we should name it accordingly
Comment 1 Nick Clemens 2018-01-03 14:09:19 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2018-01-17 20:24:14 UTC Comment hidden (obsolete)
Comment 3 Victor Grousset/tuxayo 2018-01-22 15:57:48 UTC
Is it possible that the lack of mention of CSV could confuse people who don't know what the text forms of CSV look like?

If so, should we write «CSV (Semicolon separated)» instead of «Semicolon separated text» ?
Comment 4 Victor Grousset/tuxayo 2018-01-22 16:15:28 UTC
(In reply to Victor Grousset/tuxayo from comment #3)

Not writing at least CSV might be right about the theoretical meaning of CSV. But if we only write "semicolon separated values" it might be possible that enough users don't realize that it's the same thing that I usually called CSV. (Opinion of someone having a better perceptive about this is needed)

Also: isn't the extension still .csv anyway? So the OSes will try to open the file with a spreadsheet software.
Comment 5 Victor Grousset/tuxayo 2018-01-26 13:01:33 UTC
Sorry, no enough proofreading. Here is a fixed version of my last message. Minor changes but at least it doesn't burn the eyes anymore.

Not writing CSV might be right about the theoretical meaning of CSV.
But if we only write "semicolon separated values" it might be possible that enough users don't realize that it's the same thing that is usually called CSV. (Opinion of someone having a better perceptive about this is needed)

Also: isn't the extension still .csv anyway? So the OSes will try to open the file with a spreadsheet software.
Comment 6 Nick Clemens 2018-01-26 15:42:42 UTC
Hmmm..I agree the file extension may be confusing - would you think it is better to say:

XXXX Separated text (.csv file)

that makes more sense to me loglically

and/or we could use .txt format unless generating a csv
Comment 7 Victor Grousset/tuxayo 2018-01-26 16:04:46 UTC
(In reply to Nick Clemens from comment #6)
> Hmmm..I agree the file extension may be confusing - would you think it is
> better to say:
> 
> XXXX Separated text (.csv file)
> 
> that makes more sense to me logically

+1 for using csv and the actual separator.
After a quick web search I'm not sure "XXXX Separated text" is used elsewhere.
But saying "text" in nice to convey that it's essentially text (formatted, but still text)

> and/or we could use .txt format unless generating a csv

Then it would be opened by a text editor instead of a spreadsheet software right?
Well when the file is saved and opened by the file manager. Else (Open instead of save) the mime type transferred by Koha should make it open in the spreadsheet software. (assuming Koha says "mime type csv" or something like that)
Comment 8 Nick Clemens 2018-01-26 20:31:39 UTC
Created attachment 70977 [details] [review]
Bug 19910 - Use the delimiter syspref to name download link

To test:
1 - Set delimiter syspref to anything but comma
2 - Donwload report results as comma separated text
3 - They actually follow the syspref
4 - Apply patch
5 - Download link should now match pref selection
6 - Change pref, note link changes
7 - Verify things still work as expected
Comment 9 Nick Clemens 2018-01-26 20:33:22 UTC
updated text to include (.csv) and moved to an include so can be used elsewhere
Comment 10 Katrin Fischer 2018-01-29 20:51:08 UTC
Comment on attachment 70977 [details] [review]
Bug 19910 - Use the delimiter syspref to name download link

Review of attachment 70977 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/includes/delimiter_text.inc
@@ +3,5 @@
> +    [% CASE ',' %]Comma separated text (.csv)
> +    [% CASE '/' %]Slash separated text (.csv)
> +    [% CASE ';' %]Semicolon separated text (.csv)
> +    [% CASE '\\' %]Backslash separated text (.csv)
> +    [% CASE 'tabulation' %]Tab separated text (.csv)

Please check if these are translatable - you might have to wrap them in spans to get them picked up.
Comment 11 Owen Leonard 2018-01-31 14:47:44 UTC
(In reply to Katrin Fischer from comment #10)
> Please check if these are translatable - you might have to wrap them in
> spans to get them picked up.

This is what it looks like in po/en-GB-staff-prog.po after applying the patch and updating the translation:

#. %1$s:  SWITCH Koha.Preference('delimiter') 
#. %2$s:  CASE '#' 
#. %3$s:  CASE ',' 
#. %4$s:  CASE '/' 
#. %5$s:  CASE ';' 
#. %6$s:  CASE '\\' 
#. %7$s:  CASE 'tabulation' 
#. %8$s:  END 
#: intranet-tmpl/prog/en/includes/delimiter_text.inc:1
#, c-format
msgid ""
"%s %sPound (#) separated text (.csv) %sComma separated text (.csv) %sSlash "
"separated text (.csv) %sSemicolon separated text (.csv) %sBackslash "
"separated text (.csv) %sTab separated text (.csv) %s "
msgstr ""
Comment 12 Katrin Fischer 2018-01-31 16:22:38 UTC
Nice, we are good then. Thx, Owen!
Comment 13 Victor Grousset/tuxayo 2018-02-01 10:35:05 UTC
Wrong bug, sorry >_<
Comment 14 Victor Grousset/tuxayo 2018-02-09 17:15:05 UTC
@Nick: Does the last discussion in bug 19928 affects this one?

See from https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19928#c29
Comment 15 Nick Clemens 2018-02-12 20:31:30 UTC
(In reply to Victor Grousset/tuxayo from comment #14)
> @Nick: Does the last discussion in bug 19928 affects this one?
> 
> See from https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19928#c29

I suppose it does if the buttons here seem too long too. 

Let me describe the issue here though:
1 - Staff member without access to system preferences uses a report to get some data to load back into koha (or another system)
2 - The click 'Download as CSV' and get a *.csv file
3 - They assume the file is actually comma separated
4 - They try to load the file into Koha (or another system) which wants actual commas in a csv
5 - Things don't work
6 - They don't know why
7 - :-(

Now, if you open the file in libreoffice/excel/etc. you can fix the delimiters, but sometimes you lose leading zeros etc. And it is an extra step. I just want to make sure someone who can't see the pref understands what they are getting.

It could be a tooltip and not on the button
It could be a note under the button
We could provide selection of delimiter at export and use the pref only to determine default.

What would folks prefer?
Comment 16 Katrin Fischer 2018-02-12 20:46:39 UTC
>It could be a tooltip and not on the button
>It could be a note under the button
>We could provide selection of delimiter at export and use the pref only to >determine default.

I think of those suggestions none work really well with a menu type button like the one in the toolbar in reports. Am I missing something?
Comment 17 Katrin Fischer 2018-02-12 20:56:42 UTC
Thinking about it again... as this only changes the options in the menu, the button remains the same nice length. So this should be ok?
Comment 18 Victor Grousset/tuxayo 2018-03-05 13:19:11 UTC
So no need to change anything functionally?
Comment 19 Katrin Fischer 2018-03-30 14:06:01 UTC
Let's move this bug along.
Comment 20 Owen Leonard 2018-03-30 15:53:08 UTC
Created attachment 73494 [details] [review]
Bug 19910 - Use the delimiter syspref to name download link

To test:
1 - Set delimiter syspref to anything but comma
2 - Donwload report results as comma separated text
3 - They actually follow the syspref
4 - Apply patch
5 - Download link should now match pref selection
6 - Change pref, note link changes
7 - Verify things still work as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 21 Victor Grousset/tuxayo 2018-04-01 15:55:55 UTC
Created attachment 73518 [details] [review]
Bug 19910 - Use the delimiter syspref to name download link

To test:
1 - Set delimiter syspref to anything but comma
2 - Download report results as comma separated text
3 - They actually follow the syspref
4 - Apply patch
5 - Download link should now match pref selection
6 - Change pref, note link changes
7 - Verify things still work as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>
Comment 22 Katrin Fischer 2018-04-03 21:26:55 UTC
Created attachment 73592 [details] [review]
Bug 19910 - Use the delimiter syspref to name download link

To test:
1 - Set delimiter syspref to anything but comma
2 - Download report results as comma separated text
3 - They actually follow the syspref
4 - Apply patch
5 - Download link should now match pref selection
6 - Change pref, note link changes
7 - Verify things still work as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Jonathan Druart 2018-04-04 18:55:46 UTC
Created attachment 73658 [details] [review]
Bug 19910: Use span to make translators happy
Comment 24 Jonathan Druart 2018-04-04 19:07:54 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 25 Nick Clemens 2018-05-14 21:15:02 UTC
Pushed to stable for 17.11.06

Awesome work all!
Comment 26 Fridolin SOMERS 2018-06-07 08:21:35 UTC
I choose not to backport to 17.05.x, to focus on major and +