https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42656 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 202927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202927&action=edit Bug 42656: (QA follow-up) Make error status persistence consistent, with a full trace, across FTP, SFTP and Local Two related inconsistencies remained across the three transport backends' error-handling paths: 1) Only Koha::File::Transport::SFTP's _abort_operation() persisted an "errors" status snapshot to the status column on every failed operation. FTP never did, and Local had no _abort_operation() at all - so an SFTP transport's displayed status could reflect a single transient failure from real usage (e.g. an EDI cron job) while the exact same failure on FTP/Local left the stored status untouched (stale, possibly still showing the last explicit test_connection() result). This is exactly the kind of unclear status reporting bug 42656 is about. 2) FTP's _abort_operation() carried its extra detail under an operation_params key, while SFTP (and Local's plain add_message calls) used path. The change_directory() success payload had the same split: FTP used directory, SFTP and Local used path. The admin template's change_directory row display depended on FTP's mismatched key names, which meant it silently failed to show the tested directory for SFTP transports. Adds a shared _record_error($operation, \%payload) to the Koha::File::Transport base class - add_message() plus status persistence - and routes all three backends' _abort_operation() (Local gains one) through it. Renames FTP's operation_params/directory keys to path throughout, matching SFTP and Local, and simplifies the template to read operation.detail.path unconditionally instead of ORing two differently-named keys. _record_error() persists the full trace of operations recorded on the transport object so far this session (every success leading up to the failure), not just the failing operation in isolation - the same "full trace" shape TestTransport::process() already builds for an explicit test_connection() run. A long-lived transport used across a multi-step operation (e.g. an EDI download loop touching several files before one fails) will therefore show every successful step leading up to the failure. It also logs the failure - including the built trace - via Koha::Logger at warn level, so it's visible in the Koha logs without needing to query the transport's status column. Test plan: 1. prove -r t/db_dependent/Koha/File/ t/db_dependent/Koha/BackgroundJob/TestTransport.t t/db_dependent/Koha/Edifact/Transport.t t/db_dependent/api/v1/file_transports.t => All tests successful (48 tests total), including: - new "_abort_operation persists status consistently" subtests in FTP.t, SFTP.t and Local.t that reload the transport from the DB after a simulated failure and confirm the status column was updated with the correct operation code and path detail; - a new "_record_error() tests" subtest in Transport.t proving the persisted trace includes a prior success message plus the failure (not just the failure alone), and that a warn-level log line mentioning the operation and error was emitted. 2. koha-qa.pl -v 2 --more-tests => [PASS], no critic/pod warnings. -- You are receiving this mail because: You are watching all bug changes.