Bugzilla – Attachment 4486 Details for
Bug 5379
import_borrowers.pl fails with db insert/update errors
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED OFF] Bug 5379 - import_borrowers.pl fails with db insert/update errors
SIGNED-OFF-Bug-5379---importborrowerspl-fails-with.patch (text/plain), 6.85 KB, created by
Liz Rea
on 2011-06-15 16:55:06 UTC
(
hide
)
Description:
[SIGNED OFF] Bug 5379 - import_borrowers.pl fails with db insert/update errors
Filename:
MIME Type:
Creator:
Liz Rea
Created:
2011-06-15 16:55:06 UTC
Size:
6.85 KB
patch
obsolete
>From dc3361fd64400705f5b25b2cf416fd25ffa712cb Mon Sep 17 00:00:00 2001 >From: Chris Nighswonger <cnighswonger@foundations.edu> >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
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 5379
:
2742
|
2755
|
2760
| 4486