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

--- Comment #2 from Martin Renvoize (ashimema) 
<[email protected]> ---
Created attachment 206953
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206953&action=edit
Bug 43663: Migrate Koha::File::Transports onto the polymorphic mixin

Koha::File::Transports already hand-rolled the same pattern the new
mixin generalises: a static _polymorphic_map built by hand, with each
subclass's dispatch key duplicated as a literal string both in that
map and inside the subclass's own code.

Migrates it and its three subclasses (SFTP, FTP, Local) onto
Koha::Objects::Mixin::Polymorphic:

- Koha::File::Transports now declares _polymorphic_classes (an
  explicit, reviewed list - unchanged), and composes the mixin via
  `use parent`. It doesn't declare _polymorphic_base_class: the
  mixin's default (inferred from Koha::Schema::Result::FileTransport's
  own koha_object_class) already resolves to the right answer,
  'Koha::File::Transport'.
- Each subclass self-declares its own dispatch key via
  _polymorphic_key, rather than that key living only in the plural
  class's map.
- object_class is still defined directly on Koha::File::Transports,
  delegating to _polymorphic_object_class - see the mixin's own POD
  for why that one line can't just be inherited.

The previous object_class used lc() on the transport value; dropped
here as unnecessary, since both the admin template's <select> options
and the REST swagger enum only ever produce lowercase values.

No new test is added here: the mixin's own contract is already covered
by t/db_dependent/Koha/Objects/Mixin/Polymorphic.t (previous commit),
and the full existing File::Transport test suite (Transport.t,
Transports.t, FTP.t, Local.t, SFTP.t) passes unmodified against this
migration, confirming the mixin is a drop-in replacement for the
hand-rolled version.

This is an internal refactor with no intended behaviour change, so the
test plan below is about confirming that: Administration > File
transports should look and work exactly as it did before.

Test plan:
1. Apply the patches and restart_all.
2. Go to Administration > File transports
   (/cgi-bin/koha/admin/file_transports.pl). If you already have any
   FTP/SFTP file transports configured, confirm the list still
   displays them correctly - before this fix, listing existing
   transports through the mixin failed outright with "Can't call
   method _new_from_dbic on an undefined value".
3. Click "New file transport", fill in the required fields, choose
   "FTP" as the Transport, and Save.
   - Confirm it appears in the list with Transport shown as "FTP".
   - Saving triggers an automatic connection test in the background;
     if you have a real FTP server to point it at, confirm the list
     eventually shows "Tests passing" for that row (or "Tests
     failing" with a sensible reason if the server is unreachable -
     either way, confirms an FTP-specific connection attempt actually
     ran, i.e. the row correctly dispatched to
     Koha::File::Transport::FTP).
4. Repeat step 3 choosing "SFTP" instead, confirming the row
   dispatches to Koha::File::Transport::SFTP.
5. Edit one of the transports created above (e.g. change the host)
   and Save again. Confirm the edit form is repopulated correctly and
   the change saves without error.
6. Delete one of the test transports created above and confirm it's
   removed from the list.

Expected result throughout: no visible difference from the feature's
behaviour prior to this patch.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

-- 
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