From dc3361fd64400705f5b25b2cf416fd25ffa712cb Mon Sep 17 00:00:00 2001 From: Chris Nighswonger Date: Fri, 12 Nov 2010 09:34:52 -0500 Subject: [PATCH] [SIGNED OFF] Bug 5379 - import_borrowers.pl fails with db insert/update errors Content-Type: text/plain; charset="utf-8" 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. --- 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 e03b40b..7cd88af 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 24c6f71..35d6702 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 6141998..74a5889 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'); } } -- 1.7.2.5