Bug 28327: Unify CSV delimiter special behavior 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 adds C4::Context->csv_delimiter to add a uniq metod dealing
with this behavior.
Also create Koha::Template::Plugin::Koha->CSVDelimiter for calls from
Toolkit Templates.

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'
4) Check with another delimiter

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/C4/Context.pm b/C4/Context.pm
index 8cb9bde..876126b 100644 (file)
--- a/C4/Context.pm
+++ b/C4/Context.pm
@@ -479,6 +479,26 @@ sub delete_preference {
     return 0;
 }
 
+=head2 csv_delimiter
+
+    $delimiter = C4::Context->csv_delimiter;
+
+    Returns prefered CSV delimiter, using system preference 'CSVDelimiter'.
+    If this preference is missing or empty semicolon will be returned.
+    This method is needed because of special behavior for tabulation.
+
+    You can, optionally, pass a value parameter to this routine
+    in the case of existing delimiter.
+
+=cut
+
+sub csv_delimiter {
+    my ( $self, $value ) = @_;
+    my $delimiter = $value || $self->preference('CSVDelimiter') || ';';
+    $delimiter = "\t" if $delimiter eq 'tabulation';
+    return $delimiter;
+}
+
 =head2 Zconn
 
   $Zconn = C4::Context->Zconn

diff --git a/Koha/Template/Plugin/Koha.pm b/Koha/Template/Plugin/Koha.pm
index 5ed58a9..9ecd7fd 100644 (file)
--- a/Koha/Template/Plugin/Koha.pm
+++ b/Koha/Template/Plugin/Koha.pm
@@ -55,6 +55,22 @@ sub Preference {
     return C4::Context->preference( $pref );
 }
 
