From ae48ebbcaae50a0af7ceca00d04f2aedb8d5cc2c Mon Sep 17 00:00:00 2001 From: Chris Nighswonger Date: Fri, 12 Nov 2010 09:34:52 -0500 Subject: [PATCH] Bug 5379 - import_borrowers.pl fails with db insert/update errors Some spreadsheet programs use smart quotes which causes the db to throw an error when an insert/update is attempted due to improper processing of the CSV file. This patch adds code to check for smart quotes and change them to "dumb" quotes. This patch also adds more logging of errors and a notice to the user to check the logs for errors when they occur. Signed-off-by: Liz Rea Signed-off-by: Chris Cormack --- C4/Members.pm | 29 ++++++++++++++++++----------- C4/Members/Attributes.pm | 5 +++++ tools/import_borrowers.pl | 10 ++++++++-- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index c6c6985129..4eb9979898 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -772,17 +772,17 @@ sub ModMember { } } my $execute_success=UpdateInTable("borrowers",\%data); -# ok if its an adult (type) it may have borrowers that depend on it as a guarantor -# so when we update information for an adult we should check for guarantees and update the relevant part -# of their records, ie addresses and phone numbers - my $borrowercategory= GetBorrowercategory( $data{'category_type'} ); - if ( exists $borrowercategory->{'category_type'} && $borrowercategory->{'category_type'} eq ('A' || 'S') ) { - # is adult check guarantees; - UpdateGuarantees(%data); + if ($execute_success) { # only proceed if the update was a success + # ok if its an adult (type) it may have borrowers that depend on it as a guarantor + # so when we update information for an adult we should check for guarantees and update the relevant part + # of their records, ie addresses and phone numbers + my $borrowercategory= GetBorrowercategory( $data{'category_type'} ); + if ( exists $borrowercategory->{'category_type'} && $borrowercategory->{'category_type'} eq ('A' || 'S') ) { + # is adult check guarantees; + UpdateGuarantees(%data); + } + logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ arg: $data{'borrowernumber'})") if C4::Context->preference("BorrowersLog"); } - logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ arg: $data{'borrowernumber'})") - if C4::Context->preference("BorrowersLog"); - return $execute_success; } @@ -792,7 +792,9 @@ sub ModMember { $borrowernumber = &AddMember(%borrower); insert new borrower into table -Returns the borrowernumber +Returns the borrowernumber upon success + +Returns as undef upon any db error without further processing =cut @@ -810,10 +812,15 @@ sub AddMember { my $sth = $dbh->prepare("SELECT enrolmentfee FROM categories WHERE categorycode=?"); $sth->execute($data{'categorycode'}); my ($enrolmentfee) = $sth->fetchrow; + if ($sth->err) { + warn sprintf('Database returned the following error: %s', $sth->errstr); + return; + } if ($enrolmentfee && $enrolmentfee > 0) { # insert fee in patron debts manualinvoice($data{'borrowernumber'}, '', '', 'A', $enrolmentfee); } + return $data{'borrowernumber'}; } diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm index 24c6f71a5d..35d67029a4 100644 --- a/C4/Members/Attributes.pm +++ b/C4/Members/Attributes.pm @@ -203,7 +203,12 @@ sub SetBorrowerAttributes { foreach my $attr (@$attr_list) { $attr->{password} = undef unless exists $attr->{password}; $sth->execute($borrowernumber, $attr->{code}, $attr->{value}, $attr->{password}); + if ($sth->err) { + warn sprintf('Database returned the following error: %s', $sth->errstr); + return; # bail immediately on errors + } } + return 1; # borower attributes successfully set } =head2 extended_attributes_code_value_arrayref diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index 6141998a2a..74a58895dd 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -66,7 +66,7 @@ my $columnkeystpl = [ map { {'key' => $_} } grep {$_ ne 'borrowernumber' && $_ my $input = CGI->new(); our $csv = Text::CSV->new({binary => 1}); # binary needed for non-ASCII Unicode -# push @feedback, {feedback=>1, name=>'backend', value=>$csv->backend, backend=>$csv->backend}; +#push @feedback, {feedback=>1, name=>'backend', value=>$csv->backend, backend=>$csv->backend}; #XXX my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ template_name => "tools/import_borrowers.tmpl", @@ -193,6 +193,9 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { } if ($extended) { my $attr_str = $borrower{patron_attributes}; + $attr_str =~ s/\xe2\x80\x9c/"/g; # fixup double quotes in case we are passed smart quotes + $attr_str =~ s/\xe2\x80\x9d/"/g; + push @feedback, {feedback=>1, name=>'attribute string', value=>$attr_str, filename=>$uploadborrowers}; delete $borrower{patron_attributes}; # not really a field in borrowers, so we don't want to pass it to ModMember. $patron_attributes = extended_attributes_code_value_arrayref($attr_str); } @@ -246,6 +249,8 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { } unless (ModMember(%borrower)) { $invalid++; + # untill we have better error trapping, we have no way of knowing why ModMember errored out... + push @errors, {unknown_error => 1}; $template->param('lastinvalid'=>$borrower{'surname'}.' / '.$borrowernumber); next LINE; } @@ -254,7 +259,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { my $old_attributes = GetBorrowerAttributes($borrowernumber); $patron_attributes = extended_attributes_merge($old_attributes, $patron_attributes); #TODO: expose repeatable options in template } - SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes); + push @errors, {unknown_error => 1} unless SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes); } $overwritten++; $template->param('lastoverwritten'=>$borrower{'surname'}.' / '.$borrowernumber); @@ -276,6 +281,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { $template->param('lastimported'=>$borrower{'surname'}.' / '.$borrowernumber); } else { $invalid++; + push @errors, {unknown_error => 1}; $template->param('lastinvalid'=>$borrower{'surname'}.' / AddMember'); } } -- 2.39.5