| Summary: | Add `Koha::CSV` | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | Pushed to main --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | clemens.tubach, jake.deery, jonathan.druart, kyle, lucas, m.de.rooy, martin.renvoize, michaela.sieber, tomascohen |
| Version: | Main | Keywords: | release-notes-needed |
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
26.05.00
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 41620, 41622, 41625, 41626, 41627, 41628, 41629, 41630, 41631, 41632, 41633 | ||
| Attachments: |
Bug 41619: Add Koha::CSV
Bug 41619: Add Koha::CSV Bug 41619: Add Koha::CSV Bug 41619: (Follow-up) Consider Logical Defined-Or in test Bug 41619: (Follow-up) Consider Logical Defined-Or in test Bug 41619: (Follow-up) Fix for t/Koha/CSV.t |
||
|
Description
Tomás Cohen Arazi (tcohen)
2026-01-14 19:35:10 UTC
Created attachment 191466 [details] [review] 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 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. Created attachment 191491 [details] [review] 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> Created attachment 191551 [details] [review] 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> Nice work everyone! Pushed to main for 26.05 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.
Created attachment 193461 [details] [review] 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 == 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) Created attachment 193939 [details] [review] 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> Follow-up pushed to main. Thanks Jake! The follow up broke /kohadevbox/koha/t/Koha/CSV.t Created attachment 194099 [details] [review] 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. additional follow-up pushed to main. Thanks for the quick follow-up Jake! |