Bug 28327 - System preference CSVdelimiter special case for tabulation
Summary: System preference CSVdelimiter special case for tabulation
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 28363
  Show dependency treegraph
 
Reported: 2021-05-12 09:43 UTC by Fridolin Somers
Modified: 2021-12-01 14:55 UTC (History)
2 users (show)

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


Attachments
Bug 28327: System preference CSVdelimiter special case for tabulation (8.17 KB, patch)
2021-05-12 10:12 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter (2.22 KB, patch)
2021-05-12 10:38 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: System preference CSVdelimiter special case for tabulation (8.30 KB, patch)
2021-05-12 13:17 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter (2.28 KB, patch)
2021-05-12 13:17 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28327: (QA follow-up) Add POD (1.06 KB, patch)
2021-05-12 13:17 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28327: System preference CSVdelimiter special case for tabulation in templates (8.19 KB, patch)
2021-05-12 13:39 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter (2.22 KB, patch)
2021-05-12 13:39 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: System preference CSVdelimiter special case for tabulation in perl (3.15 KB, patch)
2021-05-12 13:40 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: use semicolon when preference CSVDelimiter is missing (8.75 KB, patch)
2021-05-12 13:40 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: (QA follow-up) Add POD (1.06 KB, patch)
2021-05-12 13:40 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28135: Replace use of input type number in additem.js (2.64 KB, patch)
2021-05-12 13:43 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: CSVdelimiter special case for tabulation in templates (8.17 KB, patch)
2021-05-12 13:45 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter (2.22 KB, patch)
2021-05-12 13:45 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: CSVdelimiter special case for tabulation in perl (3.13 KB, patch)
2021-05-12 13:46 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: use semicolon when preference CSVDelimiter is missing (8.75 KB, patch)
2021-05-12 13:46 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: (QA follow-up) Add POD (1.06 KB, patch)
2021-05-12 13:46 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: CSVdelimiter special case for tabulation in templates (8.17 KB, patch)
2021-10-21 22:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter (2.22 KB, patch)
2021-10-21 22:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: CSVdelimiter special case for tabulation in perl (3.14 KB, patch)
2021-10-21 22:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: (QA follow-up) Add POD (1.06 KB, patch)
2021-10-21 22:10 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28327: CSVdelimiter special case for tabulation in templates (8.23 KB, patch)
2021-10-27 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter (2.28 KB, patch)
2021-10-27 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28327: CSVdelimiter special case for tabulation in perl (3.19 KB, patch)
2021-10-27 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28327: (QA follow-up) Add POD (1.12 KB, patch)
2021-10-27 11:12 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 Fridolin Somers 2021-05-12 09:43:11 UTC
System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

Must be fixed when preference is fetch in template.
Comment 1 Fridolin Somers 2021-05-12 10:12:37 UTC
Created attachment 120859 [details] [review]
Bug 28327: System preference CSVdelimiter special case for tabulation

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in template.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 2 Fridolin Somers 2021-05-12 10:38:41 UTC
Created attachment 120861 [details] [review]
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter

