[Bug 39215] New: EDIFACT quote files should skip orders that cannot be processed rather than failing to complete
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Bug ID: 39215 Summary: EDIFACT quote files should skip orders that cannot be processed rather than failing to complete Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org When loading an EDIFACT quote file, if the process finds problematic data (usually a mismatched LLO - Branchcode or other enumerated coded field) the load process gets stuck and displays as 'Processing' in the EDFACT messages page. This usually means that the remainder of the quotes in the file have to be manually corrected and is it's currently difficult to debug by library staff. It would be preferable if the process could skip, and report any problem data but complete all other quotes. -- 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=39215 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |38689 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38689 [Bug 38689] EDIFACT errors should log to a staff accessible location -- 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=39215 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=39215 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- A first step should be to wrap 'Koha::Item->new($item_hash)->store;' in try/catch blocks and report foreign key failures before moving to the next item creation. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |32938 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32938 [Bug 32938] Acquisitions EDI - ORDRSP messages are loaded as invoices -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41087 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41087 [Bug 41087] EDI Retry: We should add the option to retry EDI processing for errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Comma delimited| |OpenFifth list of Sponsors| |<https://openfifth.co.uk> Patch complexity|--- |Small patch Sponsorship status|--- |Sponsored Status|ASSIGNED |Needs Signoff QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201230 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201230&action=edit Bug 39215: Skip and report EDIFACT quote lines that cannot be processed When loading an EDIFACT quote, problematic data on a single order line (most commonly a mismatched branchcode in the LLO GIR field, or any other enumerated/coded value that breaches a database constraint) caused the whole process_quote run to die. The message was left stuck displaying as 'Processing' on the EDIFACT messages page and the remaining, valid quotes in the file were never imported. This patch makes quote processing resilient: - Each order line is processed inside a try/catch in process_quote, so an unexpected failure is reported and skipped rather than aborting the file. The message always resolves to 'received' or 'error', never 'processing'. - Item creation (Koha::Item->store) is wrapped in try/catch at each of the three creation sites. A failing item is reported through the EDIFACT error infrastructure and skipped, while sibling items and other order lines continue to be processed. - AddBiblio traps its own exceptions, warns them and returns an undef biblionumber. We now capture that warning, surface the underlying cause in the EDIFACT error report and skip the line, rather than carrying on to create an orphan order line against a non-existent biblio. - Database exceptions are translated into a concise, end-user friendly message (e.g. the offending field name) so library staff can identify the problem data. - A basket that contained any unprocessable line is no longer auto-ordered; it is left open with a logged explanation so staff can review and correct the affected lines before ordering manually. A FIXME notes that wrapping the order + item creation in a transaction (to roll a failed line back cleanly) is left to bug 41087 (EDI Retry). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201231 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201231&action=edit Bug 39215: Unit tests for skipping unprocessable EDIFACT quote lines Adds coverage to t/db_dependent/Koha/EDI.t: - unprocessable_line_handling: a quote whose first line references a non-existent branchcode (LLO+ZZ). Processing completes without dying, the bad line is reported with the offending field, the message status becomes 'error' (not 'processing'), the second valid line is still imported, and auto-ordering is blocked for the affected basket. - biblio_creation_failure_reporting: mocks AddBiblio to fail the way it does in production (warns and returns undef) and confirms the warning is captured into the EDIFACT error report, the line is skipped and no order is created against a missing biblio. Test plan: 1) prove t/db_dependent/Koha/EDI.t 2) Confirm all subtests pass, including the two new ones. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Comma delimited|OpenFifth |OpenFifth list of Sponsors|<https://openfifth.co.uk> |<https://openfifth.co.uk/> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201230|0 |1 is obsolete| | Attachment #201231|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=39215 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201232&action=edit Bug 39215: Skip and report EDIFACT quote lines that cannot be processed When loading an EDIFACT quote, problematic data on a single order line (most commonly a mismatched branchcode in the LLO GIR field, or any other enumerated/coded value that breaches a database constraint) caused the whole process_quote run to die. The message was left stuck displaying as 'Processing' on the EDIFACT messages page and the remaining, valid quotes in the file were never imported. This patch makes quote processing resilient: - Each order line is processed inside a try/catch in process_quote, so an unexpected failure is reported and skipped rather than aborting the file. The message always resolves to 'received' or 'error', never 'processing'. - Item creation (Koha::Item->store) is wrapped in try/catch at each of the three creation sites. A failing item is reported through the EDIFACT error infrastructure and skipped, while sibling items and other order lines continue to be processed. - AddBiblio traps its own exceptions, warns them and returns an undef biblionumber. We now capture that warning, surface the underlying cause in the EDIFACT error report and skip the line, rather than carrying on to create an orphan order line against a non-existent biblio. - Database exceptions are translated into a concise, end-user friendly message (e.g. the offending field name) so library staff can identify the problem data. - A basket that contained any unprocessable line is no longer auto-ordered; it is left open with a logged explanation so staff can review and correct the affected lines before ordering manually. A FIXME notes that wrapping the order + item creation in a transaction (to roll a failed line back cleanly) is left to bug 41087 (EDI Retry). Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201233&action=edit Bug 39215: Unit tests for skipping unprocessable EDIFACT quote lines Adds coverage to t/db_dependent/Koha/EDI.t: - unprocessable_line_handling: a quote whose first line references a non-existent branchcode (LLO+ZZ). Processing completes without dying, the bad line is reported with the offending field, the message status becomes 'error' (not 'processing'), the second valid line is still imported, and auto-ordering is blocked for the affected basket. - biblio_creation_failure_reporting: mocks AddBiblio to fail the way it does in production (warns and returns undef) and confirms the warning is captured into the EDIFACT error report, the line is skipped and no order is created against a missing biblio. Test plan: 1) prove t/db_dependent/Koha/EDI.t 2) Confirm all subtests pass, including the two new ones. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201232|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=39215 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201233|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=39215 --- Comment #6 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 201379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201379&action=edit Bug 39215: Skip and report EDIFACT quote lines that cannot be processed When loading an EDIFACT quote, problematic data on a single order line (most commonly a mismatched branchcode in the LLO GIR field, or any other enumerated/coded value that breaches a database constraint) caused the whole process_quote run to die. The message was left stuck displaying as 'Processing' on the EDIFACT messages page and the remaining, valid quotes in the file were never imported. This patch makes quote processing resilient: - Each order line is processed inside a try/catch in process_quote, so an unexpected failure is reported and skipped rather than aborting the file. The message always resolves to 'received' or 'error', never 'processing'. - Item creation (Koha::Item->store) is wrapped in try/catch at each of the three creation sites. A failing item is reported through the EDIFACT error infrastructure and skipped, while sibling items and other order lines continue to be processed. - AddBiblio traps its own exceptions, warns them and returns an undef biblionumber. We now capture that warning, surface the underlying cause in the EDIFACT error report and skip the line, rather than carrying on to create an orphan order line against a non-existent biblio. - Database exceptions are translated into a concise, end-user friendly message (e.g. the offending field name) so library staff can identify the problem data. - A basket that contained any unprocessable line is no longer auto-ordered; it is left open with a logged explanation so staff can review and correct the affected lines before ordering manually. A FIXME notes that wrapping the order + item creation in a transaction (to roll a failed line back cleanly) is left to bug 41087 (EDI Retry). Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #7 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 201380 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201380&action=edit Bug 39215: Unit tests for skipping unprocessable EDIFACT quote lines Adds coverage to t/db_dependent/Koha/EDI.t: - unprocessable_line_handling: a quote whose first line references a non-existent branchcode (LLO+ZZ). Processing completes without dying, the bad line is reported with the offending field, the message status becomes 'error' (not 'processing'), the second valid line is still imported, and auto-ordering is blocked for the affected basket. - biblio_creation_failure_reporting: mocks AddBiblio to fail the way it does in production (warns and returns undef) and confirms the warning is captured into the EDIFACT error report, the line is skipped and no order is created against a missing biblio. Test plan: 1) prove t/db_dependent/Koha/EDI.t 2) Confirm all subtests pass, including the two new ones. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201908&action=edit Bug 39215: (follow-up) Tag search engine errors in EDIFACT biblio-creation failures AddBiblio failures during EDIFACT quote processing (e.g. a failed authority auto-link lookup or post-store index update) and failures caused by bad quote data look identical in the EDIFACT error report - both are just whatever text AddBiblio happened to warn(). This makes it hard to tell, without re-investigating from scratch each time, whether a recurring failure is a search engine (Elasticsearch/Zebra) problem or a data problem. Add _looks_like_search_engine_error() to pattern-match the captured warning against known search engine exception signatures (Search::Elasticsearch, NoNodes, ZOOM::Exception) and prefix matching messages with "[SEARCH ENGINE ERROR]" in the logged detail, while preserving the full original text. Also add a fallback message for the case where AddBiblio's failure captured no warning text at all, so that case isn't silently unexplained. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201909&action=edit Bug 39215: Unit tests for search engine error tagging in EDIFACT biblio failures Adds biblio_creation_failure_search_engine_tagging to t/db_dependent/Koha/EDI.t, covering _looks_like_search_engine_error(): - a mocked AddBiblio warning matching a known search engine exception signature (Search::Elasticsearch) is tagged with [SEARCH ENGINE ERROR] in the EDIFACT error detail. - a mocked AddBiblio warning that does not match any such signature is left untagged, confirming ordinary data/FK failures are not misreported as search engine problems. Test plan: 1) prove t/db_dependent/Koha/EDI.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Withdrawing the search-engine-error tagging follow-up (attachments 201908/201909). On reflection it doesn't add anything: the code already appends the full, unmodified AddBiblio warning text to the EDIFACT error detail regardless of whether it matches a search-engine signature, so the underlying exception text (e.g. "Search::Elasticsearch::Error::NoNodes...") is already visible in the log. The [SEARCH ENGINE ERROR] prefix was just a label on information that was already there, not new information - anyone triaging could already grep the raw detail for the same signatures. Not worth the added surface area for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201908|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=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201909|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=39215 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply CC| |pedro.amorim@openfifth.co.u | |k -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201379|0 |1 is obsolete| | Attachment #201380|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=39215 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 205164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205164&action=edit Bug 39215: Skip and report EDIFACT quote lines that cannot be processed When loading an EDIFACT quote, problematic data on a single order line (most commonly a mismatched branchcode in the LLO GIR field, or any other enumerated/coded value that breaches a database constraint) caused the whole process_quote run to die. The message was left stuck displaying as 'Processing' on the EDIFACT messages page and the remaining, valid quotes in the file were never imported. This patch makes quote processing resilient: - Each order line is processed inside a try/catch in process_quote, so an unexpected failure is reported and skipped rather than aborting the file. The message always resolves to 'received' or 'error', never 'processing'. - Item creation (Koha::Item->store) is wrapped in try/catch at each of the three creation sites. A failing item is reported through the EDIFACT error infrastructure and skipped, while sibling items and other order lines continue to be processed. - AddBiblio traps its own exceptions, warns them and returns an undef biblionumber. We now capture that warning, surface the underlying cause in the EDIFACT error report and skip the line, rather than carrying on to create an orphan order line against a non-existent biblio. - Database exceptions are translated into a concise, end-user friendly message (e.g. the offending field name) so library staff can identify the problem data. - A basket that contained any unprocessable line is no longer auto-ordered; it is left open with a logged explanation so staff can review and correct the affected lines before ordering manually. A FIXME notes that wrapping the order + item creation in a transaction (to roll a failed line back cleanly) is left to bug 41087 (EDI Retry). Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 205165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205165&action=edit Bug 39215: Unit tests for skipping unprocessable EDIFACT quote lines Adds coverage to t/db_dependent/Koha/EDI.t: - unprocessable_line_handling: a quote whose first line references a non-existent branchcode (LLO+ZZ). Processing completes without dying, the bad line is reported with the offending field, the message status becomes 'error' (not 'processing'), the second valid line is still imported, and auto-ordering is blocked for the affected basket. - biblio_creation_failure_reporting: mocks AddBiblio to fail the way it does in production (warns and returns undef) and confirms the warning is captured into the EDIFACT error report, the line is skipped and no order is created against a missing biblio. Test plan: 1) prove t/db_dependent/Koha/EDI.t 2) Confirm all subtests pass, including the two new ones. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43790 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43790 [Bug 43790] EDIFACT messages Import button always runs the invoice importer, even for quotes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|41087 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41087 [Bug 41087] EDI Retry: Add Rollback and Reprocess actions for EDIFACT messages -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39215 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.11.00 released in| | Status|Passed QA |Pushed to main --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Pushed to main for 26.11 with consent from Pedro whilst he's focusing on reviewing enhancements and new features from the queue -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org