https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42656

--- Comment #11 from Martin Renvoize (ashimema) 
<[email protected]> ---
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.
_______________________________________________
Koha-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to