From 32a1b75845c5e7668a59cdea55908eb8cf6a7e60 Mon Sep 17 00:00:00 2001 From: Jake Deery Date: Wed, 4 Dec 2024 09:43:31 +0000 Subject: [PATCH] Bug 38489: (follow-up) tidy EDIfact files This patch follows-up by by tidying the following files: * Koha/Edifact/Transport.pm * admin/edi_accounts.pl --- Koha/Edifact/Transport.pm | 113 +++++++++++++++++++++----------------- admin/edi_accounts.pl | 49 ++++++++--------- 2 files changed, 85 insertions(+), 77 deletions(-) diff --git a/Koha/Edifact/Transport.pm b/Koha/Edifact/Transport.pm index 64bd0f8997e..182d2c1d671 100644 --- a/Koha/Edifact/Transport.pm +++ b/Koha/Edifact/Transport.pm @@ -23,9 +23,9 @@ use utf8; use Carp qw( carp ); use DateTime; -use Encode qw( from_to ); -use English qw{ -no_match_vars }; -use File::Copy qw( copy move ); +use Encode qw( from_to ); +use English qw{ -no_match_vars }; +use File::Copy qw( copy move ); use File::Slurp qw( read_file ); use Net::FTP; use Net::SFTP::Foreign; @@ -40,9 +40,9 @@ sub new { my $schema = $database->schema(); my $acct = $schema->resultset('VendorEdiAccount')->find($account_id); my $self = { - account => $acct, - schema => $schema, - working_dir => C4::Context::temporary_directory, #temporary work directory + account => $acct, + schema => $schema, + working_dir => C4::Context::temporary_directory, #temporary work directory transfer_date => dt_from_string(), }; @@ -65,7 +65,7 @@ sub download_messages { my $sftp_server_id = $self->{account}->download_sftp_server_id; my $sftp_server_transport; $sftp_server_transport = Koha::SFTP::Servers->find($sftp_server_id)->transport - if ( $sftp_server_id ); + if ($sftp_server_id); my @retrieved_files; @@ -88,7 +88,7 @@ sub upload_messages { my $sftp_server_id = $self->{account}->upload_sftp_server_id; my $sftp_server_transport; $sftp_server_transport = Koha::SFTP::Servers->find($sftp_server_id)->transport - if ( $sftp_server_id ); + if ($sftp_server_id); if (@messages) { if ( !$sftp_server_transport ) { @@ -109,8 +109,8 @@ sub file_download { my $file_ext = _get_file_ext( $self->{message_type} ); - my $dir = $self->{account}->download_directory; # makes code more readable - # C = ready to retrieve E = Edifact + my $dir = $self->{account}->download_directory; # makes code more readable + # C = ready to retrieve E = Edifact my $msg_hash = $self->message_hash(); if ( opendir my $dh, $dir ) { my @file_list = readdir $dh; @@ -119,8 +119,7 @@ sub file_download { if ( $filename =~ m/[.]$file_ext$/ ) { if ( copy( "$dir/$filename", $self->{working_dir} ) ) { - } - else { + } else { carp "copy of $filename failed"; next; } @@ -131,8 +130,7 @@ sub file_download { } } $self->ingest( $msg_hash, @downloaded_files ); - } - else { + } else { carp "Cannot open $dir"; return; } @@ -162,23 +160,31 @@ sub sftp_download { timeout => 10, ); if ( $sftp->error ) { - return $self->_abort_download( undef, - 'Unable to connect to remote host: ' . $sftp->error ); + return $self->_abort_download( + undef, + 'Unable to connect to remote host: ' . $sftp->error + ); } $sftp->setcwd( $self->{account}->download_directory ) - or return $self->_abort_download( $sftp, - "Cannot change remote dir: " . $sftp->error ); + or return $self->_abort_download( + $sftp, + "Cannot change remote dir: " . $sftp->error + ); my $file_list = $sftp->ls() - or return $self->_abort_download( $sftp, - "cannot get file list from server: " . $sftp->error ); + or return $self->_abort_download( + $sftp, + "cannot get file list from server: " . $sftp->error + ); foreach my $file ( @{$file_list} ) { my $filename = $file->{filename}; if ( $filename =~ m/[.]$file_ext$/ ) { $sftp->get( $filename, "$self->{working_dir}/$filename" ); if ( $sftp->error ) { - $self->_abort_download( $sftp, - "Error retrieving $filename: " . $sftp->error ); + $self->_abort_download( + $sftp, + "Error retrieving $filename: " . $sftp->error + ); last; } push @downloaded_files, $filename; @@ -188,8 +194,10 @@ sub sftp_download { #$sftp->atomic_rename( $filename, $processed_name ); my $ret = $sftp->rename( $filename, $processed_name ); if ( !$ret ) { - $self->_abort_download( $sftp, - "Error renaming $filename: " . $sftp->error ); + $self->_abort_download( + $sftp, + "Error renaming $filename: " . $sftp->error + ); last; } @@ -206,16 +214,14 @@ sub ingest { foreach my $f (@downloaded_files) { # Check file has not been downloaded already - my $existing_file = $self->{schema}->resultset('EdifactMessage') - ->find( { filename => $f, } ); + my $existing_file = $self->{schema}->resultset('EdifactMessage')->find( { filename => $f, } ); if ($existing_file) { carp "skipping ingest of $f : filename exists"; next; } $msg_hash->{filename} = $f; - my $file_content = - read_file( "$self->{working_dir}/$f", binmode => ':raw' ); + my $file_content = read_file( "$self->{working_dir}/$f", binmode => ':raw' ); if ( !defined $file_content ) { carp "Unable to read download file $f"; next; @@ -243,7 +249,7 @@ sub ftp_download { my $msg_hash = $self->message_hash(); my @downloaded_files; - my $ftp = Net::FTP->new( + my $ftp = Net::FTP->new( $sftp_server->host, Port => $sftp_server->port, Timeout => 10, @@ -254,21 +260,24 @@ sub ftp_download { "Cannot connect to " . $sftp_server->host . ": " . $EVAL_ERROR ); $ftp->login( $sftp_server->user_name, $sftp_server->plain_text_password ) - or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" ); + or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" ); $ftp->cwd( $self->{account}->download_directory ) - or return $self->_abort_download( $ftp, - "Cannot change remote dir : $ftp->message()" ); + or return $self->_abort_download( + $ftp, + "Cannot change remote dir : $ftp->message()" + ); my $file_list = $ftp->ls() - or - return $self->_abort_download( $ftp, 'cannot get file list from server' ); + or return $self->_abort_download( $ftp, 'cannot get file list from server' ); foreach my $filename ( @{$file_list} ) { if ( $filename =~ m/[.]$file_ext$/ ) { if ( !$ftp->get( $filename, "$self->{working_dir}/$filename" ) ) { - $self->_abort_download( $ftp, - "Error retrieving $filename: $ftp->message" ); + $self->_abort_download( + $ftp, + "Error retrieving $filename: $ftp->message" + ); last; } @@ -295,7 +304,7 @@ sub ftp_upload { return unless $sftp_server; - my $ftp = Net::FTP->new( + my $ftp = Net::FTP->new( $sftp_server->host, Port => $sftp_server->port, Timeout => 10, @@ -306,10 +315,12 @@ sub ftp_upload { "Cannot connect to " . $sftp_server->host . ": " . $EVAL_ERROR ); $ftp->login( $sftp_server->user_name, $sftp_server->plain_text_password ) - or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" ); + or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" ); $ftp->cwd( $self->{account}->upload_directory ) - or return $self->_abort_download( $ftp, - "Cannot change remote dir : $ftp->message()" ); + or return $self->_abort_download( + $ftp, + "Cannot change remote dir : $ftp->message()" + ); foreach my $m (@messages) { my $content = $m->raw_msg; if ($content) { @@ -319,8 +330,8 @@ sub ftp_upload { $m->transfer_date( $self->{transfer_date} ); $m->status('sent'); $m->update; - } - else { + } else { + # error in transfer } @@ -348,10 +359,12 @@ sub sftp_upload { port => $sftp_server->port, timeout => 10, ); - $sftp->die_on_error("Cannot ssh to " . $sftp_server->host); + $sftp->die_on_error( "Cannot ssh to " . $sftp_server->host ); $sftp->setcwd( $self->{account}->upload_directory ) - or return $self->_abort_download( $sftp, - "Cannot change remote dir : " . $sftp->error ); + or return $self->_abort_download( + $sftp, + "Cannot change remote dir : " . $sftp->error + ); foreach my $m (@messages) { my $content = $m->raw_msg; if ($content) { @@ -361,8 +374,8 @@ sub sftp_upload { $m->transfer_date( $self->{transfer_date} ); $m->status('sent'); $m->update; - } - else { + } else { + # error in transfer } @@ -388,15 +401,13 @@ sub file_upload { $m->transfer_date( $self->{transfer_date} ); $m->status('sent'); $m->update; - } - else { + } else { carp "Could not transfer $m->filename : $ERRNO"; next; } } } - } - else { + } else { carp "Upload directory $dir does not exist"; } return; diff --git a/admin/edi_accounts.pl b/admin/edi_accounts.pl index 90514149d21..d474992add6 100755 --- a/admin/edi_accounts.pl +++ b/admin/edi_accounts.pl @@ -21,20 +21,20 @@ use Modern::Perl; use CGI; -use C4::Auth qw( get_template_and_user ); +use C4::Auth qw( get_template_and_user ); use C4::Output qw( output_html_with_http_headers ); use Koha::Database; use Koha::Plugins; -our $input = CGI->new(); +our $input = CGI->new(); our $schema = Koha::Database->new()->schema(); our ( $template, $loggedinuser, $cookie ) = get_template_and_user( { - template_name => 'admin/edi_accounts.tt', - query => $input, - type => 'intranet', - flagsrequired => { acquisition => 'edi_manage' }, + template_name => 'admin/edi_accounts.tt', + query => $input, + type => 'intranet', + flagsrequired => { acquisition => 'edi_manage' }, } ); @@ -47,7 +47,7 @@ if ( $op eq 'acct_form' ) { my @vendors = $schema->resultset('Aqbookseller')->search( undef, { - columns => [ 'name', 'id' ], + columns => [ 'name', 'id' ], order_by => { -asc => 'name' } } ); @@ -63,35 +63,35 @@ if ( $op eq 'acct_form' ) { $template->param( sftp_servers => \@sftp_servers ); if ( C4::Context->config("enable_plugins") ) { - my @plugins = Koha::Plugins->new()->GetPlugins({ - method => 'edifact', - }); + my @plugins = Koha::Plugins->new()->GetPlugins( + { + method => 'edifact', + } + ); $template->param( plugins => \@plugins ); } -} -elsif ( $op eq 'delete_confirm' ) { +} elsif ( $op eq 'delete_confirm' ) { show_account(); $template->param( delete_confirm => 1 ); -} -else { +} else { if ( $op eq 'cud-save' ) { # validate & display my $id = $input->param('id'); my $fields = { description => scalar $input->param('description'), - upload_sftp_server_id => $input->param('upload_sftp_server_id') || undef, + upload_sftp_server_id => $input->param('upload_sftp_server_id') || undef, download_sftp_server_id => $input->param('download_sftp_server_id') || undef, vendor_id => scalar $input->param('vendor_id'), upload_directory => scalar $input->param('upload_directory'), download_directory => scalar $input->param('download_directory'), san => scalar $input->param('san'), standard => scalar $input->param('standard'), - quotes_enabled => $input->param('quotes_enabled') ? 1 : 0, - invoices_enabled => $input->param('invoices_enabled') ? 1 : 0, - orders_enabled => $input->param('orders_enabled') ? 1 : 0, + quotes_enabled => $input->param('quotes_enabled') ? 1 : 0, + invoices_enabled => $input->param('invoices_enabled') ? 1 : 0, + orders_enabled => $input->param('orders_enabled') ? 1 : 0, responses_enabled => $input->param('responses_enabled') ? 1 : 0, - auto_orders => $input->param('auto_orders') ? 1 : 0, + auto_orders => $input->param('auto_orders') ? 1 : 0, id_code_qualifier => scalar $input->param('id_code_qualifier'), plugin => scalar $input->param('plugin'), }; @@ -102,15 +102,12 @@ else { id => $id, } )->update_all($fields); - } - else { # new record + } else { # new record $schema->resultset('VendorEdiAccount')->create($fields); } - } - elsif ( $op eq 'cud-delete_confirmed' ) { + } elsif ( $op eq 'cud-delete_confirmed' ) { - $schema->resultset('VendorEdiAccount') - ->search( { id => scalar $input->param('id'), } )->delete_all; + $schema->resultset('VendorEdiAccount')->search( { id => scalar $input->param('id'), } )->delete_all; } # we do a default dispaly after deletes and saves @@ -167,6 +164,6 @@ sub show_account { my $acct = $schema->resultset('VendorEdiAccount')->find($acct_id); return unless $acct; - + return $template->param( account => $acct ); } -- 2.43.0