https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43663 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- 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 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.