From 630ac44396aede6933f1ea446ee723b4775e2216 Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Wed, 12 May 2021 12:00:31 +0200 Subject: [PATCH] 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 --- C4/Context.pm | 20 +++++++++++++++++++ Koha/Template/Plugin/Koha.pm | 16 +++++++++++++++ admin/aqplan.pl | 4 ++-- .../catalogue/itemsearch_item.csv.inc | 2 +- .../en/includes/csv_headers/acqui/basket.tt | 2 +- .../includes/csv_headers/acqui/basketgroup.tt | 2 +- .../includes/csv_headers/acqui/lateorders.tt | 2 +- .../csv_headers/catalogue/itemsearch.tt | 2 +- .../prog/en/modules/acqui/csv/basket.tt | 2 +- .../prog/en/modules/acqui/csv/basketgroup.tt | 2 +- .../prog/en/modules/acqui/csv/lateorders.tt | 2 +- misc/cronjobs/overdue_notices.pl | 5 ++--- misc/export_borrowers.pl | 3 +-- reports/acquisitions_stats.pl | 3 +-- reports/bor_issues_top.pl | 3 +-- reports/borrowers_out.pl | 3 +-- reports/borrowers_stats.pl | 3 +-- reports/cash_register_stats.pl | 2 +- reports/cat_issues_top.pl | 3 +-- reports/catalogue_stats.pl | 3 +-- reports/guided_reports.pl | 3 +-- reports/issues_avg_stats.pl | 3 +-- reports/issues_stats.pl | 3 +-- reports/orders_by_fund.pl | 4 ++-- reports/reserves_stats.pl | 3 +-- reports/serials_stats.pl | 3 +-- tools/viewlog.pl | 2 +- 27 files changed, 64 insertions(+), 41 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index 8cb9bdefe7..876126b888 100644 --- 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 5ed58a95ad..9ecd7fde0d 100644 --- 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 64b6cd05ab..cbded1ca64 100755 --- 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 7835b1aa21..5d5bb4d25e 100644 --- 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 75d246c26e..bf408307f1 100644 --- 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 63cc1ceb14..be05cb98b4 100644 --- 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 5bda385df0..bc02b07306 100644 --- 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 238253ff74..59712c7797 100644 --- 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 ebc67b91c9..9987cd35cf 100644 --- 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 c005cd67c0..84df7f7538 100644 --- 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 7bef44ee43..dd753bd95e 100644 --- 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 9fdafe13b4..edf2e603d8 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -404,8 +404,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; @@ -833,7 +832,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 d433a55acc..7cde608ece 100755 --- 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 c804f72abe..ffd91ba59e 100755 --- a/reports/acquisitions_stats.pl +++ b/reports/acquisitions_stats.pl @@ -69,8 +69,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 912798ea7b..bf28f746a5 100755 --- a/reports/bor_issues_top.pl +++ b/reports/bor_issues_top.pl @@ -54,8 +54,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 9d7fba6cac..dce275b56e 100755 --- a/reports/borrowers_out.pl +++ b/reports/borrowers_out.pl @@ -49,8 +49,7 @@ $filters[1] = eval { output_pref( { dt => dt_from_string( $filters[1]), dateonly 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 7263c3b0dc..22cd9ba2b2 100755 --- a/reports/borrowers_stats.pl +++ b/reports/borrowers_stats.pl @@ -58,8 +58,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 871032941c..e77821470e 100755 --- a/reports/cash_register_stats.pl +++ b/reports/cash_register_stats.pl @@ -157,7 +157,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 23666f314c..582db47729 100755 --- a/reports/cat_issues_top.pl +++ b/reports/cat_issues_top.pl @@ -55,8 +55,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 c3795df6d2..b4d72a384b 100755 --- 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 c497dcc0a7..7a04d8a360 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -931,9 +931,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 7be36a54c2..e417a9b930 100755 --- a/reports/issues_avg_stats.pl +++ b/reports/issues_avg_stats.pl @@ -66,8 +66,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 ca0b8ec100..0f52ce4c96 100755 --- a/reports/issues_stats.pl +++ b/reports/issues_stats.pl @@ -75,8 +75,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 a44ca3f864..e5c9e79e39 100755 --- 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; use Koha::DateUtils qw( dt_from_string output_pref ); @@ -131,8 +132,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 94bd5dad8b..052c29a61b 100755 --- a/reports/reserves_stats.pl +++ b/reports/reserves_stats.pl @@ -65,8 +65,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 f0c150532e..c6254241c1 100755 --- 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 c7f4273565..49eeb5decc 100755 --- 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(); -- 2.35.3