Bugzilla – Attachment 195494 Details for
Bug 42110
Migrate marc_order_accounts.download_directory to use local file transport option
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 42110: Migrate marc_order_accounts.download_directory to use local file transport option
c5a6bae.patch (text/plain), 22.96 KB, created by
Kyle M Hall (khall)
on 2026-03-16 17:20:00 UTC
(
hide
)
Description:
Bug 42110: Migrate marc_order_accounts.download_directory to use local file transport option
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2026-03-16 17:20:00 UTC
Size:
22.96 KB
patch
obsolete
>From c5a6bae3629a2083d0c110ddcc44fb1cb2edc32e Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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: <description> [<id>] (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 @@ > <select name="transport" id="transport" class="required"> > <option value="ftp">FTP</option> > <option value="sftp" selected="selected">SFTP</option> >+ <option value="local">Local</option> > </select> > <span class="required">Required</span> > </li> >@@ -296,6 +297,11 @@ > [% ELSE %] > <option value="sftp">SFTP</option> > [% END %] >+ [% IF file_transport.transport == 'local' %] >+ <option value="local" selected="selected">Local</option> >+ [% ELSE %] >+ <option value="local">Local</option> >+ [% END %] > </select> > <span class="required">Required</span> > </li> >@@ -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 @@ > <th>Vendor</th> > <th>Budget</th> > <th>Description</th> >- <th>Download directory</th> > <th>Actions</th> > </tr> > [% FOREACH account IN accounts %] >@@ -61,7 +60,6 @@ > <td><a href="/cgi-bin/koha/acquisition/vendors/[% account.vendor_id | uri %]">[% account.vendor.name | html %]</a></td> > <td><a href="/cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% account.budget.budget_period_id | uri %]">[% account.budget.budget_name | html %]</a></td> > <td>[% account.description | html %]</td> >- <td>[% account.download_directory | html %]</td> > <td class="actions"> > <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=acct_form&id=[% account.id | html %]"><i class="fa fa-pencil-alt"></i> Edit</a> > <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=delete_acct&id=[% account.id | html %]"><i class="fa fa-trash-can"></i> Delete</a> >@@ -135,11 +133,6 @@ > [% END %] > </div> > </li> >- <li> >- <label for="download_directory">Download directory: </label> >- <input type="text" name="download_directory" id="download_directory" size="20" value="[% account.download_directory | html %]" /> >- <div class="hint">The download directory specifies the directory in your Koha installation that should be searched for new files.</div> >- </li> > <li> > <label for="match_field">Match field: </label> > <input type="text" name="match_field" id="match_field" size="20" value="[% account.match_field | html %]" /> >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) >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 42110
: 195494