Bugzilla – Attachment 52324 Details for
Bug 12598
New misc/import_borrowers.pl command line tool
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12598 - Tidy import_borrowers.pl
Bug-12598---Tidy-importborrowerspl.patch (text/plain), 21.69 KB, created by
Kyle M Hall (khall)
on 2016-06-13 16:14:20 UTC
(
hide
)
Description:
Bug 12598 - Tidy import_borrowers.pl
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2016-06-13 16:14:20 UTC
Size:
21.69 KB
patch
obsolete
>From 3d4dcf6b1b2ea3c14cea2b8b446c2c15b949c4d1 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 27 Jan 2015 08:23:26 -0500 >Subject: [PATCH] Bug 12598 - Tidy import_borrowers.pl > >Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > tools/import_borrowers.pl | 255 ++++++++++++++++++++++++++++------------------ > 1 file changed, 156 insertions(+), 99 deletions(-) > >diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl >index 8c03274..b6b5312 100755 >--- a/tools/import_borrowers.pl >+++ b/tools/import_borrowers.pl >@@ -51,48 +51,54 @@ use Koha::Patron::Debarments; > use Koha::DateUtils; > > use Text::CSV; >+ > # Text::CSV::Unicode, even in binary mode, fails to parse lines with these diacriticals: > # Ä > # Ä > > use CGI qw ( -utf8 ); >+ > # use encoding 'utf8'; # don't do this > >-my (@errors, @feedback); >-my $extended = C4::Context->preference('ExtendedPatronAttributes'); >+my ( @errors, @feedback ); >+my $extended = C4::Context->preference('ExtendedPatronAttributes'); > my $set_messaging_prefs = C4::Context->preference('EnhancedMessagingPreferences'); >-my @columnkeys = C4::Members::columns(); >+my @columnkeys = C4::Members::columns(); > @columnkeys = map { $_ ne 'borrowernumber' ? $_ : () } @columnkeys; > if ($extended) { > push @columnkeys, 'patron_attributes'; > } > > my $input = CGI->new(); >-our $csv = Text::CSV->new({binary => 1}); # binary needed for non-ASCII Unicode >+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}; #XXX > >-my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ >+my ( $template, $loggedinuser, $cookie ) = get_template_and_user( >+ { > template_name => "tools/import_borrowers.tt", > query => $input, > type => "intranet", > authnotrequired => 0, > flagsrequired => { tools => 'import_patrons' }, > debug => 1, >-}); >+ } >+); > > # get the branches and pass them to the template > my $branches = GetBranchesLoop(); >-$template->param( branches => $branches ) if ( $branches ); >+$template->param( branches => $branches ) if ($branches); >+ > # get the patron categories and pass them to the template > my $categories = GetBorrowercategoryList(); >-$template->param( categories => $categories ) if ( $categories ); >-my $columns = C4::Templates::GetColumnDefs( $input )->{borrowers}; >+$template->param( categories => $categories ) if ($categories); >+my $columns = C4::Templates::GetColumnDefs($input)->{borrowers}; > $columns = [ grep { $_->{field} ne 'borrowernumber' ? $_ : () } @$columns ]; > $template->param( borrower_fields => $columns ); > >-if ($input->param('sample')) { >+if ( $input->param('sample') ) { > print $input->header( >- -type => 'application/vnd.sun.xml.calc', # 'application/vnd.ms-excel' ? >+ -type => 'application/vnd.sun.xml.calc', # 'application/vnd.ms-excel' ? > -attachment => 'patron_import.csv', > ); > $csv->combine(@columnkeys); >@@ -109,133 +115,168 @@ my $overwrite_cardnumber = $input->param('overwrite_cardnumber'); > $template->param( SCRIPT_NAME => '/cgi-bin/koha/tools/import_borrowers.pl' ); > > if ( $uploadborrowers && length($uploadborrowers) > 0 ) { >- push @feedback, {feedback=>1, name=>'filename', value=>$uploadborrowers, filename=>$uploadborrowers}; >- my $handle = $input->upload('uploadborrowers'); >+ push @feedback, { feedback => 1, name => 'filename', value => $uploadborrowers, filename => $uploadborrowers }; >+ my $handle = $input->upload('uploadborrowers'); > my $uploadinfo = $input->uploadInfo($uploadborrowers); >- foreach (keys %$uploadinfo) { >- push @feedback, {feedback=>1, name=>$_, value=>$uploadinfo->{$_}, $_=>$uploadinfo->{$_}}; >+ foreach ( keys %$uploadinfo ) { >+ push @feedback, { feedback => 1, name => $_, value => $uploadinfo->{$_}, $_ => $uploadinfo->{$_} }; > } > my $imported = 0; > my $alreadyindb = 0; > my $overwritten = 0; > my $invalid = 0; >- my $matchpoint_attr_type; >+ my $matchpoint_attr_type; > my %defaults = $input->Vars; > > # use header line to construct key to column map > my $borrowerline = <$handle>; >- my $status = $csv->parse($borrowerline); >- ($status) or push @errors, {badheader=>1,line=>$., lineraw=>$borrowerline}; >+ my $status = $csv->parse($borrowerline); >+ ($status) or push @errors, { badheader => 1, line => $., lineraw => $borrowerline }; > my @csvcolumns = $csv->fields(); > my %csvkeycol; > my $col = 0; > foreach my $keycol (@csvcolumns) { >- # columnkeys don't contain whitespace, but some stupid tools add it >- $keycol =~ s/ +//g; >+ >+ # columnkeys don't contain whitespace, but some stupid tools add it >+ $keycol =~ s/ +//g; > $csvkeycol{$keycol} = $col++; > } >+ > #warn($borrowerline); > my $ext_preserve = $input->param('ext_preserve') || 0; > if ($extended) { > $matchpoint_attr_type = C4::Members::AttributeTypes->fetch($matchpoint); > } > >- push @feedback, {feedback=>1, name=>'headerrow', value=>join(', ', @csvcolumns)}; >- my $today_iso = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' }); >+ push @feedback, { feedback => 1, name => 'headerrow', value => join( ', ', @csvcolumns ) }; >+ my $today_iso = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' } ); > my @criticals = qw(surname branchcode categorycode); # there probably should be others >- my @bad_dates; # I've had a few. >- LINE: while ( my $borrowerline = <$handle> ) { >+ my @bad_dates; # I've had a few. >+ LINE: while ( my $borrowerline = <$handle> ) { > my %borrower; > my @missing_criticals; > my $patron_attributes; > my $status = $csv->parse($borrowerline); > my @columns = $csv->fields(); >- if (! $status) { >- push @missing_criticals, {badparse=>1, line=>$., lineraw=>$borrowerline}; >- } elsif (@columns == @columnkeys) { >+ if ( !$status ) { >+ push @missing_criticals, { badparse => 1, line => $., lineraw => $borrowerline }; >+ } >+ elsif ( @columns == @columnkeys ) { > @borrower{@columnkeys} = @columns; >+ > # MJR: try to fill blanks gracefully by using default values > foreach my $key (@columnkeys) { >- if ($borrower{$key} !~ /\S/) { >+ if ( $borrower{$key} !~ /\S/ ) { > $borrower{$key} = $defaults{$key}; > } >- } >- } else { >+ } >+ } >+ else { > # MJR: try to recover gracefully by using default values > foreach my $key (@columnkeys) { >- if (defined($csvkeycol{$key}) and $columns[$csvkeycol{$key}] =~ /\S/) { >- $borrower{$key} = $columns[$csvkeycol{$key}]; >- } elsif ( $defaults{$key} ) { >- $borrower{$key} = $defaults{$key}; >- } elsif ( scalar grep {$key eq $_} @criticals ) { >- # a critical field is undefined >- push @missing_criticals, {key=>$key, line=>$., lineraw=>$borrowerline}; >- } else { >- $borrower{$key} = ''; >- } >+ if ( defined( $csvkeycol{$key} ) and $columns[ $csvkeycol{$key} ] =~ /\S/ ) { >+ $borrower{$key} = $columns[ $csvkeycol{$key} ]; >+ } >+ elsif ( $defaults{$key} ) { >+ $borrower{$key} = $defaults{$key}; >+ } >+ elsif ( scalar grep { $key eq $_ } @criticals ) { >+ >+ # a critical field is undefined >+ push @missing_criticals, { key => $key, line => $., lineraw => $borrowerline }; >+ } >+ else { >+ $borrower{$key} = ''; >+ } > } > } >+ > #warn join(':',%borrower); >- if ($borrower{categorycode}) { >- push @missing_criticals, {key=>'categorycode', line=>$. , lineraw=>$borrowerline, value=>$borrower{categorycode}, category_map=>1} >- unless GetBorrowercategory($borrower{categorycode}); >- } else { >- push @missing_criticals, {key=>'categorycode', line=>$. , lineraw=>$borrowerline}; >+ if ( $borrower{categorycode} ) { >+ push @missing_criticals, >+ { >+ key => 'categorycode', >+ line => $., >+ lineraw => $borrowerline, >+ value => $borrower{categorycode}, >+ category_map => 1 >+ } >+ unless GetBorrowercategory( $borrower{categorycode} ); >+ } >+ else { >+ push @missing_criticals, { key => 'categorycode', line => $., lineraw => $borrowerline }; > } >- if ($borrower{branchcode}) { >- push @missing_criticals, {key=>'branchcode', line=>$. , lineraw=>$borrowerline, value=>$borrower{branchcode}, branch_map=>1} >- unless GetBranchName($borrower{branchcode}); >- } else { >- push @missing_criticals, {key=>'branchcode', line=>$. , lineraw=>$borrowerline}; >+ if ( $borrower{branchcode} ) { >+ push @missing_criticals, >+ { >+ key => 'branchcode', >+ line => $., >+ lineraw => $borrowerline, >+ value => $borrower{branchcode}, >+ branch_map => 1 >+ } >+ unless GetBranchName( $borrower{branchcode} ); >+ } >+ else { >+ push @missing_criticals, { key => 'branchcode', line => $., lineraw => $borrowerline }; > } > if (@missing_criticals) { > foreach (@missing_criticals) { > $_->{borrowernumber} = $borrower{borrowernumber} || 'UNDEF'; >- $_->{surname} = $borrower{surname} || 'UNDEF'; >+ $_->{surname} = $borrower{surname} || 'UNDEF'; > } > $invalid++; >- (25 > scalar @errors) and push @errors, {missing_criticals=>\@missing_criticals}; >+ ( 25 > scalar @errors ) and push @errors, { missing_criticals => \@missing_criticals }; >+ > # The first 25 errors are enough. Keeping track of 30,000+ would destroy performance. > next LINE; > } > 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\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); >+ 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); > } >- # Popular spreadsheet applications make it difficult to force date outputs to be zero-padded, but we require it. >+ >+ # Popular spreadsheet applications make it difficult to force date outputs to be zero-padded, but we require it. > foreach (qw(dateofbirth dateenrolled dateexpiry)) { > my $tempdate = $borrower{$_} or next; >- $tempdate = eval { output_pref( { dt => dt_from_string( $tempdate ), dateonly => 1, dateformat => 'iso' } ); }; >+ $tempdate = >+ eval { output_pref( { dt => dt_from_string($tempdate), dateonly => 1, dateformat => 'iso' } ); }; > if ($tempdate) { > $borrower{$_} = $tempdate; >- } else { >+ } >+ else { > $borrower{$_} = ''; >- push @missing_criticals, {key=>$_, line=>$. , lineraw=>$borrowerline, bad_date=>1}; >+ push @missing_criticals, { key => $_, line => $., lineraw => $borrowerline, bad_date => 1 }; > } > } >- $borrower{dateenrolled} = $today_iso unless $borrower{dateenrolled}; >- $borrower{dateexpiry} = GetExpiryDate($borrower{categorycode},$borrower{dateenrolled}) unless $borrower{dateexpiry}; >+ $borrower{dateenrolled} = $today_iso unless $borrower{dateenrolled}; >+ $borrower{dateexpiry} = GetExpiryDate( $borrower{categorycode}, $borrower{dateenrolled} ) >+ unless $borrower{dateexpiry}; > my $borrowernumber; > my $member; >- if ( ($matchpoint eq 'cardnumber') && ($borrower{'cardnumber'}) ) { >+ if ( ( $matchpoint eq 'cardnumber' ) && ( $borrower{'cardnumber'} ) ) { > $member = GetMember( 'cardnumber' => $borrower{'cardnumber'} ); > if ($member) { > $borrowernumber = $member->{'borrowernumber'}; > } >- } elsif ( ($matchpoint eq 'userid') && ($borrower{'userid'}) ) { >+ } >+ elsif ( ( $matchpoint eq 'userid' ) && ( $borrower{'userid'} ) ) { > $member = GetMember( 'userid' => $borrower{'userid'} ); > if ($member) { > $borrowernumber = $member->{'borrowernumber'}; > } >- } elsif ($extended) { >- if (defined($matchpoint_attr_type)) { >+ } >+ elsif ($extended) { >+ if ( defined($matchpoint_attr_type) ) { > foreach my $attr (@$patron_attributes) { >- if ($attr->{code} eq $matchpoint and $attr->{value} ne '') { >- my @borrowernumbers = $matchpoint_attr_type->get_patrons($attr->{value}); >+ if ( $attr->{code} eq $matchpoint and $attr->{value} ne '' ) { >+ my @borrowernumbers = $matchpoint_attr_type->get_patrons( $attr->{value} ); > $borrowernumber = $borrowernumbers[0] if scalar(@borrowernumbers) == 1; > last; > } >@@ -244,32 +285,35 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > } > > if ( C4::Members::checkcardnumber( $borrower{cardnumber}, $borrowernumber ) ) { >- push @errors, { >+ push @errors, >+ { > invalid_cardnumber => 1, >- borrowernumber => $borrowernumber, >- cardnumber => $borrower{cardnumber} >- }; >+ borrowernumber => $borrowernumber, >+ cardnumber => $borrower{cardnumber} >+ }; > $invalid++; > next; > } > > if ($borrowernumber) { >+ > # borrower exists > unless ($overwrite_cardnumber) { > $alreadyindb++; >- $template->param('lastalreadyindb'=>$borrower{'surname'}.' / '.$borrowernumber); >+ $template->param( 'lastalreadyindb' => $borrower{'surname'} . ' / ' . $borrowernumber ); > next LINE; > } > $borrower{'borrowernumber'} = $borrowernumber; >- for my $col (keys %borrower) { >+ for my $col ( keys %borrower ) { >+ > # use values from extant patron unless our csv file includes this column or we provided a default. > # FIXME : You cannot update a field with a perl-evaluated false value using the defaults. > > # The password is always encrypted, skip it! > next if $col eq 'password'; > >- unless(exists($csvkeycol{$col}) || $defaults{$col}) { >- $borrower{$col} = $member->{$col} if($member->{$col}) ; >+ unless ( exists( $csvkeycol{$col} ) || $defaults{$col} ) { >+ $borrower{$col} = $member->{$col} if ( $member->{$col} ); > } > } > >@@ -284,9 +328,10 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > > unless (ModMember(%borrower)) { > $invalid++; >+ > # until 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); >+ push @errors, { unknown_error => 1 }; >+ $template->param( 'lastinvalid' => $borrower{'surname'} . ' / ' . $borrowernumber ); > next LINE; > } > >@@ -300,6 +345,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > comment => $borrower{debarredcomment} > } > ); >+ > # If it doesn't, then add it! > unless (@$debarrments) { > AddDebarment( >@@ -315,19 +361,23 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > if ($extended) { > if ($ext_preserve) { > my $old_attributes = GetBorrowerAttributes($borrowernumber); >- $patron_attributes = extended_attributes_merge($old_attributes, $patron_attributes); #TODO: expose repeatable options in template >+ $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes ) >+ ; #TODO: expose repeatable options in template > } >- push @errors, {unknown_error => 1} unless SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes, 'no_branch_limit' ); >+ >+ push @errors, { unknown_error => 1 } >+ unless SetBorrowerAttributes( $borrower{'borrowernumber'}, $patron_attributes ); > } > $overwritten++; >- $template->param('lastoverwritten'=>$borrower{'surname'}.' / '.$borrowernumber); >- } else { >+ $template->param( 'lastoverwritten' => $borrower{'surname'} . ' / ' . $borrowernumber ); >+ } >+ else { > # FIXME: fixup_cardnumber says to lock table, but the web interface doesn't so this doesn't either. > # At least this is closer to AddMember than in members/memberentry.pl >- if (!$borrower{'cardnumber'}) { >+ if ( !$borrower{'cardnumber'} ) { > $borrower{'cardnumber'} = fixup_cardnumber(undef); > } >- if ($borrowernumber = AddMember(%borrower)) { >+ if ( $borrowernumber = AddMember(%borrower) ) { > > if ( $borrower{debarred} ) { > AddDebarment( >@@ -340,25 +390,30 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > } > > if ($extended) { >- SetBorrowerAttributes($borrowernumber, $patron_attributes); >+ SetBorrowerAttributes( $borrowernumber, $patron_attributes ); > } > > if ($set_messaging_prefs) { >- C4::Members::Messaging::SetMessagingPreferencesFromDefaults({ borrowernumber => $borrowernumber, >- categorycode => $borrower{categorycode} }); >+ C4::Members::Messaging::SetMessagingPreferencesFromDefaults( >+ { >+ borrowernumber => $borrowernumber, >+ categorycode => $borrower{categorycode} >+ } >+ ); > } > > $imported++; >- $template->param('lastimported'=>$borrower{'surname'}.' / '.$borrowernumber); >- } else { >+ $template->param( 'lastimported' => $borrower{'surname'} . ' / ' . $borrowernumber ); >+ } >+ else { > $invalid++; >- push @errors, {unknown_error => 1}; >- $template->param('lastinvalid'=>$borrower{'surname'}.' / AddMember'); >+ push @errors, { unknown_error => 1 }; >+ $template->param( 'lastinvalid' => $borrower{'surname'} . ' / AddMember' ); > } > } > } >- (@errors ) and $template->param( ERRORS=>\@errors ); >- (@feedback) and $template->param(FEEDBACK=>\@feedback); >+ (@errors) and $template->param( ERRORS => \@errors ); >+ (@feedback) and $template->param( FEEDBACK => \@feedback ); > $template->param( > 'uploadborrowers' => 1, > 'imported' => $imported, >@@ -368,17 +423,19 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > 'total' => $imported + $alreadyindb + $invalid + $overwritten, > ); > >-} else { >+} >+else { > if ($extended) { > my @matchpoints = (); >- my @attr_types = C4::Members::AttributeTypes::GetAttributeTypes(undef, 1); >+ my @attr_types = C4::Members::AttributeTypes::GetAttributeTypes( undef, 1 ); > foreach my $type (@attr_types) { >- my $attr_type = C4::Members::AttributeTypes->fetch($type->{code}); >- if ($attr_type->unique_id()) { >- push @matchpoints, { code => "patron_attribute_" . $attr_type->code(), description => $attr_type->description() }; >+ my $attr_type = C4::Members::AttributeTypes->fetch( $type->{code} ); >+ if ( $attr_type->unique_id() ) { >+ push @matchpoints, >+ { code => "patron_attribute_" . $attr_type->code(), description => $attr_type->description() }; > } > } >- $template->param(matchpoints => \@matchpoints); >+ $template->param( matchpoints => \@matchpoints ); > } > } > >-- >2.1.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=52324">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12598
:
29807
|
29808
|
29809
|
29939
|
30755
|
30780
|
35557
|
35558
|
35559
|
35560
|
35848
|
35849
|
35850
|
35851
|
35964
|
36684
|
36685
|
36687
|
36688
|
36690
|
36692
|
36693
|
36694
|
36695
|
36696
|
39124
|
39125
|
39126
|
39127
|
39128
|
41790
|
41791
|
41792
|
41793
|
41794
|
44065
|
44066
|
44067
|
44068
|
44069
|
48753
|
48754
|
48755
|
48756
|
48757
|
50693
|
50694
|
50695
|
50696
|
50697
|
50698
|
50699
|
50700
|
50704
|
51122
|
51357
|
51358
|
51359
|
51360
|
51361
|
51362
|
51363
|
51364
|
51365
|
51366
|
51367
|
51368
|
51369
|
51370
|
51371
|
51372
|
52323
|
52324
|
52325
|
52326
|
52327
|
52328
|
52329
|
52330
|
52331
|
52332
|
52333
|
52540
|
64437
|
64438
|
64855
|
65214
|
65215
|
65216
|
65217
|
65218
|
65819
|
65820
|
65821
|
65822
|
65823
|
65824
|
66353
|
66354
|
66355
|
66356
|
66357
|
66358
|
66359
|
66575
|
66576
|
66577
|
66578
|
66579
|
66580
|
66581
|
66582
|
67276
|
67277
|
70265
|
70266
|
71755
|
71756
|
71757
|
71758
|
71759
|
71760
|
71761
|
71762
|
71763
|
71764
|
71765
|
71766