+=head3 CSVDelimiter
+
+The delimiter option 'tabs' is stored in the DB as 'tabulation' to avoid issues
+storing special characters in the DB. This helper function translates the value
+to the correct character when used in templates.
+
+You can, optionally, pass a value parameter to this routine in the case of delimiter
+being fetched in the scripts and still needing to be translated
+
+=cut
+
+sub CSVDelimiter {
+    my ( $self, $val ) = @_;
+    return C4::Context->csv_delimiter($val);
+}
+
 sub Version {
     my $version_string = Koha::version();
     my ( $major, $minor, $maintenance, $development ) = split( '\.', $version_string );

diff --git a/admin/aqplan.pl b/admin/aqplan.pl
index 64b6cd0..cbded1c 100755 (executable)
--- a/admin/aqplan.pl
+++ b/admin/aqplan.pl
@@ -97,7 +97,7 @@ my $show_actual  = $input->param('show_actual');
 my $show_percent = $input->param('show_percent');
 my $output       = $input->param("output") // q{};
 our $basename     = $input->param("basename");
-our $del          = $input->param("sep");
+our $del          = C4::Context->csv_delimiter(scalar $input->param("sep"));
 
 my $show_mine       = $input->param('show_mine') ;
 
@@ -307,7 +307,7 @@ foreach my $n (@names) {
 #         DEFAULT DISPLAY BEGINS
 
 my $CGIextChoice = ( 'CSV' ); # FIXME translation
-my $CGIsepChoice = ( C4::Context->preference("CSVDelimiter") );
+my $CGIsepChoice = ( C4::Context->csv_delimiter );
 
 my ( @budget_lines, %cell_hash );
 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.csv.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.csv.inc
index 1a1ade5..ac91b9e 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.csv.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.csv.inc
@@ -6,7 +6,7 @@
 [%- USE AuthorisedValues -%]
 [%- SET biblio = item.biblio -%]
 [%- SET biblioitem = item.biblioitem -%]
-[%- SET delimiter = Koha.Preference( 'CSVDelimiter' ) || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 "[% biblio.title | replace('"', '""') | $raw %] [% IF ( Koha.Preference( 'marcflavour' ) == 'UNIMARC' && biblio.author ) %]by [% END %][% biblio.author | replace('"', '""') | $raw %]"
 [%- delimiter | $raw -%]
 "[% (biblioitem.publicationyear || biblio.copyrightdate) | replace('"', '""') | $raw %]"

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basket.tt
index 75d246c..bf40830 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basket.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basket.tt
@@ -1,4 +1,4 @@
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference( 'CSVDelimiter' ) || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 
 [%- BLOCK -%]Contract name[% delimiter | html %]Order number[% delimiter | html %]Entry date[% delimiter | html %]ISBN[% delimiter | html %]Author[% delimiter | html %]Title[% delimiter | html %]Publication year[% delimiter | html %]Publisher[% delimiter | html %]Collection title[% delimiter | html %]Note for vendor[% delimiter | html %]Quantity[% delimiter | html %]RRP[% delimiter | html %]Delivery place[% delimiter | html %]Billing place[%- END -%]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basketgroup.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basketgroup.tt
index 63cc1ce..be05cb9 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basketgroup.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/basketgroup.tt
@@ -1,4 +1,4 @@
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference( 'CSVDelimiter' ) || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 
 [%- BLOCK -%]Account number[% delimiter | html %]Basket name[% delimiter | html %]Order number[% delimiter | html %]Author[% delimiter | html %]Title[% delimiter | html %]Publisher[% delimiter | html %]Publication year[% delimiter | html %]Collection title[% delimiter | html %]ISBN[% delimiter | html %]Quantity[% delimiter | html %]RRP tax included[% delimiter | html %]RRP tax excluded[% delimiter | html %]Discount[% delimiter | html %]Estimated cost tax included[% delimiter | html %]Estimated cost tax excluded[% delimiter | html %]Note for vendor[% delimiter | html %]Entry date[% delimiter | html %]Bookseller name[% delimiter | html %]Bookseller physical address[% delimiter | html %]Bookseller postal address[% delimiter | html %]Contract number[% delimiter | html %]Contract name[% delimiter | html %]Basket group delivery place[% delimiter | html %]Basket group billing place[% delimiter | html %]Basket delivery place[% delimiter | html %]Basket billing place[%- END -%]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt
index 5bda385..bc02b07 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt
@@ -1,4 +1,4 @@
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference('CSVDelimiter') || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 
 [%- BLOCK -%]ORDER DATE[%- delimiter | html -%]ESTIMATED DELIVERY DATE[%- delimiter | html -%]VENDOR[%- delimiter | html -%]INFORMATION[%- delimiter | html -%]TOTAL COST[%- delimiter | html -%]BASKET[%- delimiter | html -%]CLAIMS COUNT[%- delimiter | html -%]CLAIMED DATE[%- delimiter | html -%]INTERNAL NOTE[%- delimiter | html -%]VENDOR NOTE[%- delimiter | html -%]ISBN[%- END -%]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/catalogue/itemsearch.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/catalogue/itemsearch.tt
index 631e7fd..be04f9d 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/catalogue/itemsearch.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/catalogue/itemsearch.tt
@@ -1,6 +1,6 @@
 [%- USE raw -%]
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference('CSVDelimiter') || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 [%- BLOCK -%]
 "Title"
 [%- delimiter | $raw -%]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basket.tt
index ebc67b9..9987cd3 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basket.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basket.tt
@@ -1,5 +1,5 @@
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference( 'CSVDelimiter' ) || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 
 [%- INCLUDE csv_headers/acqui/basket.tt -%]
 [%- INCLUDE empty_line.inc -%]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basketgroup.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basketgroup.tt
index c005cd6..84df7f7 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basketgroup.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/basketgroup.tt
@@ -1,5 +1,5 @@
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference( 'CSVDelimiter' ) || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 
 [%- USE Price -%]
 [%- INCLUDE csv_headers/acqui/basketgroup.tt -%]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/lateorders.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/lateorders.tt
index 7bef44e..dd753bd 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/lateorders.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/csv/lateorders.tt
@@ -1,5 +1,5 @@
 [%- USE Koha -%]
-[%- SET delimiter = Koha.Preference( 'CSVDelimiter' ) || ',' -%]
+[%- SET delimiter = Koha.CSVDelimiter() -%]
 
 [%- USE KohaDates -%]
 [%- INCLUDE csv_headers/acqui/lateorders.tt -%]

diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl
index 3444b23..9c13fd2 100755 (executable)
--- a/misc/cronjobs/overdue_notices.pl
+++ b/misc/cronjobs/overdue_notices.pl
@@ -407,8 +407,7 @@ binmode( STDOUT, ':encoding(UTF-8)' );
 our $csv;       # the Text::CSV_XS object
 our $csv_fh;    # the filehandle to the CSV file.
 if ( defined $csvfilename ) {
-    my $sep_char = C4::Context->preference('CSVDelimiter') || ';';
-    $sep_char = "\t" if ($sep_char eq 'tabulation');
+    my $sep_char = C4::Context->csv_delimiter;
     $csv = Text::CSV_XS->new( { binary => 1 , sep_char => $sep_char } );
     if ( $csvfilename eq '' ) {
         $csv_fh = *STDOUT;
@@ -837,7 +836,7 @@ END_SQL
         # Generate the content of the csv with headers
         my $content;
         if ( defined $csvfilename ) {
-            my $delimiter = C4::Context->preference('CSVDelimiter') || ';';
+            my $delimiter = C4::Context->csv_delimiter;
             $content = join($delimiter, qw(title name surname address1 address2 zipcode city country email itemcount itemsinfo due_date issue_date)) . "\n";
         }
         else {

diff --git a/misc/export_borrowers.pl b/misc/export_borrowers.pl
index d433a55..7cde608 100755 (executable)
--- a/misc/export_borrowers.pl
+++ b/misc/export_borrowers.pl
@@ -91,8 +91,7 @@ my $sth   = $dbh->prepare($query);
 $sth->execute;
 
 unless ( $separator ) {
-    $separator = C4::Context->preference('CSVDelimiter') || ',';
-    $separator = "\t" if ($separator eq 'tabulation');
+    $separator = C4::Context->csv_delimiter;
 }
 
 my $csv = Text::CSV->new( { sep_char => $separator, binary => 1 } );

diff --git a/reports/acquisitions_stats.pl b/reports/acquisitions_stats.pl
index 63d0152..2c90fe3 100755 (executable)
--- a/reports/acquisitions_stats.pl
+++ b/reports/acquisitions_stats.pl
@@ -60,8 +60,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
     }
 );
 
-our $sep     = $input->param("sep") // '';
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep = C4::Context->csv_delimiter(scalar $input->param("sep"));
 
 $template->param(
     do_it                    => $do_it,

diff --git a/reports/bor_issues_top.pl b/reports/bor_issues_top.pl
index e816150..439db23 100755 (executable)
--- a/reports/bor_issues_top.pl
+++ b/reports/bor_issues_top.pl
@@ -50,8 +50,7 @@ my ($template, $borrowernumber, $cookie)
                 type => "intranet",
                 flagsrequired => {reports => '*'},
                 });
-our $sep     = $input->param("sep") || C4::Context->preference('CSVDelimiter') || ',';
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep = C4::Context->csv_delimiter(scalar $input->param("sep"));
 $template->param(do_it => $do_it,
         );
 if ($do_it) {

diff --git a/reports/borrowers_out.pl b/reports/borrowers_out.pl
index bad3e76..4678a60 100755 (executable)
--- a/reports/borrowers_out.pl
+++ b/reports/borrowers_out.pl
@@ -46,8 +46,7 @@ my @filters = $input->multi_param("Filter");
 
 my $output = $input->param("output");
 my $basename = $input->param("basename");
-our $sep     = $input->param("sep") || '';
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep     = C4::Context->csv_delimiter(scalar $input->param("sep"));
 my ($template, $borrowernumber, $cookie)
     = get_template_and_user({template_name => $fullreportname,
                 query => $input,

diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl
index 2989e33..4e0f645 100755 (executable)
--- a/reports/borrowers_stats.pl
+++ b/reports/borrowers_stats.pl
@@ -53,8 +53,7 @@ my $borstat = $input->param("status");
 my $borstat1 = $input->param("activity");
 my $output = $input->param("output");
 my $basename = $input->param("basename");
-our $sep     = $input->param("sep");
-$sep = "\t" if ($sep and $sep eq 'tabulation');
+our $sep     = C4::Context->csv_delimiter(scalar $input->param("sep"));
 
 my ($template, $borrowernumber, $cookie)
        = get_template_and_user({template_name => $fullreportname,

diff --git a/reports/cash_register_stats.pl b/reports/cash_register_stats.pl
index 6bd869d..c2b22f9 100755 (executable)
--- a/reports/cash_register_stats.pl
+++ b/reports/cash_register_stats.pl
@@ -153,7 +153,7 @@ if ($do_it) {
         my $format = 'csv';
         my $reportname = $input->param('basename');
         my $reportfilename = $reportname ? "$reportname.$format" : "reportresults.$format" ;
-        my $delimiter = C4::Context->preference('CSVDelimiter') || ',';
+        my $delimiter = C4::Context->csv_delimiter;
             my @rows;
             foreach my $row (@loopresult) {
                 my @rowValues;

diff --git a/reports/cat_issues_top.pl b/reports/cat_issues_top.pl
index 87f1944..d3d1e06 100755 (executable)
--- a/reports/cat_issues_top.pl
+++ b/reports/cat_issues_top.pl
@@ -51,8 +51,7 @@ my ($template, $borrowernumber, $cookie)
                 type => "intranet",
                 flagsrequired => { reports => '*'},
                 });
-our $sep     = $input->param("sep");
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep     = C4::Context->csv_delimiter(scalar $input->param("sep"));
 $template->param(do_it => $do_it,
         );
 if ($do_it) {

diff --git a/reports/catalogue_stats.pl b/reports/catalogue_stats.pl
index c3795df..b4d72a3 100755 (executable)
--- a/reports/catalogue_stats.pl
+++ b/reports/catalogue_stats.pl
@@ -50,8 +50,7 @@ my @filters     = $input->multi_param("Filter");
 my $cotedigits  = $input->param("cotedigits");
 my $output      = $input->param("output");
 my $basename    = $input->param("basename");
-our $sep        = $input->param("sep");
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep        = C4::Context->csv_delimiter(scalar $input->param("sep"));
 my $item_itype;
 if(C4::Context->preference('item-level_itypes')) {
        $item_itype = "items\.itype"

diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl
index 48647c3..ab79b0f 100755 (executable)
--- a/reports/guided_reports.pl
+++ b/reports/guided_reports.pl
@@ -911,9 +911,8 @@ elsif ($phase eq 'Export'){
                 $content .= join("\t", map { $_ // '' } @$row) . "\n";
             }
         } else {
-            my $delimiter = C4::Context->preference('CSVDelimiter') || ',';
             if ( $format eq 'csv' ) {
-                $delimiter = "\t" if $delimiter eq 'tabulation';
+                my $delimiter = C4::Context->csv_delimiter;
                 $type = 'application/csv';
                 my $csv = Text::CSV::Encoded->new({ encoding_out => 'UTF-8', sep_char => $delimiter});
                 $csv or die "Text::CSV::Encoded->new({binary => 1}) FAILED: " . Text::CSV::Encoded->error_diag();

diff --git a/reports/issues_avg_stats.pl b/reports/issues_avg_stats.pl
index ebba699..e1270c9 100755 (executable)
--- a/reports/issues_avg_stats.pl
+++ b/reports/issues_avg_stats.pl
@@ -56,8 +56,7 @@ my ($template, $borrowernumber, $cookie)
                 type => "intranet",
                 flagsrequired => {reports => '*'},
                     });
-our $sep     = $input->param("sep");
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep = C4::Context->csv_delimiter(scalar $input->param("sep"));
 $template->param(do_it => $do_it,
     );
 if ($do_it) {

diff --git a/reports/issues_stats.pl b/reports/issues_stats.pl
index e06bb51..52e904e 100755 (executable)
--- a/reports/issues_stats.pl
+++ b/reports/issues_stats.pl
@@ -70,8 +70,7 @@ my ($template, $borrowernumber, $cookie) = get_template_and_user({
        type => "intranet",
        flagsrequired => {reports => '*'},
 });
-our $sep     = $input->param("sep") // ';';
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep = C4::Context->csv_delimiter(scalar $input->param("sep"));
 $template->param(do_it => $do_it,
 );
 

diff --git a/reports/orders_by_fund.pl b/reports/orders_by_fund.pl
index b630350..c831bf2 100755 (executable)
--- a/reports/orders_by_fund.pl
+++ b/reports/orders_by_fund.pl
@@ -32,6 +32,7 @@ use C4::Auth qw( get_template_and_user );
 use C4::Output qw( output_html_with_http_headers );
 use C4::Budgets qw( GetBudgetsReport GetBudgetHierarchy );
 use C4::Acquisition qw( GetBasket get_rounded_price );
+use C4::Context;
 use Koha::Biblios;
 
 my $query = CGI->new;
@@ -128,8 +129,7 @@ if ( $get_orders ) {
     # If we are outputting to a file, create it and exit.
     else {
         my $basename = $params->{"basename"};
-        my $sep = $params->{"sep"};
-        $sep = "\t" if ($sep eq 'tabulation');
+        my $sep = C4::Context->csv_delimiter(scalar $params->{"sep"});
 
         # TODO Use Text::CSV to generate the CSV file
         print $query->header(

diff --git a/reports/reserves_stats.pl b/reports/reserves_stats.pl
index f7db54d..af4e0ab 100755 (executable)
--- a/reports/reserves_stats.pl
+++ b/reports/reserves_stats.pl
@@ -64,8 +64,7 @@ my ($template, $borrowernumber, $cookie) = get_template_and_user({
        type => "intranet",
        flagsrequired => {reports => '*'},
 });
-our $sep     = $input->param("sep") || '';
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep = C4::Context->csv_delimiter(scalar $input->param("sep"));
 $template->param(do_it => $do_it,
 );
 

diff --git a/reports/serials_stats.pl b/reports/serials_stats.pl
index f0c1505..c625424 100755 (executable)
--- a/reports/serials_stats.pl
+++ b/reports/serials_stats.pl
@@ -42,8 +42,7 @@ my $expired    = $input->param("expired");
 my $order      = $input->param("order");
 my $output     = $input->param("output");
 my $basename   = $input->param("basename");
-our $sep       = $input->param("sep") || '';
-$sep = "\t" if ($sep eq 'tabulation');
+our $sep       = C4::Context->csv_delimiter(scalar $input->param("sep"));
 
 my ($template, $borrowernumber, $cookie)
        = get_template_and_user({template_name => $templatename,

diff --git a/tools/viewlog.pl b/tools/viewlog.pl
index c7f4273..49eeb5d 100755 (executable)
--- a/tools/viewlog.pl
+++ b/tools/viewlog.pl
@@ -229,8 +229,8 @@ if ($do_it) {
 
         # Printing to a csv file
         my $content = q{};
-        my $delimiter = C4::Context->preference('CSVDelimiter') || ',';
         if (@data) {
+            my $delimiter = C4::Context->csv_delimiter;
             my $csv = Text::CSV::Encoded->new( { encoding_out => 'utf8', sep_char => $delimiter } );
             $csv or die "Text::CSV::Encoded->new FAILED: " . Text::CSV::Encoded->error_diag();