[Koha-patches] [PATCH] Bug 4102: import_borrowers.pl fails silently
Chris Nighswonger
cnighswonger at foundations.edu
Wed Feb 10 18:13:01 CET 2010
This patch adds code to validate column names in the CSV
header row against the current columns in the borrowers
table as well as additional valid columns.
It also adds error trapping to prevent import of files with
fatal errors such as unparsable header rows and invalid
column names.
---
C4/Members.pm | 247 ++++++++-------
.../prog/en/includes/error-messages.inc | 17 +
.../prog/en/modules/tools/import_borrowers.tmpl | 4 +-
tools/import_borrowers.pl | 336 +++++++++++---------
4 files changed, 343 insertions(+), 261 deletions(-)
diff --git a/C4/Members.pm b/C4/Members.pm
index 98adc27..651da1a 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -41,24 +41,24 @@ BEGIN {
#Get data
push @EXPORT, qw(
&Search
- &SearchMember
+ &SearchMember
&GetMemberDetails
&GetMember
- &GetGuarantees
+ &GetGuarantees
&GetMemberIssuesAndFines
&GetPendingIssues
&GetAllIssues
- &get_institutions
- &getzipnamecity
+ &get_institutions
+ &getzipnamecity
&getidcity
- &GetAge
- &GetCities
- &GetRoadTypes
- &GetRoadTypeDetails
+ &GetAge
+ &GetCities
+ &GetRoadTypes
+ &GetRoadTypeDetails
&GetSortDetails
&GetTitles
@@ -70,7 +70,7 @@ BEGIN {
&GetMemberAccountRecords
&GetBorNotifyAcctRecord
- &GetborCatFromCatType
+ &GetborCatFromCatType
&GetBorrowercategory
&GetBorrowercategoryList
@@ -84,6 +84,7 @@ BEGIN {
&DeleteMessage
&GetMessages
&GetMessagesCount
+ &get_borrower_table_fields
);
#Modify data
@@ -128,7 +129,7 @@ use C4::Members;
=head1 DESCRIPTION
-This module contains routines for adding, modifying and deleting members/patrons/borrowers
+This module contains routines for adding, modifying and deleting members/patrons/borrowers
=head1 FUNCTIONS
@@ -146,7 +147,7 @@ BUGFIX 499: C<$type> is now used to determine type of search.
if $type is "simple", search is performed on the first letter of the
surname only.
-$category_type is used to get a specified type of user.
+$category_type is used to get a specified type of user.
(mainly adults when creating a child.)
C<$searchstring> is a space-separated list of search terms. Each term
@@ -173,7 +174,7 @@ sub SearchMember {
my $count;
my @data;
my @bind = ();
-
+
# this is used by circulation everytime a new borrowers cardnumber is scanned
# so we can check an exact match first, if that works return, otherwise do the rest
$query = "SELECT * FROM borrowers
@@ -188,7 +189,7 @@ sub SearchMember {
if ( $type eq "simple" ) # simple search for one letter only
{
- $query .= ($category_type ? " AND category_type = ".$dbh->quote($category_type) : "");
+ $query .= ($category_type ? " AND category_type = ".$dbh->quote($category_type) : "");
$query .= " WHERE (surname LIKE ? OR cardnumber like ?) ";
if (C4::Context->preference("IndependantBranches") && !$showallbranches){
if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){
@@ -206,8 +207,8 @@ sub SearchMember {
if (C4::Context->preference("IndependantBranches") && !$showallbranches){
if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){
$query.=" borrowers.branchcode =".$dbh->quote(C4::Context->userenv->{'branch'})." AND " unless (C4::Context->userenv->{'branch'} eq "insecure");
- }
- }
+ }
+ }
$query.="((surname LIKE ? OR surname LIKE ?
OR firstname LIKE ? OR firstname LIKE ?
OR othernames LIKE ? OR othernames LIKE ?)
@@ -258,7 +259,7 @@ BUGFIX 499: C<$type> is now used to determine type of search.
if $type is "simple", search is performed on the first letter of the
surname only.
-$category_type is used to get a specified type of user.
+$category_type is used to get a specified type of user.
(mainly adults when creating a child.)
C<$filter> can be
@@ -427,7 +428,7 @@ fields from the reserves table of the Koha database.
=back
-All the "message" fields that include language generated in this function are deprecated,
+All the "message" fields that include language generated in this function are deprecated,
because such strings belong properly in the display layer.
The "message" field that comes from the DB is OK.
@@ -540,7 +541,7 @@ sub GetMember {
my $dbh = C4::Context->dbh;
my $select =
q{SELECT borrowers.*, categories.category_type, categories.description
- FROM borrowers
+ FROM borrowers
LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE };
my $more_p = 0;
my @values = ();
@@ -618,7 +619,7 @@ sub IsMemberBlocked {
qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber)
LEFT JOIN issuingrules ON (issuingrules.itemtype=items.itype)}
}else{
- $strsth .=
+ $strsth .=
qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber)
LEFT JOIN biblioitems ON (biblioitems.biblioitemnumber=items.biblioitemnumber)
LEFT JOIN issuingrules ON (issuingrules.itemtype=biblioitems.itemtype) };
@@ -666,8 +667,8 @@ sub GetMemberIssuesAndFines {
my $issue_count = $sth->fetchrow_arrayref->[0];
$sth = $dbh->prepare(
- "SELECT COUNT(*) FROM issues
- WHERE borrowernumber = ?
+ "SELECT COUNT(*) FROM issues
+ WHERE borrowernumber = ?
AND date_due < now()"
);
$sth->execute($borrowernumber);
@@ -719,7 +720,7 @@ sub ModMember {
# is adult check guarantees;
UpdateGuarantees(%data);
}
- logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ arg: $data{'borrowernumber'})")
+ logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ arg: $data{'borrowernumber'})")
if C4::Context->preference("BorrowersLog");
return $execute_success;
@@ -743,10 +744,10 @@ sub AddMember {
my $dbh = C4::Context->dbh;
$data{'password'} = '!' if (not $data{'password'} and $data{'userid'});
$data{'password'} = md5_base64( $data{'password'} ) if $data{'password'};
- $data{'borrowernumber'}=InsertInTable("borrowers",\%data);
+ $data{'borrowernumber'}=InsertInTable("borrowers",\%data);
# mysql_insertid is probably bad. not necessarily accurate and mysql-specific at best.
logaction("MEMBERS", "CREATE", $data{'borrowernumber'}, "") if C4::Context->preference("BorrowersLog");
-
+
# check for enrollment fee & add it if needed
my $sth = $dbh->prepare("SELECT enrolmentfee FROM categories WHERE categorycode=?");
$sth->execute($data{'categorycode'});
@@ -814,9 +815,9 @@ sub changepassword {
$sth->execute( $uid, $digest, $member );
$resultcode=1;
}
-
+
logaction("MEMBERS", "CHANGE PASS", $member, "") if C4::Context->preference("BorrowersLog");
- return $resultcode;
+ return $resultcode;
}
@@ -891,7 +892,7 @@ sub fixup_cardnumber ($) {
my ($result) = $sth->fetchrow;
return $result + 1;
}
- return $cardnumber; # just here as a fallback/reminder
+ return $cardnumber; # just here as a fallback/reminder
}
=head2 GetGuarantees
@@ -921,14 +922,14 @@ sub GetGuarantees {
$sth->execute($borrowernumber);
my @dat;
- my $data = $sth->fetchall_arrayref({});
+ my $data = $sth->fetchall_arrayref({});
return ( scalar(@$data), $data );
}
=head2 UpdateGuarantees
&UpdateGuarantees($parent_borrno);
-
+
C<&UpdateGuarantees> borrower data for an adult and updates all the guarantees
with the modified information
@@ -946,7 +947,7 @@ sub UpdateGuarantees {
# It looks like the $i is only being returned to handle walking through
# the array, which is probably better done as a foreach loop.
#
- my $guaquery = qq|UPDATE borrowers
+ my $guaquery = qq|UPDATE borrowers
SET address='$data{'address'}',fax='$data{'fax'}',
B_city='$data{'B_city'}',mobile='$data{'mobile'}',city='$data{'city'}',phone='$data{'phone'}'
WHERE borrowernumber='$guarantees->[$i]->{'borrowernumber'}'
@@ -1042,19 +1043,19 @@ sub GetAllIssues {
my $dbh = C4::Context->dbh;
my $count = 0;
my $query =
- "SELECT *,issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
- FROM issues
+ "SELECT *,issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
+ FROM issues
LEFT JOIN items on items.itemnumber=issues.itemnumber
LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
- WHERE borrowernumber=?
+ WHERE borrowernumber=?
UNION ALL
- SELECT *,old_issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
- FROM old_issues
+ SELECT *,old_issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
+ FROM old_issues
LEFT JOIN items on items.itemnumber=old_issues.itemnumber
LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
- WHERE borrowernumber=?
+ WHERE borrowernumber=?
order by $order";
if ( $limit != 0 ) {
$query .= " limit $limit";
@@ -1083,7 +1084,7 @@ sub GetAllIssues {
LEFT JOIN items ON items.itemnumber=oldissues.itemnumber
LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
- WHERE borrowernumber=?
+ WHERE borrowernumber=?
ORDER BY $order";
if ( $limit != 0 ) {
$limit = $limit - $count;
@@ -1124,8 +1125,8 @@ sub GetMemberAccountRecords {
my @acctlines;
my $numlines = 0;
my $strsth = qq(
- SELECT *
- FROM accountlines
+ SELECT *
+ FROM accountlines
WHERE borrowernumber=?);
my @bind = ($borrowernumber);
if ($date && $date ne ''){
@@ -1170,11 +1171,11 @@ sub GetBorNotifyAcctRecord {
my @acctlines;
my $numlines = 0;
my $sth = $dbh->prepare(
- "SELECT *
- FROM accountlines
- WHERE borrowernumber=?
- AND notify_id=?
- AND amountoutstanding != '0'
+ "SELECT *
+ FROM accountlines
+ WHERE borrowernumber=?
+ AND notify_id=?
+ AND amountoutstanding != '0'
ORDER BY notify_id,accounttype
");
# AND (accounttype='FU' OR accounttype='N' OR accounttype='M'OR accounttype='A'OR accounttype='F'OR accounttype='L' OR accounttype='IP' OR accounttype='CH' OR accounttype='RE' OR accounttype='RL')
@@ -1237,16 +1238,16 @@ sub checkcardnumber {
my $sth = $dbh->prepare($query);
if ($borrowernumber) {
$sth->execute($cardnumber,$borrowernumber);
- } else {
+ } else {
$sth->execute($cardnumber);
- }
+ }
if (my $data= $sth->fetchrow_hashref()){
return 1;
}
else {
return 0;
}
-}
+}
=head2 getzipnamecity (OUEST-PROVENCE)
@@ -1284,7 +1285,7 @@ sub getidcity {
}
-=head2 GetExpiryDate
+=head2 GetExpiryDate
$expirydate = GetExpiryDate($categorycode, $dateenrolled);
@@ -1314,7 +1315,7 @@ sub GetExpiryDate {
=head2 checkuserpassword (OUEST-PROVENCE)
check for the password and login are not used
-return the number of record
+return the number of record
0=> NOT USED 1=> USED
=cut
@@ -1347,10 +1348,10 @@ to category descriptions.
#'
sub GetborCatFromCatType {
my ( $category_type, $action ) = @_;
- # FIXME - This API seems both limited and dangerous.
+ # FIXME - This API seems both limited and dangerous.
my $dbh = C4::Context->dbh;
- my $request = qq| SELECT categorycode,description
- FROM categories
+ my $request = qq| SELECT categorycode,description
+ FROM categories
$action
ORDER BY categorycode|;
my $sth = $dbh->prepare($request);
@@ -1377,7 +1378,7 @@ sub GetborCatFromCatType {
Given the borrower's category code, the function returns the corresponding
data hashref for a comprehensive information display.
-
+
$arrayref_hashref = &GetBorrowercategory;
If no category code provided, the function returns all the categories.
@@ -1389,20 +1390,20 @@ sub GetBorrowercategory {
if ($catcode){
my $sth =
$dbh->prepare(
- "SELECT description,dateofbirthrequired,upperagelimit,category_type
- FROM categories
+ "SELECT description,dateofbirthrequired,upperagelimit,category_type
+ FROM categories
WHERE categorycode = ?"
);
$sth->execute($catcode);
my $data =
$sth->fetchrow_hashref;
return $data;
- }
- return;
+ }
+ return;
} # sub getborrowercategory
=head2 GetBorrowercategoryList
-
+
$arrayref_hashref = &GetBorrowercategoryList;
If no category code provided, the function returns all the categories.
@@ -1412,8 +1413,8 @@ sub GetBorrowercategoryList {
my $dbh = C4::Context->dbh;
my $sth =
$dbh->prepare(
- "SELECT *
- FROM categories
+ "SELECT *
+ FROM categories
ORDER BY description"
);
$sth->execute;
@@ -1556,8 +1557,8 @@ sub GetCities {
#my ($type_city) = @_;
my $dbh = C4::Context->dbh;
- my $query = qq|SELECT cityid,city_zipcode,city_name
- FROM cities
+ my $query = qq|SELECT cityid,city_zipcode,city_name
+ FROM cities
ORDER BY city_name|;
my $sth = $dbh->prepare($query);
@@ -1591,7 +1592,7 @@ sub GetCities {
Returns the authorized value details
C<&$lib>return value of authorized value details
-C<&$sortvalue>this is the value of authorized value
+C<&$sortvalue>this is the value of authorized value
C<&$category>this is the value of authorized value category
=cut
@@ -1599,8 +1600,8 @@ C<&$category>this is the value of authorized value category
sub GetSortDetails {
my ( $category, $sortvalue ) = @_;
my $dbh = C4::Context->dbh;
- my $query = qq|SELECT lib
- FROM authorised_values
+ my $query = qq|SELECT lib
+ FROM authorised_values
WHERE category=?
AND authorised_value=? |;
my $sth = $dbh->prepare($query);
@@ -1624,8 +1625,8 @@ Copy the record from borrowers to deletedborrowers table.
sub MoveMemberToDeleted {
my ($member) = shift or return;
my $dbh = C4::Context->dbh;
- my $query = qq|SELECT *
- FROM borrowers
+ my $query = qq|SELECT *
+ FROM borrowers
WHERE borrowernumber=?|;
my $sth = $dbh->prepare($query);
$sth->execute($member);
@@ -1653,8 +1654,8 @@ sub DelMember {
#warn "in delmember with $borrowernumber";
return unless $borrowernumber; # borrowernumber is mandatory.
- my $query = qq|DELETE
- FROM reserves
+ my $query = qq|DELETE
+ FROM reserves
WHERE borrowernumber=?|;
my $sth = $dbh->prepare($query);
$sth->execute($borrowernumber);
@@ -1687,8 +1688,8 @@ sub ExtendMemberSubscriptionTo {
$date = GetExpiryDate( $borrower->{'categorycode'}, $date );
}
my $sth = $dbh->do(<<EOF);
-UPDATE borrowers
-SET dateexpiry='$date'
+UPDATE borrowers
+SET dateexpiry='$date'
WHERE borrowernumber='$borrowerid'
EOF
# add enrolmentfee if needed
@@ -1716,8 +1717,8 @@ codes, and a reference-to-hash, which maps the road type of the road .
sub GetRoadTypes {
my $dbh = C4::Context->dbh;
my $query = qq|
-SELECT roadtypeid,road_type
-FROM roadtype
+SELECT roadtypeid,road_type
+FROM roadtype
ORDER BY road_type|;
my $sth = $dbh->prepare($query);
$sth->execute();
@@ -1839,8 +1840,8 @@ sub GetRoadTypeDetails {
my ($roadtypeid) = @_;
my $dbh = C4::Context->dbh;
my $query = qq|
-SELECT road_type
-FROM roadtype
+SELECT road_type
+FROM roadtype
WHERE roadtypeid=?|;
my $sth = $dbh->prepare($query);
$sth->execute($roadtypeid);
@@ -1853,19 +1854,19 @@ WHERE roadtypeid=?|;
&GetBorrowersWhoHaveNotBorrowedSince($date)
this function get all borrowers who haven't borrowed since the date given on input arg.
-
+
=cut
sub GetBorrowersWhoHaveNotBorrowedSince {
my $filterdate = shift||POSIX::strftime("%Y-%m-%d",localtime());
my $filterexpiry = shift;
- my $filterbranch = shift ||
- ((C4::Context->preference('IndependantBranches')
- && C4::Context->userenv
- && C4::Context->userenv->{flags} % 2 !=1
+ my $filterbranch = shift ||
+ ((C4::Context->preference('IndependantBranches')
+ && C4::Context->userenv
+ && C4::Context->userenv->{flags} % 2 !=1
&& C4::Context->userenv->{branch})
? C4::Context->userenv->{branch}
- : "");
+ : "");
my $dbh = C4::Context->dbh;
my $query = "
SELECT borrowers.borrowernumber,
@@ -1874,12 +1875,12 @@ sub GetBorrowersWhoHaveNotBorrowedSince {
FROM borrowers
JOIN categories USING (categorycode)
LEFT JOIN old_issues USING (borrowernumber)
- LEFT JOIN issues USING (borrowernumber)
+ LEFT JOIN issues USING (borrowernumber)
WHERE category_type <> 'S'
- AND borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0)
+ AND borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0)
";
my @query_params;
- if ($filterbranch && $filterbranch ne ""){
+ if ($filterbranch && $filterbranch ne ""){
$query.=" AND borrowers.branchcode= ?";
push @query_params,$filterbranch;
}
@@ -1888,20 +1889,20 @@ sub GetBorrowersWhoHaveNotBorrowedSince {
push @query_params,$filterdate;
}
$query.=" GROUP BY borrowers.borrowernumber";
- if ($filterdate){
- $query.=" HAVING (latestissue < ? OR latestissue IS NULL)
+ if ($filterdate){
+ $query.=" HAVING (latestissue < ? OR latestissue IS NULL)
AND currentissue IS NULL";
push @query_params,$filterdate;
}
warn $query if $debug;
my $sth = $dbh->prepare($query);
- if (scalar(@query_params)>0){
+ if (scalar(@query_params)>0){
$sth->execute(@query_params);
- }
+ }
else {
$sth->execute;
- }
-
+ }
+
my @results;
while ( my $data = $sth->fetchrow_hashref ) {
push @results, $data;
@@ -1920,13 +1921,13 @@ I<$result> is a ref to an array which all elements are a hasref.
=cut
sub GetBorrowersWhoHaveNeverBorrowed {
- my $filterbranch = shift ||
- ((C4::Context->preference('IndependantBranches')
- && C4::Context->userenv
- && C4::Context->userenv->{flags} % 2 !=1
+ my $filterbranch = shift ||
+ ((C4::Context->preference('IndependantBranches')
+ && C4::Context->userenv
+ && C4::Context->userenv->{flags} % 2 !=1
&& C4::Context->userenv->{branch})
? C4::Context->userenv->{branch}
- : "");
+ : "");
my $dbh = C4::Context->dbh;
my $query = "
SELECT borrowers.borrowernumber,max(timestamp) as latestissue
@@ -1935,20 +1936,20 @@ sub GetBorrowersWhoHaveNeverBorrowed {
WHERE issues.borrowernumber IS NULL
";
my @query_params;
- if ($filterbranch && $filterbranch ne ""){
+ if ($filterbranch && $filterbranch ne ""){
$query.=" AND borrowers.branchcode= ?";
push @query_params,$filterbranch;
}
warn $query if $debug;
-
+
my $sth = $dbh->prepare($query);
- if (scalar(@query_params)>0){
+ if (scalar(@query_params)>0){
$sth->execute(@query_params);
- }
+ }
else {
$sth->execute;
- }
-
+ }
+
my @results;
while ( my $data = $sth->fetchrow_hashref ) {
push @results, $data;
@@ -1970,25 +1971,25 @@ This hashref is containt the number of time this borrowers has borrowed before I
sub GetBorrowersWithIssuesHistoryOlderThan {
my $dbh = C4::Context->dbh;
my $date = shift ||POSIX::strftime("%Y-%m-%d",localtime());
- my $filterbranch = shift ||
- ((C4::Context->preference('IndependantBranches')
- && C4::Context->userenv
- && C4::Context->userenv->{flags} % 2 !=1
+ my $filterbranch = shift ||
+ ((C4::Context->preference('IndependantBranches')
+ && C4::Context->userenv
+ && C4::Context->userenv->{flags} % 2 !=1
&& C4::Context->userenv->{branch})
? C4::Context->userenv->{branch}
- : "");
+ : "");
my $query = "
SELECT count(borrowernumber) as n,borrowernumber
FROM old_issues
WHERE returndate < ?
- AND borrowernumber IS NOT NULL
- ";
+ AND borrowernumber IS NOT NULL
+ ";
my @query_params;
push @query_params, $date;
if ($filterbranch){
$query.=" AND branchcode = ?";
push @query_params, $filterbranch;
- }
+ }
$query.=" GROUP BY borrowernumber ";
warn $query if $debug;
my $sth = $dbh->prepare($query);
@@ -2014,10 +2015,10 @@ This hashref is containt the number of time this borrowers has borrowed before I
sub GetBorrowersNamesAndLatestIssue {
my $dbh = C4::Context->dbh;
- my @borrowernumbers=@_;
+ my @borrowernumbers=@_;
my $query = "
SELECT surname,lastname, phone, email,max(timestamp)
- FROM borrowers
+ FROM borrowers
LEFT JOIN issues ON borrowers.borrowernumber=issues.borrowernumber
GROUP BY borrowernumber
";
@@ -2051,7 +2052,7 @@ sub DebarMember {
return ModMember( borrowernumber => $borrowernumber,
debarred => 1 );
-
+
}
=head2 AddMessage
@@ -2189,6 +2190,28 @@ sub DeleteMessage {
}
+=head2 get_borrower_table_fields
+
+=over 4
+
+ at field_list = get_borrower_table_fields();
+
+=back
+
+=cut
+
+sub get_borrower_table_fields {
+ my $dbh = C4::Context->dbh;
+ my $query = "DESCRIBE borrowers";
+ my $sth = $dbh->prepare($query);
+ $sth->execute();
+ my $columns = [];
+ while (my $row = $sth->fetchrow_hashref) {
+ push @$columns, $row;
+ }
+ return $columns;
+}
+
END { } # module clean-up code here (global destructor)
1;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/error-messages.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/error-messages.inc
index 7eb0fe8..1dcbc6c 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/error-messages.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/error-messages.inc
@@ -71,22 +71,26 @@ window.onload=function(){
<div id='mypopup' name='mypopup' style='position: absolute; width: 400px; height: 131px; display: none; background: #FFC url(/intranet-tmpl/prog/img/alert-bg.gif) repeat-x left 0; border: 1px solid #bcbcbc; right: 0px; top: 500px'>
<span id="message" style="position: absolute; top: 5px; left: 5px;">
<strong style="color: #900;">WARNING:</strong>
+ <!-- NOTE: 100 Errors apply to creator database operations. -->
<!-- TMPL_IF NAME="101" -->
The database returned an error while <!-- TMPL_IF NAME="card_element" -->saving <!-- TMPL_VAR NAME="card_element" --> <!-- TMPL_VAR NAME="element_id" --><!-- TMPL_ELSE -->attempting a save operation<!-- /TMPL_IF -->. Please have your system administrator check the error log for details.
<!-- TMPL_ELSIF NAME="102" -->
The database returned an error while <!-- TMPL_IF NAME="card_element" -->deleteing <!-- TMPL_VAR NAME="card_element" --> <!-- TMPL_VAR NAME="element_id" --><!-- TMPL_ELSIF NAME=image_ids --><!-- TMPL_VAR NAME="image_ids" --><!-- TMPL_ELSE -->attempting a delete operation<!-- /TMPL_IF -->. Please have your system administrator check the error log for details.
+ <!-- NOTE: 200 Errors apply to creator manage scripts. -->
<!-- TMPL_ELSIF NAME="201" -->
An unsupported operation was attempted<!-- TMPL_IF NAME="element_id" --> on <!-- TMPL_VAR NAME="card_element" --> <!-- TMPL_VAR NAME="element_id" --><!-- /TMPL_IF -->. Please have your system administrator check the error log for details.
<!-- TMPL_ELSIF NAME="202" -->
An error has occurred. Please ask your system administrator to check the error log for more details.
<!-- TMPL_ELSIF NAME="203" -->
A non-existent or invalid branch code was supplied. Please <a href="/cgi-bin/koha/circ/selectbranchprinter.pl">verify</a> that you have a branch selected.
+ <!-- NOTE: 300 Errors apply to image upload scripts. -->
<!-- TMPL_ELSIF NAME="301" -->
An error has occurred while attempting to upload the image file. Please ask you system administrator to check the error log for more details.
<!-- TMPL_ELSIF NAME="302" -->
Image exceeds 500KB. Please resize and import again.
<!-- TMPL_ELSIF NAME="303" -->
The database image quota currently only allows a maximum of <!-- TMPL_VAR NAME="image_limit" --> images to be stored at any one time. Please delete one or more images to free up quota space.
+ <!-- NOTE: 400 Errors apply to creator batch scripts. -->
<!-- TMPL_ELSIF NAME="401" -->
An error has occurred and the item(s) was not added to batch <!-- TMPL_VAR NAME="batch_id" -->. Please have your system administrator check the error log for details.
<!-- TMPL_ELSIF NAME="402" -->
@@ -97,6 +101,19 @@ window.onload=function(){
An error has occurred and batch <!-- TMPL_VAR NAME="batch_id" --> was not deleted. Please have your system administrator check the error log for details.
<!-- TMPL_ELSIF NAME="405" -->
An error has occurred and batch <!-- TMPL_VAR NAME="batch_id" --> not fully de-duplicated.
+ <!-- NOTE: 500 Errors apply to the import_borrowers.pl script. -->
+ <!-- TMPL_ELSIF NAME="501" -->
+ The uploaded CSV file contains the following invalid column name(s):
+ <div style="margin: 3px;">
+ <ul>
+ <!-- TMPL_LOOP NAME="errors" -->
+ <li><b><!-- TMPL_VAR NAME="column" --></b></li>
+ <!-- /TMPL_LOOP -->
+ </ul>
+ </div>
+ Please correct and re-import.
+ <!-- TMPL_ELSIF NAME="502" -->
+ Header row could not be parsed. Please correct and re-import.
<!-- TMPL_ELSE -->
<!-- /TMPL_IF -->
</span>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl
index e71fc5f..0e0e97b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl
@@ -17,6 +17,7 @@
<div id="bd">
<div id="yui-main">
<div class="yui-b">
+ <!-- TMPL_INCLUDE NAME="error-messages.inc" -->
<div class="yui-g">
<div class="yui-u first">
<h1>Import Patrons</h1>
@@ -53,7 +54,6 @@
<h5>Error analysis:</h5>
<ul>
<!-- TMPL_LOOP NAME="ERRORS" -->
- <!-- TMPL_IF NAME="badheader" --><li>Header row could not be parsed</li><!-- /TMPL_IF -->
<!-- TMPL_LOOP NAME="missing_criticals" -->
<li class="line_error">
Line <span class="linenumber"><!-- TMPL_VAR NAME="line" --></span>
@@ -147,7 +147,7 @@
<!-- TMPL_LOOP name="columnkeys" -->'<!-- TMPL_VAR name="key" -->', <!-- /TMPL_LOOP -->
</li></ul></li>
<!-- TMPL_IF NAME="ExtendedPatronAttributes" -->
-<li>If loading patron attributes, the 'patron_attributes' field should contain a comma-separated list of attribute types
+<li>If loading patron attributes, the 'patron_attributes' field should contain a comma-separated list of attribute types
and values. The attribute type code and a ':' should precede each value. For example: "INSTID:12345,LANG:fr". This
means that if an input record has more than one attribute, the 'patron_attributes' field must be wrapped in double quotation marks.
</li>
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index 1baf079..8fbb888 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -46,6 +46,7 @@ use C4::Members::Attributes qw(:all);
use C4::Members::AttributeTypes;
use C4::Members::Messaging;
+use autouse 'Data::Dumper' => qw(Dumper);
use Text::CSV;
# Text::CSV::Unicode, even in binary mode, fails to parse lines with these diacriticals:
# Ä—
@@ -55,6 +56,8 @@ use CGI;
# use encoding 'utf8'; # don't do this
my (@errors, @feedback);
+my $errstr= 0;
+my $invalid_columns = [];
my $extended = C4::Context->preference('ExtendedPatronAttributes');
my $set_messaging_prefs = C4::Context->preference('EnhancedMessagingPreferences');
my @columnkeys = C4::Members->columns;
@@ -109,13 +112,23 @@ if ( $uploadborrowers && length($uploadborrowers) > 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};
+ if (!$status) {
+ push @errors, {badheader => 1};
+ $errstr = 502;
+ $template->param(
+ error => ($errstr ? 1 : 0),
+ $errstr => 1,
+ errors => \@errors,
+ );
+ output_html_with_http_headers $input, $cookie, $template->output;
+ exit; # fatal so bail here
+ }
my @csvcolumns = $csv->fields();
my %csvkeycol;
my $col = 0;
@@ -124,173 +137,196 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
$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 = C4::Dates->new()->output('iso');
- my @criticals = qw(surname branchcode categorycode); # there probably should be others
- my @bad_dates; # I've had a few.
- my $date_re = C4::Dates->new->regexp('syspref');
- my $iso_re = C4::Dates->new->regexp('iso');
- 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) {
- @borrower{@columnkeys} = @columns;
- # MJR: try to fill blanks gracefully by using default values
- foreach my $key (@criticals) {
- if ($borrower{$key} !~ /\S/) {
- $borrower{$key} = $defaults{$key};
- }
- }
- } 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} = '';
- }
- }
- }
- #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{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';
- }
- $invalid++;
- (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;
+# verify submitted columns against actual borrowers table columns and additional valid columns
+ my $valid_columns = get_borrower_table_fields;
+ push @$valid_columns, {Field => 'patron_attributes'}; # add valid columns not in borrowers table here
+ my $valid = 0;
+ foreach (keys %csvkeycol) {
+ foreach my $column (@$valid_columns) {
+ $valid = 1 if $column->{'Field'} eq $_;
}
+ push @errors, {column => $_} if !$valid;
+ $valid = 0;
+ }
+ if (@errors) {
+ $errstr = 501;
+ $template->param(
+ error => ($errstr ? 1 : 0),
+ $errstr => 1,
+ errors => \@errors,
+ );
+ output_html_with_http_headers $input, $cookie, $template->output;
+ exit; # fatal so bail here
+ }
+ unless ($errstr) {
+ die "OPPS..."; #XXX
+ my $ext_preserve = $input->param('ext_preserve') || 0;
if ($extended) {
- my $attr_str = $borrower{patron_attributes};
- 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);
+ $matchpoint_attr_type = C4::Members::AttributeTypes->fetch($matchpoint);
}
- # 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;
- if ($tempdate =~ /$date_re/) {
- $borrower{$_} = format_date_in_iso($tempdate);
- } elsif ($tempdate =~ /$iso_re/) {
- $borrower{$_} = $tempdate;
+
+ push @feedback, {feedback=>1, name=>'headerrow', value=>join(', ', @csvcolumns)};
+ my $today_iso = C4::Dates->new()->output('iso');
+ my @criticals = qw(surname branchcode categorycode); # there probably should be others
+ my @bad_dates; # I've had a few.
+ my $date_re = C4::Dates->new->regexp('syspref');
+ my $iso_re = C4::Dates->new->regexp('iso');
+ 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) {
+ @borrower{@columnkeys} = @columns;
+ # MJR: try to fill blanks gracefully by using default values
+ foreach my $key (@criticals) {
+ if ($borrower{$key} !~ /\S/) {
+ $borrower{$key} = $defaults{$key};
+ }
+ }
} else {
- $borrower{$_} = '';
- 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};
- my $borrowernumber;
- my $member;
- if ( ($matchpoint eq 'cardnumber') && ($borrower{'cardnumber'}) ) {
- $member = GetMember( 'cardnumber' => $borrower{'cardnumber'} );
- if ($member) {
- $borrowernumber = $member->{'borrowernumber'};
- }
- } 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});
- $borrowernumber = $borrowernumbers[0] if scalar(@borrowernumbers) == 1;
- last;
+ # 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 ($borrowernumber) {
- # borrower exists
- unless ($overwrite_cardnumber) {
- $alreadyindb++;
- $template->param('lastalreadyindb'=>$borrower{'surname'}.' / '.$borrowernumber);
- next LINE;
+ #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};
}
- $borrower{'borrowernumber'} = $borrowernumber;
- 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.
- unless(exists($csvkeycol{$col}) || $defaults{$col}) {
- $borrower{$col} = $member->{$col} if($member->{$col}) ;
- }
+ 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};
}
- unless (ModMember(%borrower)) {
+ if (@missing_criticals) {
+ foreach (@missing_criticals) {
+ $_->{borrowernumber} = $borrower{borrowernumber} || 'UNDEF';
+ $_->{surname} = $borrower{surname} || 'UNDEF';
+ }
$invalid++;
- $template->param('lastinvalid'=>$borrower{'surname'}.' / '.$borrowernumber);
+ (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) {
- if ($ext_preserve) {
- my $old_attributes = GetBorrowerAttributes($borrowernumber);
- $patron_attributes = extended_attributes_merge($old_attributes, $patron_attributes); #TODO: expose repeatable options in template
+ my $attr_str = $borrower{patron_attributes};
+ 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.
+ foreach (qw(dateofbirth dateenrolled dateexpiry)) {
+ my $tempdate = $borrower{$_} or next;
+ if ($tempdate =~ /$date_re/) {
+ $borrower{$_} = format_date_in_iso($tempdate);
+ } elsif ($tempdate =~ /$iso_re/) {
+ $borrower{$_} = $tempdate;
+ } else {
+ $borrower{$_} = '';
+ push @missing_criticals, {key=>$_, line=>$. , lineraw=>$borrowerline, bad_date=>1};
}
- SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes);
}
- $overwritten++;
- $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'}) {
- $borrower{'cardnumber'} = fixup_cardnumber(undef);
+ $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'}) ) {
+ $member = GetMember( 'cardnumber' => $borrower{'cardnumber'} );
+ if ($member) {
+ $borrowernumber = $member->{'borrowernumber'};
+ }
+ } 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});
+ $borrowernumber = $borrowernumbers[0] if scalar(@borrowernumbers) == 1;
+ last;
+ }
+ }
+ }
}
- if ($borrowernumber = AddMember(%borrower)) {
- if ($extended) {
- SetBorrowerAttributes($borrowernumber, $patron_attributes);
+
+ if ($borrowernumber) {
+ # borrower exists
+ unless ($overwrite_cardnumber) {
+ $alreadyindb++;
+ $template->param('lastalreadyindb'=>$borrower{'surname'}.' / '.$borrowernumber);
+ next LINE;
+ }
+ $borrower{'borrowernumber'} = $borrowernumber;
+ 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.
+ unless(exists($csvkeycol{$col}) || $defaults{$col}) {
+ $borrower{$col} = $member->{$col} if($member->{$col}) ;
+ }
+ }
+ unless (ModMember(%borrower)) {
+ $invalid++;
+ $template->param('lastinvalid'=>$borrower{'surname'}.' / '.$borrowernumber);
+ next LINE;
}
- if ($set_messaging_prefs) {
- C4::Members::Messaging::SetMessagingPreferencesFromDefaults({ borrowernumber => $borrowernumber,
- categorycode => $borrower{categorycode} });
+ 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
+ }
+ SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes);
}
- $imported++;
- $template->param('lastimported'=>$borrower{'surname'}.' / '.$borrowernumber);
+ $overwritten++;
+ $template->param('lastoverwritten'=>$borrower{'surname'}.' / '.$borrowernumber);
} else {
- $invalid++;
- $template->param('lastinvalid'=>$borrower{'surname'}.' / AddMember');
+ # 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'}) {
+ $borrower{'cardnumber'} = fixup_cardnumber(undef);
+ }
+ if ($borrowernumber = AddMember(%borrower)) {
+ if ($extended) {
+ SetBorrowerAttributes($borrowernumber, $patron_attributes);
+ }
+ if ($set_messaging_prefs) {
+ C4::Members::Messaging::SetMessagingPreferencesFromDefaults({ borrowernumber => $borrowernumber,
+ categorycode => $borrower{categorycode} });
+ }
+ $imported++;
+ $template->param('lastimported'=>$borrower{'surname'}.' / '.$borrowernumber);
+ } else {
+ $invalid++;
+ $template->param('lastinvalid'=>$borrower{'surname'}.' / AddMember');
+ }
}
}
+ (@errors ) and $template->param( ERRORS=>\@errors );
+ (@feedback) and $template->param(FEEDBACK=>\@feedback);
+ $template->param(
+ 'uploadborrowers' => 1,
+ 'imported' => $imported,
+ 'overwritten' => $overwritten,
+ 'alreadyindb' => $alreadyindb,
+ 'invalid' => $invalid,
+ 'total' => $imported + $alreadyindb + $invalid + $overwritten,
+ );
}
- (@errors ) and $template->param( ERRORS=>\@errors );
- (@feedback) and $template->param(FEEDBACK=>\@feedback);
- $template->param(
- 'uploadborrowers' => 1,
- 'imported' => $imported,
- 'overwritten' => $overwritten,
- 'alreadyindb' => $alreadyindb,
- 'invalid' => $invalid,
- 'total' => $imported + $alreadyindb + $invalid + $overwritten,
- );
-
-} else {
+}
+else {
if ($extended) {
my @matchpoints = ();
my @attr_types = C4::Members::AttributeTypes::GetAttributeTypes();
@@ -304,5 +340,11 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
}
}
+$template->param(
+ error => ($errstr ? 1 : 0),
+ $errstr => 1,
+ invalid_columns => $invalid_columns,
+);
+
output_html_with_http_headers $input, $cookie, $template->output;
--
1.6.0.4
More information about the Koha-patches
mailing list