Bug 27017 - Add further defensive coding to EDI Invoice handling
Summary: Add further defensive coding to EDI Invoice handling
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-13 13:45 UTC by Colin Campbell
Modified: 2023-06-08 22:32 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Patch adding log entries (1.42 KB, patch)
2020-11-13 13:54 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 27017: Code more defensively in process_invoice (1.80 KB, patch)
2022-10-27 12:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27017: Code more defensively in process_invoice (1.86 KB, patch)
2022-10-27 17:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27017: (QA follow-up) Don't die if aqorders.biblionumber is null (965 bytes, patch)
2022-10-27 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2020-11-13 13:45:08 UTC
It is useful to log to the trace log the bib numbers affected by Invoice processing. It makes it much easier to establish what processing has occured if any question has been raised
Comment 1 Colin Campbell 2020-11-13 13:54:28 UTC
Created attachment 113604 [details] [review]
Patch adding log entries

Patch adds two log entries one if no order match one to record bib if processing ok. If no match code jumps to next line to avoid possible runtime errors further down.

Recording this info in the the edi log has proved very useful.
Comment 2 David Nind 2022-09-18 22:18:55 UTC
Hi Colin.

The patch still applies.

I'm not familiar with EDI, but I am happy to test if you could include a test plan on how to set this up for testing.

Also, the QA Team might ask you to fix up the commit message for the patch - see https://wiki.koha-community.org/wiki/Commit_messages
Comment 3 Katrin Fischer 2022-09-19 06:15:07 UTC
I've updated assignee to Martin for now - afaik Colin is retired :)
Comment 4 David Nind 2022-09-19 18:01:15 UTC
Thanks Katrin!
Comment 5 Martin Renvoize 2022-10-27 12:47:29 UTC
Created attachment 142714 [details] [review]
Bug 27017: Code more defensively in process_invoice

This patch adds a check for ordernumber precence in the EDI Invoice
message lines and skips process if one is not found.

We log the failure as a trace log to allow administrators the oportunity
to track down the issue further.

We also add a trace log to entries to show which bib the invoice line
is receipting to.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2022-10-27 12:58:20 UTC
I've raised this one with Kyle for a QA run.. EDI is always hard to test.. but the code is pretty simple to read and understand here so hopefully we can get it moving without too much pain.
Comment 7 Kyle M Hall 2022-10-27 17:52:38 UTC
Created attachment 142726 [details] [review]
Bug 27017: Code more defensively in process_invoice

This patch adds a check for ordernumber precence in the EDI Invoice
message lines and skips process if one is not found.

We log the failure as a trace log to allow administrators the oportunity
to track down the issue further.

We also add a trace log to entries to show which bib the invoice line
is receipting to.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2022-10-27 18:03:28 UTC
Created attachment 142730 [details] [review]
Bug 27017: (QA follow-up) Don't die if aqorders.biblionumber is null
Comment 9 Tomás Cohen Arazi 2022-10-27 18:05:11 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!