Bug 40387 - t/db_dependent/Koha/EDI.t generates warnings
Summary: t/db_dependent/Koha/EDI.t generates warnings
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Martin Renvoize (ashimema)
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 40386
Blocks: 40444
  Show dependency treegraph
 
Reported: 2025-07-15 12:39 UTC by Jonathan Druart
Modified: 2025-08-22 08:29 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.03
Circulation function:


Attachments
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing (2.79 KB, patch)
2025-07-18 13:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40387: Add test for undefined tax rate handling in EDI processing (4.55 KB, patch)
2025-07-18 13:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger (3.50 KB, patch)
2025-07-18 13:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40387: Add tests for create_edi_order logger calls (3.71 KB, patch)
2025-07-18 13:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing (2.83 KB, patch)
2025-07-18 21:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 40387: Add test for undefined tax rate handling in EDI processing (4.60 KB, patch)
2025-07-18 21:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger (3.55 KB, patch)
2025-07-18 21:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 40387: Add tests for create_edi_order logger calls (3.76 KB, patch)
2025-07-18 21:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 40387: Comment some of the superflous diag (1.46 KB, patch)
2025-07-23 09:53 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing (2.91 KB, patch)
2025-07-23 12:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40387: Add test for undefined tax rate handling in EDI processing (4.67 KB, patch)
2025-07-23 12:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger (3.62 KB, patch)
2025-07-23 12:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40387: Add tests for create_edi_order logger calls (3.83 KB, patch)
2025-07-23 12:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40387: Comment some of the superflous diag (1.54 KB, patch)
2025-07-23 12:35 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-07-15 12:39:33 UTC
03:27:57 koha_1       | substr outside of string at /kohadevbox/koha/Koha/Edifact/Segment.pm line 101.
03:27:57 koha_1       | Use of uninitialized value $seg in substitution (s///) at /kohadevbox/koha/Koha/Edifact/Segment.pm line 103.
03:27:57 koha_1       | Use of uninitialized value $seg in split at /kohadevbox/koha/Koha/Edifact/Segment.pm line 104.
03:27:57 koha_1       | substr outside of string at /kohadevbox/koha/Koha/Edifact/Segment.pm line 101.
03:27:57 koha_1       | Use of uninitialized value $seg in substitution (s///) at /kohadevbox/koha/Koha/Edifact/Segment.pm line 103.
03:27:57 koha_1       | Use of uninitialized value $seg in split at /kohadevbox/koha/Koha/Edifact/Segment.pm line 104.
03:27:57 koha_1       |         #     Loaded QUOTE file with 1 Message that contains 2 LIN segments with the first
03:27:57 koha_1       |         #     consistent of 1 item and the second with 3 items with 2 different funds.
03:27:57 koha_1       | No matching EAN found for 5412345000013 at t/db_dependent/Koha/EDI.t line 385.
03:27:57 koha_1       |         # Looking at order: 1
03:27:57 koha_1       |         # Looking at order: 2
03:27:57 koha_1       |         # Looking at order: 3
03:27:57 koha_1       |         # Second LIN split into 2 Orderlines, one for each Fund
03:27:57 koha_1       |         #     Loading QUOTE file with 1 Message that contains 2 LIN segments with the first
03:27:57 koha_1       |         #     consistent of 1 item and the second with 3 items with 2 different undefined funds.
03:27:57 koha_1       |         #     Loading QUOTE file with 2 messages
03:27:57 koha_1       | No orderlines for basket 285 at t/db_dependent/Koha/EDI.t line 618.
03:27:57 koha_1       | No orderlines for basket 286 at t/db_dependent/Koha/EDI.t line 618.
03:27:57 koha_1       | Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/EDI.pm line 420.
03:27:57 koha_1       | Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/EDI.pm line 420.
Comment 1 Martin Renvoize (ashimema) 2025-07-18 13:48:20 UTC
Created attachment 184379 [details] [review]
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing

When processing EDI invoices without TAX segments, warnings were
generated due to undefined tax rates in multiplication operations.

The tax_rate() method in Koha::Edifact::Line correctly returns undef
when no TAX segments are present in the EDI message. However, the
invoice processing code in Koha::EDI didn't handle this case properly.

This patch fixes the issue by:
- Adding checks for undefined tax rates before using them in calculations
- Defaulting to 0 tax rate when no tax information is present
- Ensuring tax_value calculations use 0 instead of undefined values

This eliminates "Use of uninitialized value in multiplication" warnings
while maintaining correct processing of invoices without tax information.
Comment 2 Martin Renvoize (ashimema) 2025-07-18 13:48:22 UTC
Created attachment 184380 [details] [review]
Bug 40387: Add test for undefined tax rate handling in EDI processing

This adds a test to verify that EDI invoice processing correctly handles
invoices without TAX segments (undefined tax rates).

The test covers:
1. Processing invoices without TAX segments
2. Verifying no warnings about uninitialized values in multiplication
3. Confirming tax rates are set to 0 when no tax information is present
4. Ensuring invoice processing completes successfully
Comment 3 Martin Renvoize (ashimema) 2025-07-18 13:48:24 UTC
Created attachment 184381 [details] [review]
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger

This patch adds Test::NoWarnings to our EDI unit tests. In doing so, it
reminded me that we still had some old carp style error handling in the
process_quote section and that we shoudl replace those with calls to
Koha::Logger instead.

Changes made:
1. Added Test::NoWarnings import to catch any warnings
2. Replace 'carp "Message"' appropriately with
   '$logger->warn("Message")'
Comment 4 Martin Renvoize (ashimema) 2025-07-18 13:48:26 UTC
Created attachment 184382 [details] [review]
Bug 40387: Add tests for create_edi_order logger calls

Tests for the logger calls introduced in the create_edi_order
in the last commit.
Comment 5 David Nind 2025-07-18 21:56:02 UTC
Created attachment 184414 [details] [review]
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing

When processing EDI invoices without TAX segments, warnings were
generated due to undefined tax rates in multiplication operations.

The tax_rate() method in Koha::Edifact::Line correctly returns undef
when no TAX segments are present in the EDI message. However, the
invoice processing code in Koha::EDI didn't handle this case properly.

This patch fixes the issue by:
- Adding checks for undefined tax rates before using them in calculations
- Defaulting to 0 tax rate when no tax information is present
- Ensuring tax_value calculations use 0 instead of undefined values

This eliminates "Use of uninitialized value in multiplication" warnings
while maintaining correct processing of invoices without tax information.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2025-07-18 21:56:05 UTC
Created attachment 184415 [details] [review]
Bug 40387: Add test for undefined tax rate handling in EDI processing

This adds a test to verify that EDI invoice processing correctly handles
invoices without TAX segments (undefined tax rates).

The test covers:
1. Processing invoices without TAX segments
2. Verifying no warnings about uninitialized values in multiplication
3. Confirming tax rates are set to 0 when no tax information is present
4. Ensuring invoice processing completes successfully

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-07-18 21:56:08 UTC
Created attachment 184416 [details] [review]
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger

This patch adds Test::NoWarnings to our EDI unit tests. In doing so, it
reminded me that we still had some old carp style error handling in the
process_quote section and that we shoudl replace those with calls to
Koha::Logger instead.

Changes made:
1. Added Test::NoWarnings import to catch any warnings
2. Replace 'carp "Message"' appropriately with
   '$logger->warn("Message")'

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2025-07-18 21:56:12 UTC
Created attachment 184417 [details] [review]
Bug 40387: Add tests for create_edi_order logger calls

Tests for the logger calls introduced in the create_edi_order
in the last commit.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2025-07-18 21:57:08 UTC
Testing notes (using KTD):
1. prove t/db_dependent/Koha/EDI.t
2. Output from running tests:

prove t/db_dependent/Koha/EDI.t
t/db_dependent/Koha/EDI.t ..         #     Loaded QUOTE file with 1 Message that contains 2 LIN segments with the first
        #     consistent of 1 item and the second with 3 items with 2 different funds.
        # Looking at order: 1
        # Looking at order: 2
        # Looking at order: 3
        # Second LIN split into 2 Orderlines, one for each Fund
        #     Loading QUOTE file with 1 Message that contains 2 LIN segments with the first
        #     consistent of 1 item and the second with 3 items with 2 different undefined funds.
        #     Loading QUOTE file with 2 messages
t/db_dependent/Koha/EDI.t .. ok   
All tests successful.
Files=1, Tests=5,  5 wallclock secs ( 0.02 usr  0.00 sys +  3.04 cusr  1.19 csys =  4.25 CPU)
Result: PASS
Comment 10 Jonathan Druart 2025-07-21 07:25:46 UTC
Martin, do you think it's useful to keep the diag output here? I've been tempted to remove all kind of output from the tests if there were passing.
Comment 11 Martin Renvoize (ashimema) 2025-07-23 09:53:36 UTC
Created attachment 184539 [details] [review]
Bug 40387: Comment some of the superflous diag

The test output was a little verbose in places, however the diag's can
be helpful when debugging failures. I've opted to comment rather than
completely remove them.
Comment 12 Jonathan Druart 2025-07-23 12:35:01 UTC
Created attachment 184544 [details] [review]
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing

When processing EDI invoices without TAX segments, warnings were
generated due to undefined tax rates in multiplication operations.

The tax_rate() method in Koha::Edifact::Line correctly returns undef
when no TAX segments are present in the EDI message. However, the
invoice processing code in Koha::EDI didn't handle this case properly.

This patch fixes the issue by:
- Adding checks for undefined tax rates before using them in calculations
- Defaulting to 0 tax rate when no tax information is present
- Ensuring tax_value calculations use 0 instead of undefined values

This eliminates "Use of uninitialized value in multiplication" warnings
while maintaining correct processing of invoices without tax information.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2025-07-23 12:35:03 UTC
Created attachment 184545 [details] [review]
Bug 40387: Add test for undefined tax rate handling in EDI processing

This adds a test to verify that EDI invoice processing correctly handles
invoices without TAX segments (undefined tax rates).

The test covers:
1. Processing invoices without TAX segments
2. Verifying no warnings about uninitialized values in multiplication
3. Confirming tax rates are set to 0 when no tax information is present
4. Ensuring invoice processing completes successfully

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2025-07-23 12:35:06 UTC
Created attachment 184546 [details] [review]
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger

This patch adds Test::NoWarnings to our EDI unit tests. In doing so, it
reminded me that we still had some old carp style error handling in the
process_quote section and that we shoudl replace those with calls to
Koha::Logger instead.

Changes made:
1. Added Test::NoWarnings import to catch any warnings
2. Replace 'carp "Message"' appropriately with
   '$logger->warn("Message")'

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2025-07-23 12:35:08 UTC
Created attachment 184547 [details] [review]
Bug 40387: Add tests for create_edi_order logger calls

Tests for the logger calls introduced in the create_edi_order
in the last commit.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Jonathan Druart 2025-07-23 12:35:10 UTC
Created attachment 184548 [details] [review]
Bug 40387: Comment some of the superflous diag

The test output was a little verbose in places, however the diag's can
be helpful when debugging failures. I've opted to comment rather than
completely remove them.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Lucas Gass (lukeg) 2025-07-23 22:42:30 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 18 Paul Derscheid 2025-08-22 08:29:38 UTC
Nice work everyone!

Pushed to 25.05.x