We currently log a lot of detail using the editrace log, but it all goes onto server side only files. We should do similar to what we do for email delivery failures and log to a table and display user friendly errors in the UI so that end users are a) aware of such issues and b) may be able to resolve such issue's by updating their config and/or mappings.
Created attachment 175788 [details] [review] Bug 38689: Add edifact_errors table
Created attachment 175789 [details] [review] Bug 38689: DBIC Schema Update
Created attachment 175790 [details] [review] Bug 38689: Record errors on invoice processing
Created attachment 175793 [details] [review] Bug 38689: DBIC Schema Update
Created attachment 175794 [details] [review] Bug 38689: Record errors on invoice processing
Created attachment 175795 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 175796 [details] [review] Bug 38689: Add basic rendering for errors in edifact messages display
This is an initial proof of concept.. we add a new table to store edifact processing errors and then we expose those on the API and finally in the edifact messages page for display. Currently, I've only dealt with basic processing errors that appear for invoices as a proof of concept and we could also do with working on the display of errors which is currently a tooltip on hover for the new 'with errors' that appears in the status column. I'm not really looking for a signoff here yet.. more looking for input on the approach
Created attachment 175797 [details] Current state of display
Created attachment 175809 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message.
Created attachment 175810 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message.
Created attachment 175811 [details] Updated display using Modal
Created attachment 175812 [details] [review] Bug 38689: Add edifact_errors table
Created attachment 175813 [details] [review] Bug 38689: DBIC Schema Update
Created attachment 175814 [details] [review] Bug 38689: Record errors on invoice processing
Created attachment 175815 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 175816 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message.
Test plan The simplest way to test this is to comment the database rollback line in the EDI.t unit test and run the unit test to generate some errors and leave them in the database. Then you can view them by enabling the Edifact system preference and navigating to Acquisitions > EDI Messages.
(In reply to Martin Renvoize (ashimema) from comment #8) > I'm not really looking for a signoff here yet.. more looking for input on > the approach Making problems easily visible would be good. We have a report on our Acquisitions homepage that tries to find problems with EDI but if staff don't run the report, nothing gets done. This bug triggered me running that report and I've seen a recent QUOTE message that errored, and the titles in it are not in our database, suggesting the report isn't routinely being run. (Maybe I should have used the Patron Emailer or runreport.pl in some way to routinely get problems made visible!).
Created attachment 175817 [details] [review] Bug 38689: Update unit test We switch out the carp for a logger call in process_invoice, so we need ot update the unit test to compensate for that.
Created attachment 176127 [details] [review] Bug 38689: Add edifact_errors table
Created attachment 176128 [details] [review] Bug 38689: DBIC Schema Update
Created attachment 176129 [details] [review] Bug 38689: Record errors on invoice processing
Created attachment 176130 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176131 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message.
Created attachment 176132 [details] [review] Bug 38689: Update unit test We switch out the carp for a logger call in process_invoice, so we need ot update the unit test to compensate for that.
Created attachment 176133 [details] [review] Bug 38689: Record errors on quote processing
Created attachment 176134 [details] [review] Bug 38689: Add logging tests for process_quote This adds missing tests for the Koha::Logger based logging of messages in the process_quote process. We also tidy up the log lines a tiny bit for consistency.
Created attachment 176173 [details] [review] Bug 38689: Add edifact_errors table
Created attachment 176174 [details] [review] Bug 38689: DBIC Schema Update
Created attachment 176175 [details] [review] Bug 38689: Record errors on invoice processing
Created attachment 176176 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176177 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message.
Created attachment 176178 [details] [review] Bug 38689: Update unit test We switch out the carp for a logger call in process_invoice, so we need ot update the unit test to compensate for that.
Created attachment 176179 [details] [review] Bug 38689: Record errors on quote processing
Created attachment 176180 [details] [review] Bug 38689: Add logging tests for process_quote This adds missing tests for the Koha::Logger based logging of messages in the process_quote process. We also tidy up the log lines a tiny bit for consistency.
Created attachment 176181 [details] [review] Bug 38689: Improve logging of section We attempt to add the relevant part of the EDI message to the 'section' field of the edifact_errors record.
Created attachment 176182 [details] [review] Bug 38689: Add section to output
Created attachment 176183 [details] Display of errors
Let me know what you think of the two output options.. one including the section of the edifact message that's throwing the error or the original without the message line split out.
Created attachment 176373 [details] [review] Bug 38689: Add edifact_errors table Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176374 [details] [review] Bug 38689: DBIC Schema Update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176375 [details] [review] Bug 38689: Record errors on invoice processing Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176376 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176377 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176378 [details] [review] Bug 38689: Update unit test We switch out the carp for a logger call in process_invoice, so we need ot update the unit test to compensate for that. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176379 [details] [review] Bug 38689: Record errors on quote processing Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176380 [details] [review] Bug 38689: Add logging tests for process_quote This adds missing tests for the Koha::Logger based logging of messages in the process_quote process. We also tidy up the log lines a tiny bit for consistency. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176381 [details] [review] Bug 38689: Improve logging of section We attempt to add the relevant part of the EDI message to the 'section' field of the edifact_errors record. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176382 [details] [review] Bug 38689: Add section to output Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176524 [details] [review] Bug 38689: Add edifact_errors table Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176525 [details] [review] Bug 38689: DBIC Schema Update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176526 [details] [review] Bug 38689: Record errors on invoice processing Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> error message improvement
Created attachment 176527 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176528 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176529 [details] [review] Bug 38689: Update unit test We switch out the carp for a logger call in process_invoice, so we need ot update the unit test to compensate for that. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176530 [details] [review] Bug 38689: Record errors on quote processing Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176531 [details] [review] Bug 38689: Add logging tests for process_quote This adds missing tests for the Koha::Logger based logging of messages in the process_quote process. We also tidy up the log lines a tiny bit for consistency. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176532 [details] [review] Bug 38689: Add section to output We attempt to add the relevant part of the EDI message to the 'section' field of the edifact_errors record and then display this alongside the error details. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176533 [details] [review] Bug 38689: Improve test coverage This patch drastically enhances the existing tests. We add tests for the various codepaths for handling EDI QUOTES including LIN segment splitting for multi-fund orders.
Created attachment 176605 [details] [review] Bug 38689: Add edifact_errors table Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176606 [details] [review] Bug 38689: DBIC Schema Update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176607 [details] [review] Bug 38689: Record errors on invoice processing Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> error message improvement
Created attachment 176608 [details] [review] Bug 38689: Add Koha::Edifact::File::Error(s) classes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176609 [details] [review] Bug 38689: Display errors in a modal This patch adds a `(with errors)` link next to the status of any edifact messages that produced errors on processing. The link will popup a modal containing the error details recorded for that message. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176610 [details] [review] Bug 38689: Update unit test We switch out the carp for a logger call in process_invoice, so we need ot update the unit test to compensate for that. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176611 [details] [review] Bug 38689: Record errors on quote processing Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176612 [details] [review] Bug 38689: Add logging tests for process_quote This adds missing tests for the Koha::Logger based logging of messages in the process_quote process. We also tidy up the log lines a tiny bit for consistency. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176613 [details] [review] Bug 38689: Add section to output We attempt to add the relevant part of the EDI message to the 'section' field of the edifact_errors record and then display this alongside the error details. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 176614 [details] [review] Bug 38689: Improve test coverage This patch drastically enhances the existing tests. We add tests for the various codepaths for handling EDI QUOTES including LIN segment splitting for multi-fund orders.
Fully covered by tests now :)