Run prove t/Koha_Template_Plugin_Koha.t
Comment 3 Nick Clemens 2021-05-12 11:01:28 UTC
(In reply to Fridolin Somers from comment #2)
> Created attachment 120861 [details] [review] [review]
> Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter
> 
> Run prove t/Koha_Template_Plugin_Koha.t

Why do you add the 'val' passthrough to the routine?
Comment 4 Fridolin Somers 2021-05-12 12:25:24 UTC
(In reply to Nick Clemens from comment #3)
> (In reply to Fridolin Somers from comment #2)
> > Created attachment 120861 [details] [review] [review] [review]
> > Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter
> > 
> > Run prove t/Koha_Template_Plugin_Koha.t
> 
> Why do you add the 'val' passthrough to the routine?

In case we use it with an existing value from DB.
For example in CSV profile we store delimiter (not possible to store \t in DB I  bet).
So we may need to call this method in the future with a separator value, just to transform 'tabulation' into '\t'.
Comment 5 Nick Clemens 2021-05-12 13:17:06 UTC
Created attachment 120867 [details] [review]
Bug 28327: System preference CSVdelimiter special case for tabulation

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in template.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens 2021-05-12 13:17:09 UTC
Created attachment 120868 [details] [review]
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter

Run prove t/Koha_Template_Plugin_Koha.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens 2021-05-12 13:17:13 UTC
Created attachment 120869 [details] [review]
Bug 28327: (QA follow-up) Add POD
Comment 8 Nick Clemens 2021-05-12 13:19:05 UTC
This bug is ugly, moving to PQA as very trivial, thanks Frido!
Comment 9 Fridolin Somers 2021-05-12 13:24:24 UTC
Wait I have more patches ;)
Comment 10 Fridolin Somers 2021-05-12 13:39:33 UTC
Created attachment 120870 [details] [review]
Bug 28327: System preference CSVdelimiter special case for tabulation in templates

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in template.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 11 Fridolin Somers 2021-05-12 13:39:50 UTC
Created attachment 120871 [details] [review]
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter

Run prove t/Koha_Template_Plugin_Koha.t
Comment 12 Fridolin Somers 2021-05-12 13:40:08 UTC
Created attachment 120872 [details] [review]
Bug 28327: System preference CSVdelimiter special case for tabulation in perl

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in perl.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 13 Fridolin Somers 2021-05-12 13:40:26 UTC
Created attachment 120873 [details] [review]
Bug 28327: use semicolon when preference CSVDelimiter is missing

Some places use comma as value when preference CSVDelimiter is missing
or empty.
From installer/data/mysql/mandatory/sysprefs.sql whe see that defaut
install value is semicolon.

This patch sets semicolon when preference CSVDelimiter is missing.

"We are perl, we love semicolon ;)"
Comment 14 Fridolin Somers 2021-05-12 13:40:42 UTC
Created attachment 120874 [details] [review]
Bug 28327: (QA follow-up) Add POD
Comment 15 Fridolin Somers 2021-05-12 13:43:46 UTC Comment hidden (obsolete)
Comment 16 Fridolin Somers 2021-05-12 13:45:26 UTC
Created attachment 120877 [details] [review]
Bug 28327: CSVdelimiter special case for tabulation in templates

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in template.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 17 Fridolin Somers 2021-05-12 13:45:45 UTC
Created attachment 120878 [details] [review]
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter

Run prove t/Koha_Template_Plugin_Koha.t
Comment 18 Fridolin Somers 2021-05-12 13:46:01 UTC
Created attachment 120879 [details] [review]
Bug 28327: CSVdelimiter special case for tabulation in perl

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in perl.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 19 Fridolin Somers 2021-05-12 13:46:17 UTC
Created attachment 120880 [details] [review]
Bug 28327: use semicolon when preference CSVDelimiter is missing

Some places use comma as value when preference CSVDelimiter is missing
or empty.
From installer/data/mysql/mandatory/sysprefs.sql whe see that defaut
install value is semicolon.

This patch sets semicolon when preference CSVDelimiter is missing.

"We are perl, we love semicolon ;)"
Comment 20 Fridolin Somers 2021-05-12 13:46:33 UTC
Created attachment 120881 [details] [review]
Bug 28327: (QA follow-up) Add POD
Comment 21 Fridolin Somers 2021-05-12 13:48:41 UTC
(In reply to Fridolin Somers from comment #9)
> Wait I have more patches ;)

Here they are.

Tell me if your OK with semicolon ';' as default value since it is value in installer/data/mysql/mandatory/sysprefs.sql
Comment 22 Nick Clemens 2021-05-12 14:17:35 UTC
(In reply to Fridolin Somers from comment #21)
> (In reply to Fridolin Somers from comment #9)
> > Wait I have more patches ;)
> 
> Here they are.
> 
> Tell me if your OK with semicolon ';' as default value since it is value in
> installer/data/mysql/mandatory/sysprefs.sql

If a library has the pref not set, this will change behavior - I would move that to its own bug
Comment 23 Fridolin Somers 2021-05-17 10:32:32 UTC
(In reply to Nick Clemens from comment #22)
> (In reply to Fridolin Somers from comment #21)
> > (In reply to Fridolin Somers from comment #9)
> > > Wait I have more patches ;)
> > 
> > Here they are.
> > 
> > Tell me if your OK with semicolon ';' as default value since it is value in
> > installer/data/mysql/mandatory/sysprefs.sql
> 
> If a library has the pref not set, this will change behavior - I would move
> that to its own bug

OK i'm on it
Comment 24 Fridolin Somers 2021-10-21 22:08:59 UTC
Created attachment 126682 [details] [review]
Bug 28327: CSVdelimiter special case for tabulation in templates

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in template.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 25 Fridolin Somers 2021-10-21 22:09:24 UTC
Created attachment 126683 [details] [review]
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter

Run prove t/Koha_Template_Plugin_Koha.t
Comment 26 Fridolin Somers 2021-10-21 22:09:47 UTC
Created attachment 126684 [details] [review]
Bug 28327: CSVdelimiter special case for tabulation in perl

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in perl.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'
Comment 27 Fridolin Somers 2021-10-21 22:10:05 UTC
Created attachment 126685 [details] [review]
Bug 28327: (QA follow-up) Add POD
Comment 28 Fridolin Somers 2021-10-21 22:11:10 UTC
Fixed a typo in "CSVdelimiter special case for tabulation in perl" and rebase on current master
Comment 29 Nick Clemens 2021-10-27 11:12:39 UTC
Created attachment 126964 [details] [review]
Bug 28327: CSVdelimiter special case for tabulation in templates

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in template.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 30 Nick Clemens 2021-10-27 11:12:43 UTC
Created attachment 126965 [details] [review]
Bug 28327: Add unit tests for Koha::Template::Plugin::Koha::CSVDelimiter

Run prove t/Koha_Template_Plugin_Koha.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 31 Nick Clemens 2021-10-27 11:12:46 UTC
Created attachment 126966 [details] [review]
Bug 28327: CSVdelimiter special case for tabulation in perl

System preference 'CSVdelimiter' has a special case for tabulation.
Preference value contains string 'tabulation' but string '\t' must be used in CSV file.

This is OK in many places, for exemple Bug 17590.

This patch fixed when preference is fetch in perl.

Test plan :
1) Set system preference 'CSVdelimiter' = 'tabs'.
2) Create CSV export in impacted pages
3) Check columns are separated by tabulation character and not string 'tabulation'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 32 Nick Clemens 2021-10-27 11:12:49 UTC
Created attachment 126967 [details] [review]
Bug 28327: (QA follow-up) Add POD

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 33 Jonathan Druart 2021-12-01 14:55:20 UTC
It would have been better to have a new method in Koha.Config that could be called from all the other places as well:

admin/aqplan.pl:$del = "\t" if $del eq 'tabulation';
misc/cronjobs/overdue_notices.pl:    $sep_char = "\t" if ($sep_char eq 'tabulation');
misc/cronjobs/overdue_notices.pl:            $delimiter = "\t" if $delimiter eq 'tabulation';
misc/export_borrowers.pl:    $separator = "\t" if ($separator eq 'tabulation');
reports/acquisitions_stats.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/bor_issues_top.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/borrowers_out.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/borrowers_stats.pl:$sep = "\t" if ($sep and $sep eq 'tabulation');
reports/cash_register_stats.pl:        $delimiter = "\t" if $delimiter eq 'tabulation';
reports/cat_issues_top.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/catalogue_stats.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/guided_reports.pl:                $delimiter = "\t" if $delimiter eq 'tabulation';
reports/issues_avg_stats.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/issues_stats.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/orders_by_fund.pl:        $sep = "\t" if ($sep eq 'tabulation');
reports/reserves_stats.pl:$sep = "\t" if ($sep eq 'tabulation');
reports/serials_stats.pl:$sep = "\t" if ($sep eq 'tabulation');
t/Koha_Template_Plugin_Koha.t:    is($plugin->CSVDelimiter(), "\t", "CSVDelimiter() returns \\t when preference is tabulation");
t/Koha_Template_Plugin_Koha.t:    is($plugin->CSVDelimiter('tabulation'), "\t", "CSVDelimiter(arg) returns \\t value when arg is tabulation");
tools/viewlog.pl:            $delimiter = "\t" if $delimiter eq 'tabulation';


With this patch we are adding yet one more occurrences...