When choosing tabulation as the delimiter on a CSV profile for the lost item report, it's exported as shown below: title\tauthor\tstocknumber\tcallnumber "E Street shuffle :"\"Heylin, Clinton."\\ Tabs works best as default for Excel, so it would be great to see this fixed. Reads similar like other older closed bugs like bug 17590 and bug 7251.
Originally found in 20.05.03, but confirmed in master now.
Created attachment 113890 [details] [review] Bug 27045: Fix items lost report export if delimiter is tab
Created attachment 113891 [details] [review] Bug 27045: Fix other exports
Hum, I lost myself. I first thought it was because we were not using Text::CSV::Encoded. But found (after I rewrote itemslost.pl export) that it was because of the way we handle "\t". I think those 2 patches fixes the problem for all the CSV exports, but have only tried with itemslost so far.
Is this ready for testing now?
I don't know, see previous comment. Feel free to test it!
my $delimiter = $csv_profile->csv_separator; $delimiter = "\t" if $delimiter eq "\\t"; Ah strange : In system preference stored value is string "tabulation" : https://git.koha-community.org/Koha-community/Koha/src/commit/99c10bf4484635e9befdc9c09a56c0f7278c7976/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref#L50 We see in other places : $delimiter = "\t" if $delimiter eq 'tabulation'; https://git.koha-community.org/Koha-community/Koha/src/commit/65542edbe021d58ca6ef1332a8e3ca760b198391/reports/guided_reports.pl#L903 Looks like use of CSVdelimiter is wrong in many places where preference is fetched in TT. I create a new bug report for that. Maybe we shoud add a method C4::Context->csv_delimiter() ? And add a method to TT plugin.
In this case it's independent of the preference - for CSV profile the delimiter is chosen in configuration. But in general both (pref and CSV profiles) need fixing.
Created attachment 137790 [details] [review] Bug 27045: Fix items lost report export if delimiter is tab When choosing tabulation as the delimiter for a CSV profile for the lost item report, the tabs appear in the file as \t instead of as proper tabs. title\tauthor\tstocknumber\tcallnumber "E Street shuffle :"\"Heylin, Clinton."\\ To test: * Upate the existing sample lost item CSV profile to use tabs as separator * Make sure you have some lost items in your database or create some * Go to Reports > Lost items * Run the report * Check all or some of the checkboxes in the result list * Export using the link on top and the CSV profile * Verify the tabs are not exported correctly * Apply patch * Verify tabs now are proper tabs instead of \t
Created attachment 137791 [details] [review] Bug 27045: Fix other exports using CSV profiles This patch corrects the export of the 2 other reports using CSV profiles: * Late issues (serials) * Basket (acquisitions) To test: 1) Late issues * Update the late issues sample report to use tab as separator * Create a subscription * Go to serial collection and 'generate next' to get some late issues * Go to Claims * Export the late issues and verify format is correct * Verify exported file has tabs 2) Basket summary * Create an order with several order lines * Create an SQL type CSV profile for basket export using tab as separator Example: aqorders.quantity|aqordres.listprice|Title=biblio.title * Export the basket using your configured CSV profile * Verify exported file has tabs
Hi Joubu, I've rebased the patch set, cleaned up the descriptions and added test plans. There is only one problem: this works almost perfectly, but the header rows for the basket and late serial issues still show the \t, while the data has the proper tabs. Example: aqorders.quantity\taqordres.listprice\tTitle 1 12.000000 "Perl best practices /" Could you help?
Created attachment 137792 [details] [review] Bug 27045: Fix items lost report export if delimiter is tab When choosing tabulation as the delimiter for a CSV profile for the lost item report, the tabs appear in the file as \t instead of as proper tabs. title\tauthor\tstocknumber\tcallnumber "E Street shuffle :"\"Heylin, Clinton."\\ To test: * Upate the existing sample lost item CSV profile to use tabs as separator * Make sure you have some lost items in your database or create some * Go to Reports > Lost items * Run the report * Check all or some of the checkboxes in the result list * Export using the link on top and the CSV profile * Verify the tabs are not exported correctly * Apply patch * Verify tabs now are proper tabs instead of \t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137793 [details] [review] Bug 27045: Fix other exports using CSV profiles This patch corrects the export of the 2 other reports using CSV profiles: * Late issues (serials) * Basket (acquisitions) To test: 1) Late issues * Update the late issues sample report to use tab as separator * Create a subscription * Go to serial collection and 'generate next' to get some late issues * Go to Claims * Export the late issues and verify format is correct * Verify exported file has tabs 2) Basket summary * Create an order with several order lines * Create an SQL type CSV profile for basket export using tab as separator Example: aqorders.quantity|aqordres.listprice|Title=biblio.title * Export the basket using your configured CSV profile * Verify exported file has tabs Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137794 [details] [review] Bug 27045: (follow-up) Fix delimiter in header rows The header rows still showed \t because the newly defined variable wasn't used there. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(In reply to Katrin Fischer from comment #11) > Hi Joubu, > > I've rebased the patch set, cleaned up the descriptions and added test > plans. There is only one problem: this works almost perfectly, but the > header rows for the basket and late serial issues still show the \t, while > the data has the proper tabs. > > Could you help? Never mind - found the fix myself :)
Created attachment 137933 [details] [review] Bug 27045: Fix items lost report export if delimiter is tab When choosing tabulation as the delimiter for a CSV profile for the lost item report, the tabs appear in the file as \t instead of as proper tabs. title\tauthor\tstocknumber\tcallnumber "E Street shuffle :"\"Heylin, Clinton."\\ To test: * Upate the existing sample lost item CSV profile to use tabs as separator * Make sure you have some lost items in your database or create some * Go to Reports > Lost items * Run the report * Check all or some of the checkboxes in the result list * Export using the link on top and the CSV profile * Verify the tabs are not exported correctly * Apply patch * Verify tabs now are proper tabs instead of \t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 137934 [details] [review] Bug 27045: Fix other exports using CSV profiles This patch corrects the export of the 2 other reports using CSV profiles: * Late issues (serials) * Basket (acquisitions) To test: 1) Late issues * Update the late issues sample report to use tab as separator * Create a subscription * Go to serial collection and 'generate next' to get some late issues * Go to Claims * Export the late issues and verify format is correct * Verify exported file has tabs 2) Basket summary * Create an order with several order lines * Create an SQL type CSV profile for basket export using tab as separator Example: aqorders.quantity|aqordres.listprice|Title=biblio.title * Export the basket using your configured CSV profile * Verify exported file has tabs Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 137935 [details] [review] Bug 27045: (follow-up) Fix delimiter in header rows The header rows still showed \t because the newly defined variable wasn't used there. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 22.11. Nice work everyone, thanks!
Please have a look at Bug 28327 where I try to centralize the tabulation special case.
(In reply to Fridolin Somers from comment #20) > Please have a look at Bug 28327 where I try to centralize the tabulation > special case. This doesn't depend on the pref, but is a per CSV profile setting. So my impresison was they complement each other.
(In reply to Katrin Fischer from comment #21) > (In reply to Fridolin Somers from comment #20) > > Please have a look at Bug 28327 where I try to centralize the tabulation > > special case. > > This doesn't depend on the pref, but is a per CSV profile setting. So my > impresison was they complement each other. In Bug 28327 I add a method csv_delimiter() that take a value to convert and fallsback to syspref if no value. So it may be used for CSV profiles.
Backported to 22.05.x for 22.05.05
thx pushed to 21.11.x for 21.11.12
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved.
(In reply to Victor Grousset/tuxayo from comment #25) > Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. > > Nothing to document, marking resolved. It's needed, we noticed this in 20.11 :)
Ok, will backport before next release :)
Backported: Pushed to 21.05.x branch for 21.05.19