From c5a6bae3629a2083d0c110ddcc44fb1cb2edc32e Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 16 Mar 2026 12:38:05 -0400 Subject: [PATCH] Bug 42110: Migrate marc_order_accounts.download_directory to use local file transport option As detailed in bug 41020, we should be using the pre-existing local file transport for marc order accounts. This offers a simple migration path to using FTP/SFTP after migration. Test Plan: 1) Apply this patch on top of the bug 41020 patches 2) Before running the database update, create marc order accounts: a) Two with different download_directory values set b) One with no download_directory set c) One with a download_directory and a file_transport_id already set 3) Run the database update (installer/data/mysql/atomicupdate/bug_42110.pl) 4) Verify the migration results: a) Accounts from 2a have new local file transports created and linked, named "MARC order account: [] (Migrated)" b) Account from 2b has no file transport created or linked c) Account from 2c is unchanged (existing file_transport_id preserved) d) The download_directory column is removed from marc_order_accounts 5) Rebuild the API bundle: yarn api:bundle 6) Restart all the things! 7) Go to Administration > File transports a) Verify the migrated local file transports appear in the list b) Verify you can create a new file transport with transport type "Local" c) Verify that selecting "Local" disables and clears host, port, passive mode, authentication mode, username, password, and key file fields d) Verify switching back to SFTP/FTP re-enables those fields e) Verify saving a Local transport stores blank values for the disabled fields 8) Go to Administration > MARC order accounts a) Verify the download_directory column and form field are removed b) Verify you can create/edit accounts and select a file transport 9) Test the cron job (misc/cronjobs/marc_ordering_process.pl): a) Place .mrc files in the download directory of a local file transport b) Run: perl marc_ordering_process.pl -v -c Verify files are found and processed via the file transport c) Run with --delete flag, verify source files are deleted d) Run with --rename .done flag, verify source files are renamed e) Run with --archive /tmp/marc_archive flag, verify processed files are copied to the archive directory --- admin/marc_order_accounts.pl | 31 ++-- .../swagger/definitions/file_transport.yaml | 2 +- .../data/mysql/atomicupdate/bug_42110.pl | 52 +++++++ installer/data/mysql/kohastructure.sql | 1 - .../prog/en/modules/admin/file_transports.tt | 36 +++-- .../en/modules/admin/marc_order_accounts.tt | 7 - misc/cronjobs/marc_ordering_process.pl | 141 +++++++++--------- 7 files changed, 161 insertions(+), 109 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_42110.pl diff --git a/admin/marc_order_accounts.pl b/admin/marc_order_accounts.pl index dc97ab2f6bb..02bf1729f7d 100755 --- a/admin/marc_order_accounts.pl +++ b/admin/marc_order_accounts.pl @@ -75,22 +75,21 @@ if ( $op eq 'acct_form' ) { } else { if ( $op eq 'cud-save' ) { my $fields = { - id => scalar $input->param('id'), - description => scalar $input->param('description'), - vendor_id => scalar $input->param('vendor_id'), - budget_id => scalar $input->param('budget_id'), - download_directory => scalar $input->param('download_directory'), - matcher_id => scalar $input->param('matcher'), - overlay_action => scalar $input->param('overlay_action'), - nomatch_action => scalar $input->param('nomatch_action'), - parse_items => scalar $input->param('parse_items'), - item_action => scalar $input->param('item_action'), - record_type => scalar $input->param('record_type'), - encoding => scalar $input->param('encoding') || 'UTF-8', - match_field => scalar $input->param('match_field'), - match_value => scalar $input->param('match_value'), - basket_name_field => scalar $input->param('basket_name_field'), - file_transport_id => scalar $input->param('file_transport_id') || undef, + id => scalar $input->param('id'), + description => scalar $input->param('description'), + vendor_id => scalar $input->param('vendor_id'), + budget_id => scalar $input->param('budget_id'), + matcher_id => scalar $input->param('matcher'), + overlay_action => scalar $input->param('overlay_action'), + nomatch_action => scalar $input->param('nomatch_action'), + parse_items => scalar $input->param('parse_items'), + item_action => scalar $input->param('item_action'), + record_type => scalar $input->param('record_type'), + encoding => scalar $input->param('encoding') || 'UTF-8', + match_field => scalar $input->param('match_field'), + match_value => scalar $input->param('match_value'), + basket_name_field => scalar $input->param('basket_name_field'), + file_transport_id => scalar $input->param('file_transport_id') || undef, }; if ( scalar $input->param('id') ) { diff --git a/api/v1/swagger/definitions/file_transport.yaml b/api/v1/swagger/definitions/file_transport.yaml index 8ad648577f9..ad52c01c46d 100644 --- a/api/v1/swagger/definitions/file_transport.yaml +++ b/api/v1/swagger/definitions/file_transport.yaml @@ -19,7 +19,7 @@ properties: enum: - ftp - sftp - - file + - local description: Transport of File transport (optional) auth_mode: type: string diff --git a/installer/data/mysql/atomicupdate/bug_42110.pl b/installer/data/mysql/atomicupdate/bug_42110.pl new file mode 100755 index 00000000000..ca65cf5cb1c --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_42110.pl @@ -0,0 +1,52 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_success say_info); + +return { + bug_number => "42110", + description => "Migrate marc_order_accounts.download_directory to local type file transport", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + if ( column_exists( 'marc_order_accounts', 'download_directory' ) ) { + my $accounts = $dbh->selectall_arrayref( + q{ + SELECT id, description, download_directory + FROM marc_order_accounts + WHERE download_directory IS NOT NULL + AND download_directory != '' + AND file_transport_id IS NULL + }, + { Slice => {} } + ); + + for my $acct ( @{$accounts} ) { + $dbh->do( + q{ + INSERT INTO file_transports (name, host, port, transport, passive, auth_mode, download_directory, debug) + VALUES (?, 'localhost', 0, 'local', 1, 'noauth', ?, 0) + }, + undef, + "MARC order account: $acct->{description} [$acct->{id}] (Migrated)", + $acct->{download_directory} + ); + my $file_transport_id = $dbh->last_insert_id( undef, undef, 'file_transports', undef ); + + $dbh->do( + q{ UPDATE marc_order_accounts SET file_transport_id = ? WHERE id = ? }, + undef, + $file_transport_id, + $acct->{id} + ) == 1 + && say_success( + $out, + "Migrated download_directory for account '$acct->{description}' to file_transport $file_transport_id" + ); + } + + $dbh->do(q{ ALTER TABLE `marc_order_accounts` DROP COLUMN `download_directory` }); + say_success( $out, "Dropped column 'marc_order_accounts.download_directory'" ); + } + + }, +}; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 80ca8e2f6cc..bef6b296a57 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4666,7 +4666,6 @@ CREATE TABLE `marc_order_accounts` ( `description` varchar(250) NOT NULL COMMENT 'description of this account', `vendor_id` int(11) DEFAULT NULL COMMENT 'vendor id for this account', `budget_id` int(11) DEFAULT NULL COMMENT 'budget id for this account', - `download_directory` mediumtext DEFAULT NULL COMMENT 'download directory for this account', `matcher_id` int(11) DEFAULT NULL COMMENT 'the id of the match rule used (matchpoints.matcher_id)', `overlay_action` varchar(50) DEFAULT NULL COMMENT 'how to handle duplicate records', `nomatch_action` varchar(50) DEFAULT NULL COMMENT 'how to handle records where no match is found', diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt index c804da7d3ea..76971b8d068 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt @@ -146,6 +146,7 @@ Required @@ -296,6 +297,11 @@ [% ELSE %] [% END %] + [% IF file_transport.transport == 'local' %] + + [% ELSE %] + + [% END %] Required @@ -689,19 +695,15 @@ $("#confirm_key_accept_submit").on("click", function (event) { event.preventDefault(); - if ($("#add").length > 0) { - if ($("#add").valid() == true) { - modalChange(); - $("#confirm_key_accept").modal("show"); - } else { - $("#confirm_key_accept").modal("hide"); - } - } - - if ($("#edit_save").length > 0) { - if ($("#edit_save").valid() == true) { - modalChange(); - $("#confirm_key_accept").modal("show"); + var form = $("#add").length > 0 ? $("#add") : $("#edit_save"); + if (form.length > 0) { + if (form.valid() == true) { + if ($("#transport").val() == "local") { + form.submit(); + } else { + modalChange(); + $("#confirm_key_accept").modal("show"); + } } else { $("#confirm_key_accept").modal("hide"); } @@ -762,6 +764,14 @@ if (port == 21) $("#port").val("22"); return authModeChange(); + } else if (transport.val() == "local") { + $("#host").val("").attr("disabled", "disabled"); + $("#port").val("").attr("disabled", "disabled"); + $("#passive").val("1").attr("disabled", "disabled"); + $("#auth_mode").val("noauth").attr("disabled", "disabled"); + $("#user_name").val("").attr("disabled", "disabled"); + $("#password").val("").attr("disabled", "disabled"); + $("#key_file").val("").attr("disabled", "disabled"); } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt index adfe590a5de..82757c038b8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt @@ -52,7 +52,6 @@ Vendor Budget Description - Download directory Actions [% FOREACH account IN accounts %] @@ -61,7 +60,6 @@ [% account.vendor.name | html %] [% account.budget.budget_name | html %] [% account.description | html %] - [% account.download_directory | html %] Edit Delete @@ -135,11 +133,6 @@ [% END %] -
  • - - -
    The download directory specifies the directory in your Koha installation that should be searched for new files.
    -
  • diff --git a/misc/cronjobs/marc_ordering_process.pl b/misc/cronjobs/marc_ordering_process.pl index 27327c21c44..fbd0f7c551f 100755 --- a/misc/cronjobs/marc_ordering_process.pl +++ b/misc/cronjobs/marc_ordering_process.pl @@ -23,7 +23,7 @@ marc_ordering_process.pl - cron script to retrieve MARC files and create order l =head1 SYNOPSIS -./marc_ordering_process.pl [-c|--confirm] [-v|--verbose] [--dr|--delete-remote] [--rr|--rename-remote] ext] +./marc_ordering_process.pl [-c|--confirm] [-v|--verbose] [-d|--delete] [-r|--rename ext] or, in crontab: # Once every day @@ -31,8 +31,8 @@ or, in crontab: =head1 DESCRIPTION -This script searches for new MARC files in an SFTP location -If there are new files, it stages those files, adds bilbios/items and creates order lines +This script searches for new MARC files via configured file transports, +stages those files, adds biblios/items and creates order lines. =head1 OPTIONS @@ -48,15 +48,15 @@ Without this parameter no changes will be made =item B<-d|--delete> -Delete the file once it has been processed +Delete the remote source file once it has been processed -=item B<--dr|--delete-remote> +=item B<-r|--rename ext> -Delete the remote file once it has been downloaded +Rename the remote source file once it has been processed, adding the given file extension -=item B<-rr|--rename-remove ext> +=item B<-a|--archive dir> -Rename the remote file once it has been downloaded, adding the given file extension +Copy downloaded files into the specified local archive directory after processing =back @@ -65,7 +65,8 @@ Rename the remote file once it has been downloaded, adding the given file extens use Modern::Perl; use Pod::Usage qw( pod2usage ); use Getopt::Long qw( GetOptions ); -use File::Copy qw( copy move ); +use File::Copy qw( copy ); +use File::Temp qw( tempdir ); use Koha::Script -cron; use Koha::MarcOrder; @@ -76,14 +77,14 @@ use C4::Log qw( cronlogaction ); my $command_line_options = join( " ", @ARGV ); cronlogaction( { info => $command_line_options } ); -my ( $help, $verbose, $confirm, $delete, $rename_ext, $delete_remote ); +my ( $help, $verbose, $confirm, $delete, $rename_ext, $archive_dir ); GetOptions( - 'h|help' => \$help, - 'v|verbose' => \$verbose, - 'c|confirm' => \$confirm, - 'd|delete' => \$delete, - 'dr|delete-remote' => \$delete_remote, - 'rr|rename-remote=s' => \$rename_ext, + 'h|help' => \$help, + 'v|verbose' => \$verbose, + 'c|confirm' => \$confirm, + 'd|delete' => \$delete, + 'r|rename=s' => \$rename_ext, + 'a|archive=s' => \$archive_dir, ) || pod2usage(1); pod2usage(0) if $help; @@ -104,70 +105,64 @@ if ( scalar(@accounts) == 0 ) { my $valid_file_extensions = qr/\.(mrc|marcxml|mrk)$/i; foreach my $acct (@accounts) { + my $file_transport = $acct->file_transport; + unless ($file_transport) { + say "No file transport configured for account: " . $acct->description if $verbose; + next; + } + + my $working_dir = $file_transport->download_directory; + unless ($working_dir) { + say "No download directory configured for file transport: " . $file_transport->id if $verbose; + next; + } + if ($verbose) { say sprintf "Starting MARC ordering process for %s", $acct->vendor->name; - say sprintf "Looking for new files in %s", $acct->download_directory; + say sprintf "Looking for new files in %s", $working_dir; } - my $working_dir = $acct->download_directory; - - my $file_transport = $acct->file_transport; - if ($file_transport) { - if ( $file_transport->connect() ) { - my $download_dir = $file_transport->download_directory; + unless ( $file_transport->connect() ) { + say "Failed to connect to file transport: " . $file_transport->id; + next; + } - my $success = $download_dir ? $file_transport->change_directory($download_dir) : 1; - if ( $download_dir && !$success ) { - say "Failed to change to download directory: $download_dir"; - } else { + my $success = $file_transport->change_directory($working_dir); + if ( !$success ) { + say "Failed to change to download directory: $working_dir"; + next; + } - # Get file list - my $file_list = $file_transport->list_files(); - if ($file_list) { - - # Process files matching our criteria - foreach my $file ( @{$file_list} ) { - my $filename = $file->{filename}; - - if ( $filename =~ $valid_file_extensions ) { - - my $local_file = "$working_dir/$filename"; - - # Download the file - if ( $file_transport->download_file( $filename, $local_file ) ) { - if ($rename_ext) { - $file_transport->rename_file( $filename, "$filename.$rename_ext" ); - } elsif ($delete_remote) { - $file_transport->delete_file($filename); - } - } else { - say "Failed to download file: $filename"; - } - } - } - - } else { - say "Failed to get file list from transport"; - } + # Get file list + my $file_list = $file_transport->list_files(); + unless ($file_list) { + say "Failed to get file list from transport"; + next; + } - } - } else { - say "Failed to connect to file transport: " . $file_transport->id; - } + my @files; + foreach my $file ( @{$file_list} ) { + my $filename = $file->{filename}; + next unless $filename =~ $valid_file_extensions; + push @files, $filename; } - opendir my $dir, $working_dir or die "Can't open filepath"; - my @files = grep { /$valid_file_extensions/ } readdir $dir; - closedir $dir; print "No new files found\n" if scalar(@files) == 0; my $files_processed = 0; + my $local_dir = tempdir( CLEANUP => 1 ); foreach my $filename (@files) { - my $full_path = "$working_dir/$filename"; - my $args = { + my $local_file = "$local_dir/$filename"; + + unless ( $file_transport->download_file( $filename, $local_file ) ) { + say "Failed to download file: $filename"; + next; + } + + my $args = { filename => $filename, - filepath => $full_path, + filepath => $local_file, profile => $acct, agent => 'cron' }; @@ -182,13 +177,17 @@ foreach my $acct (@accounts) { if ( $result->{success} ) { $files_processed++; say sprintf "Successfully processed file: %s", $filename if $verbose; + if ($archive_dir) { + mkdir $archive_dir unless -d $archive_dir; + say sprintf "Archiving file: %s to %s", $filename, $archive_dir if $verbose; + copy( $local_file, "$archive_dir/$filename" ); + } if ($delete) { - say sprintf "Deleting processed file: %s", $filename if $verbose; - unlink $full_path; - } else { - mkdir "$working_dir/archive" unless -d "$working_dir/archive"; - say sprintf "Moving file to archive: %s", $filename if $verbose; - move( $full_path, "$working_dir/archive/$filename" ); + say sprintf "Deleting file: %s", $filename if $verbose; + $file_transport->delete_file($filename); + } elsif ($rename_ext) { + say sprintf "Renaming file: %s to %s.%s", $filename, $filename, $rename_ext if $verbose; + $file_transport->rename_file( $filename, "$filename.$rename_ext" ); } } else { say sprintf "Error processing file: %s", $filename if $verbose; -- 2.50.1 (Apple Git-155)