[Bug 41619] New: Add `Koha::CSV`
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Bug ID: 41619 Summary: Add `Koha::CSV` Initiative type: --- Sponsorship --- status: Product: Koha Version: 24.11 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org I propose we add a wrapper class on top of `Text::CSV` that takes care of applying all the required settings we might need. It should provide a way to override those settings (e.g. separator character, formula, etc). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |martin.renvoize@openfifth.c | |o.uk, tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Version|24.11 |Main -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 191466 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191466&action=edit Bug 41619: Add Koha::CSV This patch introduces Koha::CSV, a wrapper around Text::CSV_XS that provides consistent defaults and methods for CSV generation across Koha. The class: - Inherits CSV delimiter from CSVDelimiter system preference - Enforces security defaults (binary=1, formula='empty') - Provides convenient methods for common CSV operations - Allows per-instance configuration overrides This establishes a foundation for standardizing CSV exports throughout Koha, eliminating inconsistencies in CSV generation and providing a testable, maintainable approach to CSV handling. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Koha/CSV.t => SUCCESS: Tests pass! 3. Tests cover: - Default initialization with CSVDelimiter preference - Preference value inheritance (comma, semicolon, tabulation) - Constructor parameter overrides - add_row() method with various data types - combine() and string() methods - print() to filehandle - Proper quote escaping and empty field handling 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41620 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41620 [Bug 41620] Make itemsearch.pl use Koha::CSV for CSV generation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41621 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41621 [Bug 41621] Use Koha::CSV everywhere -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|41621 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41621 [Bug 41621] Use Koha::CSV everywhere [UMBRELLA] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41622 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41622 [Bug 41622] Make patron export use Koha::CSV -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I used AI to analyze which `Text::CSV` methods we use across the codebase, and so required being added to `Koha::CSV`. Forgot to mention in the commit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41625 Blocks| |41626 Blocks| |41627 Blocks| |41628 Blocks| |41629 Blocks| |41630 Blocks| |41631 Blocks| |41632 Blocks| |41633 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41625 [Bug 41625] Add Koha::CSV::Labels for label exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41626 [Bug 41626] Add Koha::CSV::Reports for report exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41627 [Bug 41627] Add Koha::CSV::Acquisitions for acquisition exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41628 [Bug 41628] Add Koha::CSV::Serials for serials exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41629 [Bug 41629] Add Koha::CSV::Inventory for inventory exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41630 [Bug 41630] Add Koha::CSV::ActionLog for action log exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41631 [Bug 41631] Add Koha::CSV::LostItems for lost items exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41632 [Bug 41632] Add Koha::CSV::Overdue for overdue exports https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41633 [Bug 41633] Add Koha::CSV::Framework for framework exports -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191466|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 191491 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191491&action=edit Bug 41619: Add Koha::CSV This patch introduces Koha::CSV, a wrapper around Text::CSV_XS that provides consistent defaults and methods for CSV generation across Koha. The class: - Inherits CSV delimiter from CSVDelimiter system preference - Enforces security defaults (binary=1, formula='empty') - Provides convenient methods for common CSV operations - Allows per-instance configuration overrides This establishes a foundation for standardizing CSV exports throughout Koha, eliminating inconsistencies in CSV generation and providing a testable, maintainable approach to CSV handling. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Koha/CSV.t => SUCCESS: Tests pass! 3. Tests cover: - Default initialization with CSVDelimiter preference - Preference value inheritance (comma, semicolon, tabulation) - Constructor parameter overrides - add_row() method with various data types - combine() and string() methods - print() to filehandle - Proper quote escaping and empty field handling 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|Medium patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191491|0 |1 is obsolete| | --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 191551 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191551&action=edit Bug 41619: Add Koha::CSV This patch introduces Koha::CSV, a wrapper around Text::CSV_XS that provides consistent defaults and methods for CSV generation across Koha. The class: - Inherits CSV delimiter from CSVDelimiter system preference - Enforces security defaults (binary=1, formula='empty') - Provides convenient methods for common CSV operations - Allows per-instance configuration overrides This establishes a foundation for standardizing CSV exports throughout Koha, eliminating inconsistencies in CSV generation and providing a testable, maintainable approach to CSV handling. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Koha/CSV.t => SUCCESS: Tests pass! 3. Tests cover: - Default initialization with CSVDelimiter preference - Preference value inheritance (comma, semicolon, tabulation) - Constructor parameter overrides - add_row() method with various data types - combine() and string() methods - print() to filehandle - Proper quote escaping and empty field handling 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clemens.tubach@kit.edu, | |michaela.sieber@kit.edu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #5 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- xt/author/Text_CSV_Various.t .. 1/23 # Files using Text::CSV without formula protection: # Koha/CSV.pm # # CSV formula injection protection is required for security. # Add 'formula => "empty"' to Text::CSV constructor options. # Valid formula modes: 'empty' (recommended), 'die', 'croak', 'diag', or numeric 1-5 # Failed test 'All Text::CSV usage includes formula injection protection' # at xt/author/Text_CSV_Various.t line 157. # got: '1' # expected: '0' # Looks like you failed 1 test of 2. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #7 from Jake Deery <jake.deery@openfifth.co.uk> --- Created attachment 193461 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193461&action=edit Bug 41619: (Follow-up) Consider Logical Defined-Or in test This patch fixes xt/author/Text_CSV_Various.t by using a regex that can consider logical defined-or statements (|| and //, etc) when checking which formula has been used. TO TEST: a) run prove -vv xt/author/Text_CSV_Various.t *) notice the test fails == APPLY PATCH == b) repeat step a) *) notice the test now passes c) inspect diff for xt/author/Text_CSV_Various.t *) notice we are now using a wildcard (.*) between => and the first ' to skip variable declarations d) inspect diff for Koha/CSV.pm *) notice we are now hard-coding 'empty' as a defined-or in case someone decides not to define a format at the class level (which was not being done before anyway and is now defaulted to undefined) == SIGN OFF == -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Jake Deery <jake.deery@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jake.deery@openfifth.co.uk --- Comment #8 from Jake Deery <jake.deery@openfifth.co.uk> --- I've been getting this, too. I've resolved it by tweaking the regex and moving the hard-coded 'empty' to the Text::CSV_XS call. Others may have a different idea of how to resolve this, but my work is attached nonetheless. Ta, Jake (D) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #193461|0 |1 is obsolete| | --- Comment #9 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Created attachment 193939 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193939&action=edit Bug 41619: (Follow-up) Consider Logical Defined-Or in test This patch fixes xt/author/Text_CSV_Various.t by using a regex that can consider logical defined-or statements (|| and //, etc) when checking which formula has been used. TO TEST: a) run prove -vv xt/author/Text_CSV_Various.t *) notice the test fails == APPLY PATCH == b) repeat step a) *) notice the test now passes c) inspect diff for xt/author/Text_CSV_Various.t *) notice we are now using a wildcard (.*) between => and the first ' to skip variable declarations d) inspect diff for Koha/CSV.pm *) notice we are now hard-coding 'empty' as a defined-or in case someone decides not to define a format at the class level (which was not being done before anyway and is now defaulted to undefined) == SIGN OFF == Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #10 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Follow-up pushed to main. Thanks Jake! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #11 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- The follow up broke /kohadevbox/koha/t/Koha/CSV.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 --- Comment #12 from Jake Deery <jake.deery@openfifth.co.uk> --- Created attachment 194099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194099&action=edit Bug 41619: (Follow-up) Fix for t/Koha/CSV.t This patch fixes an omission that caused t/Koha/CSV.t to fail. To test, prove t/Koha/CSV.t and notice how it now passes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #13 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- additional follow-up pushed to main. Thanks for the quick follow-up Jake! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |jacob.omara@openfifth.co.uk --- Comment #14 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Enhancement, not backporting. Scream at me if you feel strongly it should be backported. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41619 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED Keywords|release-notes-needed | --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- This is internal libraries and doesn't need a Manual entry. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org