From b46418478ed5c0b28360aa6d0a01c1042f1bd28d Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 4 Sep 2023 09:30:14 -0300 Subject: [PATCH] Bug 34043: Remove incorrect spaces This bug looked cool and safe, but tests highlighted that the (introduced) newlines were translated into spaces, which is not correct in the CSV format (i.e. q{"Column 1" , "Column 2"} is not really correct). Also, the double quotes were forcibly introduced (semi-correct) but the tests weren't adjusted. We should really stop using templates for generating CSV, and use a library for the task instead of manually crafting them. But that's for another bug report. This patch: * Removes extra spaces in TT-generated CSV headers * Adjusts the tests to the new format introduced by this report Signed-off-by: Tomas Cohen Arazi --- .../en/includes/csv_headers/acqui/basket.tt | 28 +--------- .../includes/csv_headers/acqui/basketgroup.tt | 52 +------------------ .../includes/csv_headers/acqui/lateorders.tt | 22 +------- .../csv_headers/catalogue/itemsearch.tt | 34 +----------- .../reports/cash_register_stats.tt | 24 +-------- .../csv_headers/reports/orders_by_budget.tt | 34 +----------- t/db_dependent/Acquisition/GetBasketAsCSV.t | 23 ++++---- .../Acquisition/GetBasketGroupAsCSV.t | 27 ++++++---- 8 files changed, 38 insertions(+), 206 deletions(-) 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 d6ff935aa00..0f5ee36c3fa 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 @@ -3,32 +3,6 @@ [%- PROCESS 'i18n.inc' -%] [%- SET delimiter = Koha.CSVDelimiter() -%] [%- BLOCK -%] - "[% t("Contract name") | html %]" - [% delimiter | html %] - "[% t("Order number") | html %]" - [% delimiter | html %] - "[% t("Entry date") | html %]" - [% delimiter | html %] - "[% t("ISBN") | html %]" - [% delimiter | html %] - "[% t("Author") | html %]" - [% delimiter | html %] - "[% t("Title") | html %]" - [% delimiter | html %] - "[% t("Publication year") | html %]" - [% delimiter | html %] - "[% t("Publisher") | html %]" - [% delimiter | html %] - "[% t("Collection title") | html %]" - [% delimiter | html %] - "[% t("Note for vendor") | html %]" - [% delimiter | html %] - "[% t("Quantity") | html %]" - [% delimiter | html %] - "[% t("RRP") | html %]" - [% delimiter | html %] - "[% t("Delivery place") | html %]" - [% delimiter | html %] - "[% t("Billing place") | html %]" + "[% t("Contract name") | html %]"[% delimiter | html %]"[% t("Order number") | html %]"[% delimiter | html %]"[% t("Entry date") | html %]"[% delimiter | html %]"[% t("ISBN") | html %]"[% delimiter | html %]"[% t("Author") | html %]"[% delimiter | html %]"[% t("Title") | html %]"[% delimiter | html %]"[% t("Publication year") | html %]"[% delimiter | html %]"[% t("Publisher") | html %]"[% delimiter | html %]"[% t("Collection title") | html %]"[% delimiter | html %]"[% t("Note for vendor") | html %]"[% delimiter | html %]"[% t("Quantity") | html %]"[% delimiter | html %]"[% t("RRP") | html %]"[% delimiter | html %]"[% t("Delivery place") | html %]"[% delimiter | html %]"[% t("Billing place") | html %]" [%- END -%] [%- 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 d0f21a38073..b053130963b 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 @@ -3,56 +3,6 @@ [%- PROCESS 'i18n.inc' -%] [%- SET delimiter = Koha.CSVDelimiter() -%] [%- BLOCK -%] - "[% t("Account number") | html %]" - [% delimiter | html %] - "[% t("Basket name") | html %]" - [% delimiter | html %] - "[% t("Order number") | html %]" - [% delimiter | html %] - "[% t("Author") | html %]" - [% delimiter | html %] - "[% t("Title") | html %]" - [% delimiter | html %] - "[% t("Publisher") | html %]" - [% delimiter | html %] - "[% t("Publication year") | html %]" - [% delimiter | html %] - "[% t("Collection title") | html %]" - [% delimiter | html %] - "[% t("ISBN") | html %]" - [% delimiter | html %] - "[% t("Quantity") | html %]" - [% delimiter | html %] - "[% t("RRP tax included") | html %]" - [% delimiter | html %] - "[% t("RRP tax excluded") | html %]" - [% delimiter | html %] - "[% t("Discount") | html %]" - [% delimiter | html %] - "[% t("Estimated cost tax included") | html %]" - [% delimiter | html %] - "[% t("Estimated cost tax excluded") | html %]" - [% delimiter | html %] - "[% t("Note for vendor") | html %]" - [% delimiter | html %] - "[% t("Entry date") | html %]" - [% delimiter | html %] - "[% t("Bookseller name") | html %]" - [% delimiter | html %] - "[% t("Bookseller physical address") | html %]" - [% delimiter | html %] - "[% t("Bookseller postal address") | html %]" - [% delimiter | html %] - "[% t("Contract number") | html %]" - [% delimiter | html %] - "[% t("Contract name") | html %]" - [% delimiter | html %] - "[% t("Basket group delivery place") | html %]" - [% delimiter | html %] - "[% t("Basket group billing place") | html %]" - [% delimiter | html %] - "[% t("Basket delivery place") | html %]" - [% delimiter | html %] - "[% t("Basket billing place") | html %]" + "[% t("Account number") | html %]"[% delimiter | html %]"[% t("Basket name") | html %]"[% delimiter | html %]"[% t("Order number") | html %]"[% delimiter | html %]"[% t("Author") | html %]"[% delimiter | html %]"[% t("Title") | html %]"[% delimiter | html %]"[% t("Publisher") | html %]"[% delimiter | html %]"[% t("Publication year") | html %]"[% delimiter | html %]"[% t("Collection title") | html %]"[% delimiter | html %]"[% t("ISBN") | html %]"[% delimiter | html %]"[% t("Quantity") | html %]"[% delimiter | html %]"[% t("RRP tax included") | html %]"[% delimiter | html %]"[% t("RRP tax excluded") | html %]"[% delimiter | html %]"[% t("Discount") | html %]"[% delimiter | html %]"[% t("Estimated cost tax included") | html %]"[% delimiter | html %]"[% t("Estimated cost tax excluded") | html %]"[% delimiter | html %]"[% t("Note for vendor") | html %]"[% delimiter | html %]"[% t("Entry date") | html %]"[% delimiter | html %]"[% t("Bookseller name") | html %]"[% delimiter | html %]"[% t("Bookseller physical address") | html %]"[% delimiter | html %]"[% t("Bookseller postal address") | html %]"[% delimiter | html %]"[% t("Contract number") | html %]"[% delimiter | html %]"[% t("Contract name") | html %]"[% delimiter | html %]"[% t("Basket group delivery place") | html %]"[% delimiter | html %]"[% t("Basket group billing place") | html %]"[% delimiter | html %]"[% t("Basket delivery place") | html %]"[% delimiter | html %]"[% t("Basket billing place") | html %]" [%- END -%] [%- 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 d787966a162..ccd9c9446c9 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 @@ -3,26 +3,6 @@ [%- PROCESS 'i18n.inc' -%] [%- SET delimiter = Koha.CSVDelimiter() -%] [%- BLOCK -%] - "[% t("ORDER DATE") | html %]" - [%- delimiter | html -%] - "[% t("ESTIMATED DELIVERY DATE") | html %]" - [%- delimiter | html -%] - "[% t("VENDOR") | html %]" - [%- delimiter | html -%] - "[% t("INFORMATION") | html %]" - [%- delimiter | html -%] - "[% t("TOTAL COST") | html %]" - [%- delimiter | html -%] - "[% t("BASKET") | html %]" - [%- delimiter | html -%] - "[% t("CLAIMS COUNT") | html %]" - [%- delimiter | html -%] - "[% t("CLAIMED DATE") | html %]" - [%- delimiter | html -%] - "[% t("INTERNAL NOTE") | html %]" - [%- delimiter | html -%] - "[% t("VENDOR NOTE") | html %]" - [%- delimiter | html -%] - "[% t("ISBN") | html %]" + "[% t("ORDER DATE") | html %]"[%- delimiter | html -%]"[% t("ESTIMATED DELIVERY DATE") | html %]"[%- delimiter | html -%]"[% t("VENDOR") | html %]"[%- delimiter | html -%]"[% t("INFORMATION") | html %]"[%- delimiter | html -%]"[% t("TOTAL COST") | html %]"[%- delimiter | html -%]"[% t("BASKET") | html %]"[%- delimiter | html -%]"[% t("CLAIMS COUNT") | html %]"[%- delimiter | html -%]"[% t("CLAIMED DATE") | html %]"[%- delimiter | html -%]"[% t("INTERNAL NOTE") | html %]"[%- delimiter | html -%]"[% t("VENDOR NOTE") | html %]"[%- delimiter | html -%]"[% t("ISBN") | html %]" [%- END -%] [%- 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 930d9250ece..cf82cb47ee4 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 @@ -3,37 +3,5 @@ [%- PROCESS 'i18n.inc' -%] [%- SET delimiter = Koha.CSVDelimiter() -%] [%- BLOCK -%] - "[% t("Title") | html %]" - [%- delimiter | $raw -%] - "[% t("Publication date") | html %]" - [%- delimiter | $raw -%] - "[% t("Publisher") | html %]" - [%- delimiter | $raw -%] - "[% t("Collection") | html %]" - [%- delimiter | $raw -%] - "[% t("Barcode") | html %]" - [%- delimiter | $raw -%] - "[% t("Serial enumeration") | html %]" - [%- delimiter | $raw -%] - "[% t("Call number") | html %]" - [%- delimiter | $raw -%] - "[% t("Home library") | html %]" - [%- delimiter | $raw -%] - "[% t("Current library") | html %]" - [%- delimiter | $raw -%] - "[% t("Shelving location") | html %]" - [%- delimiter | $raw -%] - "[% t("Item type") | html %]" - [%- delimiter | $raw -%] - "[% t("Inventory number") | html %]" - [%- delimiter | $raw -%] - "[% t("Not for loan status") | html %]" - [%- delimiter | $raw -%] - "[% t("Lost status") | html %]" - [%- delimiter | $raw -%] - "[% t("Withdrawn status") | html %]" - [%- delimiter | $raw -%] - "[% t("Checkouts") | html %]" - [%- delimiter | $raw -%] - "[% t("Due date") | html %]" + "[% t("Title") | html %]"[%- delimiter | $raw -%]"[% t("Publication date") | html %]"[%- delimiter | $raw -%]"[% t("Publisher") | html %]"[%- delimiter | $raw -%]"[% t("Collection") | html %]"[%- delimiter | $raw -%]"[% t("Barcode") | html %]"[%- delimiter | $raw -%]"[% t("Serial enumeration") | html %]"[%- delimiter | $raw -%]"[% t("Call number") | html %]"[%- delimiter | $raw -%]"[% t("Home library") | html %]"[%- delimiter | $raw -%]"[% t("Current library") | html %]"[%- delimiter | $raw -%]"[% t("Shelving location") | html %]"[%- delimiter | $raw -%]"[% t("Item type") | html %]"[%- delimiter | $raw -%]"[% t("Inventory number") | html %]"[%- delimiter | $raw -%]"[% t("Not for loan status") | html %]"[%- delimiter | $raw -%]"[% t("Lost status") | html %]"[%- delimiter | $raw -%]"[% t("Withdrawn status") | html %]"[%- delimiter | $raw -%]"[% t("Checkouts") | html %]"[%- delimiter | $raw -%]"[% t("Due date") | html %]" [%- END -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/cash_register_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/cash_register_stats.tt index e77a60adddc..501aeaa7341 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/cash_register_stats.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/cash_register_stats.tt @@ -1,28 +1,6 @@ [% FILTER collapse %] [%- PROCESS 'i18n.inc' -%] [%- BLOCK -%] - "[% t("Manager name") | html %]" - [% sep | html %] - "[% t("Patron cardnumber") | html %]" - [% sep | html %] - "[% t("Patron name") | html %]" - [% sep | html %] - "[% t("Transaction library") | html %]" - [% sep | html %] - "[% t("Transaction date") | html %]" - [% sep | html %] - "[% t("Updated") | html %]" - [% sep | html %] - "[% t("Transaction type") | html %]" - [% sep |html %] - "[% t("Notes") | html %]" - [% sep | html %] - "[% t("Amount") | html %]" - [% sep | html %] - "[% t("Title") | html %]" - [% sep | html %] - "[% t("Barcode") | html %]" - [% sep | html %] - "[% t("Item type") | html %]" + "[% t("Manager name") | html %]"[% sep | html %]"[% t("Patron cardnumber") | html %]"[% sep | html %]"[% t("Patron name") | html %]"[% sep | html %]"[% t("Transaction library") | html %]"[% sep | html %]"[% t("Transaction date") | html %]"[% sep | html %]"[% t("Updated") | html %]"[% sep | html %]"[% t("Transaction type") | html %]"[% sep |html %]"[% t("Notes") | html %]"[% sep | html %]"[% t("Amount") | html %]"[% sep | html %]"[% t("Title") | html %]"[% sep | html %]"[% t("Barcode") | html %]"[% sep | html %]"[% t("Item type") | html %]" [%- END -%] [%- END -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/orders_by_budget.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/orders_by_budget.tt index 7961e0624bf..69382dce2b6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/orders_by_budget.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/reports/orders_by_budget.tt @@ -1,38 +1,6 @@ [% FILTER collapse %] [%- PROCESS 'i18n.inc' -%] [%- BLOCK -%] - "[% t("Fund") | html %]" - [% sep | html %] - "[% t("Basket num") | html %]" - [% sep | html %] - "[% t("Basket name") | html %]" - [% sep | html %] - "[% t("Authorised by") | html %]" - [% sep | html %] - "[% t("Biblio number") | html %]" - [% sep | html %] - "[% t("Title") | html %]" - [% sep | html %] - "[% t("Currency") | html %]" - [% sep | html %] - "[% t("Vendor price") | html %]" - [% sep | html %] - "[% t("RRP") | html %]" - [% sep | html %] - "[% t("Budgeted cost") | html %]" - [% sep | html %] - "[% t("Quantity") | html %]" - [% sep | html %] - "[% t("Total RRP") | html %]" - [% sep | html %] - "[% t("Total cost") | html %]" - [% sep | html %] - "[% t("Entry date") | html %]" - [% sep | html %] - "[% t("Date received") | html %]" - [% sep | html %] - "[% t("Internal note") | html %]" - [% sep | html %] - "[% t("Vendor note") | html %]" + "[% t("Fund") | html %]"[% sep | html %]"[% t("Basket num") | html %]"[% sep | html %]"[% t("Basket name") | html %]"[% sep | html %]"[% t("Authorised by") | html %]"[% sep | html %]"[% t("Biblio number") | html %]"[% sep | html %]"[% t("Title") | html %]"[% sep | html %]"[% t("Currency") | html %]"[% sep | html %]"[% t("Vendor price") | html %]"[% sep | html %]"[% t("RRP") | html %]"[% sep | html %]"[% t("Budgeted cost") | html %]"[% sep | html %]"[% t("Quantity") | html %]"[% sep | html %]"[% t("Total RRP") | html %]"[% sep | html %]"[% t("Total cost") | html %]"[% sep | html %]"[% t("Entry date") | html %]"[% sep | html %]"[% t("Date received") | html %]"[% sep | html %]"[% t("Internal note") | html %]"[% sep | html %]"[% t("Vendor note") | html %]" [%- END -%] [%- END -%] diff --git a/t/db_dependent/Acquisition/GetBasketAsCSV.t b/t/db_dependent/Acquisition/GetBasketAsCSV.t index d1dabb7e531..047e51bc612 100755 --- a/t/db_dependent/Acquisition/GetBasketAsCSV.t +++ b/t/db_dependent/Acquisition/GetBasketAsCSV.t @@ -75,11 +75,14 @@ is($basket_csv1, 'autor,title,quantity ', 'CSV should be generated with user profile'); # Use default template -t::lib::Mocks::mock_preference('CSVDelimiter', ','); -my $basket_csv2 = C4::Acquisition::GetBasketAsCSV($basketno, $query); -is($basket_csv2, 'Contract name,Order number,Entry date,ISBN,Author,Title,Publication year,Publisher,Collection title,Note for vendor,Quantity,RRP,Delivery place,Billing place -"",' . $order->ordernumber . ',2016-01-02,,"King, Stephen","Test Record",,"","","",3,,"","" -', 'CSV should be generated with default template'); +t::lib::Mocks::mock_preference( 'CSVDelimiter', ',' ); +my $basket_csv2 = C4::Acquisition::GetBasketAsCSV( $basketno, $query ); +is( + $basket_csv2, + '"Contract name","Order number","Entry date","ISBN","Author","Title","Publication year","Publisher","Collection title","Note for vendor","Quantity","RRP","Delivery place","Billing place" +"",' . $order->ordernumber . ',2016-01-02,,"King, Stephen","Test Record",,"","","",3,,"","" +', 'CSV should be generated with default template' +); my $basket_csv3 = C4::Acquisition::GetBasketAsCSV($basketno, $query, $csv_profile2->export_format_id); is($basket_csv3, 'biblio.author,title,quantity @@ -94,10 +97,12 @@ try { }; Koha::Biblios->find($biblionumber)->delete; -my $basket_csv4 = C4::Acquisition::GetBasketAsCSV($basketno, $query); -is($basket_csv4, 'Contract name,Order number,Entry date,ISBN,Author,Title,Publication year,Publisher,Collection title,Note for vendor,Quantity,RRP,Delivery place,Billing place +my $basket_csv4 = C4::Acquisition::GetBasketAsCSV( $basketno, $query ); +is( + $basket_csv4, + '"Contract name","Order number","Entry date","ISBN","Author","Title","Publication year","Publisher","Collection title","Note for vendor","Quantity","RRP","Delivery place","Billing place" "",' . $order->ordernumber . ',2016-01-02,,"","",,"","","",3,,"","" -', 'CSV should not fail if biblio does not exist'); - +', 'CSV should not fail if biblio does not exist' +); $schema->storage->txn_rollback(); diff --git a/t/db_dependent/Acquisition/GetBasketGroupAsCSV.t b/t/db_dependent/Acquisition/GetBasketGroupAsCSV.t index 577a8d52d29..3b531d335b9 100755 --- a/t/db_dependent/Acquisition/GetBasketGroupAsCSV.t +++ b/t/db_dependent/Acquisition/GetBasketGroupAsCSV.t @@ -58,16 +58,25 @@ my $order = Koha::Acquisition::Order->new({ entrydate => '2016-01-02', })->store; -my $basketgroup_csv1 = C4::Acquisition::GetBasketGroupAsCSV($basketgroupid, $query); -is($basketgroup_csv1, 'Account number,Basket name,Order number,Author,Title,Publisher,Publication year,Collection title,ISBN,Quantity,RRP tax included,RRP tax excluded,Discount,Estimated cost tax included,Estimated cost tax excluded,Note for vendor,Entry date,Bookseller name,Bookseller physical address,Bookseller postal address,Contract number,Contract name,Basket group delivery place,Basket group billing place,Basket delivery place,Basket billing place -,"",' . $order->ordernumber . ',"King, Stephen","Test Record","",,"",,3,0.00,0.00,,0.00,0.00,"",2016-01-02,"my vendor","vendor address","",,"","","","","" -', 'CSV should be generated'); +my $basketgroup_csv1 = C4::Acquisition::GetBasketGroupAsCSV( $basketgroupid, $query ); +is( + $basketgroup_csv1, + '"Account number","Basket name","Order number","Author","Title","Publisher","Publication year","Collection title","ISBN","Quantity","RRP tax included","RRP tax excluded","Discount","Estimated cost tax included","Estimated cost tax excluded","Note for vendor","Entry date","Bookseller name","Bookseller physical address","Bookseller postal address","Contract number","Contract name","Basket group delivery place","Basket group billing place","Basket delivery place","Basket billing place" +,"",' + . $order->ordernumber + . ',"King, Stephen","Test Record","",,"",,3,0.00,0.00,,0.00,0.00,"",2016-01-02,"my vendor","vendor address","",,"","","","","" +', 'CSV should be generated' +); Koha::Biblios->find($biblionumber)->delete; -my $basketgroup_csv2 = C4::Acquisition::GetBasketGroupAsCSV($basketgroupid, $query); -is($basketgroup_csv2, 'Account number,Basket name,Order number,Author,Title,Publisher,Publication year,Collection title,ISBN,Quantity,RRP tax included,RRP tax excluded,Discount,Estimated cost tax included,Estimated cost tax excluded,Note for vendor,Entry date,Bookseller name,Bookseller physical address,Bookseller postal address,Contract number,Contract name,Basket group delivery place,Basket group billing place,Basket delivery place,Basket billing place -,"",' . $order->ordernumber . ',"","","",,"",,3,0.00,0.00,,0.00,0.00,"",2016-01-02,"my vendor","vendor address","",,"","","","","" -', 'CSV should not fail if biblio does not exist'); - +my $basketgroup_csv2 = C4::Acquisition::GetBasketGroupAsCSV( $basketgroupid, $query ); +is( + $basketgroup_csv2, + '"Account number","Basket name","Order number","Author","Title","Publisher","Publication year","Collection title","ISBN","Quantity","RRP tax included","RRP tax excluded","Discount","Estimated cost tax included","Estimated cost tax excluded","Note for vendor","Entry date","Bookseller name","Bookseller physical address","Bookseller postal address","Contract number","Contract name","Basket group delivery place","Basket group billing place","Basket delivery place","Basket billing place" +,"",' + . $order->ordernumber + . ',"","","",,"",,3,0.00,0.00,,0.00,0.00,"",2016-01-02,"my vendor","vendor address","",,"","","","","" +', 'CSV should not fail if biblio does not exist' +); $schema->storage->txn_rollback(); -- 2.42.0