Cleanup Members.pm - minor
[koha.git] / C4 / Members.pm
index 385faf9..5d9cc1f 100644 (file)
@@ -27,6 +27,7 @@ use C4::Log; # logaction
 use C4::Overdues;
 use C4::Reserves;
 use C4::Accounts;
+use C4::Biblio;
 
 our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,$debug);
 
@@ -96,15 +97,16 @@ BEGIN {
        );
 
        #Check data
-       push @EXPORT, qw(
-               &checkuniquemember 
-               &checkuserpassword
-               &Check_Userid
-               &fixEthnicity
-               &ethnicitycategories 
-               &fixup_cardnumber
-               &checkcardnumber
-       );
+    push @EXPORT, qw(
+        &checkuniquemember
+        &checkuserpassword
+        &Check_Userid
+        &Generate_Userid
+        &fixEthnicity
+        &ethnicitycategories
+        &fixup_cardnumber
+        &checkcardnumber
+    );
 }
 
 =head1 NAME
@@ -182,7 +184,7 @@ sub SearchMember {
         $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}!=1 && C4::Context->userenv->{'branch'}){
+          if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){
             $query.=" AND borrowers.branchcode =".$dbh->quote(C4::Context->userenv->{'branch'}) unless (C4::Context->userenv->{'branch'} eq "insecure");
           }
         }
@@ -195,7 +197,7 @@ sub SearchMember {
         $count = @data;
         $query .= " WHERE ";
         if (C4::Context->preference("IndependantBranches") && !$showallbranches){
-          if (C4::Context->userenv && C4::Context->userenv->{flags}!=1 && C4::Context->userenv->{'branch'}){
+          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");
           }      
         }     
@@ -354,11 +356,11 @@ sub GetMemberDetails {
     my $query;
     my $sth;
     if ($borrowernumber) {
-        $sth = $dbh->prepare("select borrowers.*,category_type from borrowers left join categories on borrowers.categorycode=categories.categorycode where  borrowernumber=?");
+        $sth = $dbh->prepare("select borrowers.*,category_type,categories.description from borrowers left join categories on borrowers.categorycode=categories.categorycode where  borrowernumber=?");
         $sth->execute($borrowernumber);
     }
     elsif ($cardnumber) {
-        $sth = $dbh->prepare("select borrowers.*,category_type from borrowers left join categories on borrowers.categorycode=categories.categorycode where cardnumber=?");
+        $sth = $dbh->prepare("select borrowers.*,category_type,categories.description from borrowers left join categories on borrowers.categorycode=categories.categorycode where cardnumber=?");
         $sth->execute($cardnumber);
     }
     else {
@@ -526,7 +528,7 @@ SELECT borrowers.*, categories.category_type, categories.description
 FROM borrowers 
 LEFT JOIN categories on borrowers.categorycode=categories.categorycode 
 ";
-    if ( defined $type && ( $type eq 'cardnumber' || $type eq 'firstname'|| $type eq 'userid'|| $type eq 'borrowernumber' ) ){
+    if (defined($type) and ( $type eq 'cardnumber' || $type eq 'firstname'|| $type eq 'userid'|| $type eq 'borrowernumber' ) ){
         $information = uc $information;
         $sth = $dbh->prepare("$select WHERE $type=?");
     } else {
@@ -534,14 +536,12 @@ LEFT JOIN categories on borrowers.categorycode=categories.categorycode
     }
     $sth->execute($information);
     my $data = $sth->fetchrow_hashref;
-    $sth->finish;
     ($data) and return ($data);
 
-    if ($type eq 'cardnumber' || $type eq 'firstname') {    # otherwise, try with firstname
+    if (defined($type) and ($type eq 'cardnumber' || $type eq 'firstname')) {    # otherwise, try with firstname
         $sth = $dbh->prepare("$select WHERE firstname like ?");
         $sth->execute($information);
         $data = $sth->fetchrow_hashref;
-        $sth->finish;
         ($data) and return ($data);
     }
     return undef;        
@@ -800,6 +800,21 @@ sub Check_Userid {
     }
 }
 
+sub Generate_Userid {
+  my ($borrowernumber, $firstname, $surname) = @_;
+  my $newuid;
+  my $offset = 0;
+  do {
+    $firstname =~ s/[[:digit:][:space:][:blank:][:punct:][:cntrl:]]//g;
+    $surname =~ s/[[:digit:][:space:][:blank:][:punct:][:cntrl:]]//g;
+    $newuid = lc("$firstname.$surname");
+    $newuid .= $offset unless $offset == 0;
+    $offset++;
+
+   } while (!Check_Userid($newuid,$borrowernumber));
+
+   return $newuid;
+}
 
 sub changepassword {
     my ( $uid, $member, $digest ) = @_;
@@ -842,76 +857,65 @@ my @weightings = ( 8, 4, 6, 3, 5, 2, 1 );
 
 sub fixup_cardnumber ($) {
     my ($cardnumber) = @_;
-    my $autonumber_members = C4::Context->boolean_preference('autoMemberNum');
-    $autonumber_members = 0 unless defined $autonumber_members;
+    my $autonumber_members = C4::Context->boolean_preference('autoMemberNum') || 0;
 
     # Find out whether member numbers should be generated
     # automatically. Should be either "1" or something else.
     # Defaults to "0", which is interpreted as "no".
 
     #     if ($cardnumber !~ /\S/ && $autonumber_members) {
-    if ($autonumber_members) {
-        my $dbh = C4::Context->dbh;
-        if ( C4::Context->preference('checkdigit') eq 'katipo' ) {
-
-            # if checkdigit is selected, calculate katipo-style cardnumber.
-            # otherwise, just use the max()
-            # purpose: generate checksum'd member numbers.
-            # We'll assume we just got the max value of digits 2-8 of member #'s
-            # from the database and our job is to increment that by one,
-            # determine the 1st and 9th digits and return the full string.
-            my $sth =
-              $dbh->prepare(
-                "select max(substring(borrowers.cardnumber,2,7)) from borrowers"
-              );
-            $sth->execute;
-
-            my $data = $sth->fetchrow_hashref;
-            $cardnumber = $data->{'max(substring(borrowers.cardnumber,2,7))'};
-            $sth->finish;
-            if ( !$cardnumber ) {    # If DB has no values,
-                $cardnumber = 1000000;    # start at 1000000
-            }
-            else {
-                $cardnumber += 1;
-            }
-
-            my $sum = 0;
-            for ( my $i = 0 ; $i < 8 ; $i += 1 ) {
+    ($autonumber_members) or return $cardnumber;
+    my $checkdigit = C4::Context->preference('checkdigit');
+    my $dbh = C4::Context->dbh;
+    if ( $checkdigit and $checkdigit eq 'katipo' ) {
+
+        # if checkdigit is selected, calculate katipo-style cardnumber.
+        # otherwise, just use the max()
+        # purpose: generate checksum'd member numbers.
+        # We'll assume we just got the max value of digits 2-8 of member #'s
+        # from the database and our job is to increment that by one,
+        # determine the 1st and 9th digits and return the full string.
+        my $sth = $dbh->prepare(
+            "select max(substring(borrowers.cardnumber,2,7)) as new_num from borrowers"
+        );
+        $sth->execute;
+        my $data = $sth->fetchrow_hashref;
+        $cardnumber = $data->{new_num};
+        if ( !$cardnumber ) {    # If DB has no values,
+            $cardnumber = 1000000;    # start at 1000000
+        } else {
+            $cardnumber += 1;
+        }
 
-                # read weightings, left to right, 1 char at a time
-                my $temp1 = $weightings[$i];
+        my $sum = 0;
+        for ( my $i = 0 ; $i < 8 ; $i += 1 ) {
+            # read weightings, left to right, 1 char at a time
+            my $temp1 = $weightings[$i];
 
-                # sequence left to right, 1 char at a time
-                my $temp2 = substr( $cardnumber, $i, 1 );
+            # sequence left to right, 1 char at a time
+            my $temp2 = substr( $cardnumber, $i, 1 );
 
-                # mult each char 1-7 by its corresponding weighting
-                $sum += $temp1 * $temp2;
-            }
+            # mult each char 1-7 by its corresponding weighting
+            $sum += $temp1 * $temp2;
+        }
 
-            my $rem = ( $sum % 11 );
-            $rem = 'X' if $rem == 10;
+        my $rem = ( $sum % 11 );
+        $rem = 'X' if $rem == 10;
 
-            $cardnumber = "V$cardnumber$rem";
-        }
-        else {
+        return "V$cardnumber$rem";
+     } else {
 
      # MODIFIED BY JF: mysql4.1 allows casting as an integer, which is probably
      # better. I'll leave the original in in case it needs to be changed for you
-            my $sth =
-              $dbh->prepare(
-                "select max(cast(cardnumber as signed)) from borrowers");
-
-      #my $sth=$dbh->prepare("select max(borrowers.cardnumber) from borrowers");
-
-            $sth->execute;
-
-            my ($result) = $sth->fetchrow;
-            $sth->finish;
-            $cardnumber = $result + 1;
-        }
+     # my $sth=$dbh->prepare("select max(borrowers.cardnumber) from borrowers");
+        my $sth = $dbh->prepare(
+            "select max(cast(cardnumber as signed)) from borrowers"
+        );
+        $sth->execute;
+        my ($result) = $sth->fetchrow;
+        return $result + 1;
     }
-    return $cardnumber;
+    return $cardnumber;     # just here as a fallback/reminder 
 }
 
 =head2 GetGuarantees
@@ -951,7 +955,7 @@ sub GetGuarantees {
   &UpdateGuarantees($parent_borrno);
   
 
-C<&UpdateGuarantees> borrower data for an adulte and updates all the guarantees
+C<&UpdateGuarantees> borrower data for an adult and updates all the guarantees
 with the modified information
 
 =cut
@@ -979,44 +983,60 @@ sub UpdateGuarantees {
 }
 =head2 GetPendingIssues
 
-  ($count, $issues) = &GetPendingIssues($borrowernumber);
+  my $issues = &GetPendingIssues($borrowernumber);
 
 Looks up what the patron with the given borrowernumber has borrowed.
 
-C<&GetPendingIssues> returns a two-element array. C<$issues> is a
-reference-to-array, where each element is a reference-to-hash; the
-keys are the fields from the C<issues>, C<biblio>, and C<items> tables
-in the Koha database. C<$count> is the number of elements in
-C<$issues>.
+C<&GetPendingIssues> returns a
+reference-to-array where each element is a reference-to-hash; the
+keys are the fields from the C<issues>, C<biblio>, and C<items> tables.
+The keys include C<biblioitems> fields except marc and marcxml.
 
 =cut
 
 #'
 sub GetPendingIssues {
     my ($borrowernumber) = @_;
-    my $dbh              = C4::Context->dbh;
-
-    my $sth              = $dbh->prepare(
-   "SELECT * FROM issues 
-      LEFT JOIN items ON issues.itemnumber=items.itemnumber
-      LEFT JOIN biblio ON     items.biblionumber=biblio.biblionumber 
-      LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
+    # must avoid biblioitems.* to prevent large marc and marcxml fields from killing performance
+    # FIXME: namespace collision: each table has "timestamp" fields.  Which one is "timestamp" ?
+    # FIXME: circ/ciculation.pl tries to sort by timestamp!
+    # FIXME: C4::Print::printslip tries to sort by timestamp!
+    # FIXME: namespace collision: other collisions possible.
+    # FIXME: most of this data isn't really being used by callers.
+    my $sth = C4::Context->dbh->prepare(
+   "SELECT issues.*,
+            items.*,
+           biblio.*,
+           biblioitems.volume,
+           biblioitems.number,
+           biblioitems.itemtype,
+           biblioitems.isbn,
+           biblioitems.issn,
+           biblioitems.publicationyear,
+           biblioitems.publishercode,
+           biblioitems.volumedate,
+           biblioitems.volumedesc,
+           biblioitems.lccn,
+           biblioitems.url,
+           issues.timestamp AS timestamp,
+           issues.renewals  AS renewals,
+            items.renewals  AS totalrenewals
+    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=? 
+      borrowernumber=?
     ORDER BY issues.issuedate"
     );
     $sth->execute($borrowernumber);
     my $data = $sth->fetchall_arrayref({});
-    my $today = POSIX::strftime("%Y%m%d", localtime);
-    foreach( @$data ) {
-        my $datedue = $_->{'date_due'};
-        $datedue =~ s/-//g;
-        if ( $datedue < $today ) {
-            $_->{'overdue'} = 1;
-        }
+    my $today = C4::Dates->new->output('iso');
+    foreach (@$data) {
+        $_->{date_due} or next;
+        ($_->{date_due} lt $today) and $_->{overdue} = 1;
     }
-    $sth->finish;
-    return ( scalar(@$data), $data );
+    return $data;
 }
 
 =head2 GetAllIssues
@@ -1048,14 +1068,14 @@ sub GetAllIssues {
     my $dbh   = C4::Context->dbh;
     my $count = 0;
     my $query =
-  "SELECT *,items.timestamp AS itemstimestamp 
+  "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=? 
   UNION ALL
-  SELECT *,items.timestamp AS itemstimestamp 
+  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
@@ -1145,10 +1165,14 @@ sub GetMemberAccountRecords {
     $sth->execute( @bind );
     my $total = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
+               my $biblio = GetBiblioFromItemNumber($data->{itemnumber}) if $data->{itemnumber};
+               $data->{biblionumber} = $biblio->{biblionumber};
+               $data->{title} = $biblio->{title};
         $acctlines[$numlines] = $data;
         $numlines++;
-        $total += $data->{'amountoutstanding'};
+        $total += int(100 * $data->{'amountoutstanding'}); # convert float to integer to avoid round-off errors
     }
+    $total /= 100;
     $sth->finish;
     return ( $total, \@acctlines,$numlines);
 }
@@ -1189,8 +1213,9 @@ sub GetBorNotifyAcctRecord {
     while ( my $data = $sth->fetchrow_hashref ) {
         $acctlines[$numlines] = $data;
         $numlines++;
-        $total += $data->{'amountoutstanding'};
+        $total += int(100 * $data->{'amountoutstanding'});
     }
+    $total /= 100;
     $sth->finish;
     return ( $total, \@acctlines, $numlines );
 }
@@ -1218,12 +1243,16 @@ sub checkuniquemember {
     my $dbh = C4::Context->dbh;
     my $request = ($collectivity) ?
         "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? " :
-        "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=? and dateofbirth=? ";
+            ($dateofbirth) ?
+            "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=?  and dateofbirth=?" :
+            "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=?";
     my $sth = $dbh->prepare($request);
     if ($collectivity) {
         $sth->execute( uc($surname) );
-    } els{
+    } elsif($dateofbirth){
         $sth->execute( uc($surname), ucfirst($firstname), $dateofbirth );
+    }else{
+        $sth->execute( uc($surname), ucfirst($firstname));
     }
     my @data = $sth->fetchrow;
     $sth->finish;
@@ -1617,32 +1646,32 @@ sub GetSortDetails {
     return ($sortvalue) unless ($lib);
 }
 
-=head2 DeleteBorrower 
+=head2 MoveMemberToDeleted
 
-  () = &DeleteBorrower($member);
+  $result = &MoveMemberToDeleted($borrowernumber);
 
-delete all data fo borrowers and add record to deletedborrowers table
-C<&$member>this is the borrowernumber
+Copy the record from borrowers to deletedborrowers table.
 
 =cut
 
+# FIXME: should do it in one SQL statement w/ subquery
+# Otherwise, we should return the @data on success
+
 sub MoveMemberToDeleted {
-    my ($member) = @_;
+    my ($member) = shift or return;
     my $dbh = C4::Context->dbh;
-    my $query;
-    $query = qq|SELECT * 
+    my $query = qq|SELECT * 
           FROM borrowers 
           WHERE borrowernumber=?|;
     my $sth = $dbh->prepare($query);
     $sth->execute($member);
     my @data = $sth->fetchrow_array;
-    $sth->finish;
+    (@data) or return;  # if we got a bad borrowernumber, there's nothing to insert
     $sth =
       $dbh->prepare( "INSERT INTO deletedborrowers VALUES ("
           . ( "?," x ( scalar(@data) - 1 ) )
           . "?)" );
     $sth->execute(@data);
-    $sth->finish;
 }
 
 =head2 DelMember
@@ -1705,7 +1734,7 @@ EOF
     $sth = $dbh->prepare("SELECT enrolmentfee FROM categories WHERE categorycode=?");
     $sth->execute($borrower->{'categorycode'});
     my ($enrolmentfee) = $sth->fetchrow;
-    if ($enrolmentfee) {
+    if ($enrolmentfee && $enrolmentfee > 0) {
         # insert fee in patron debts
         manualinvoice($borrower->{'borrowernumber'}, '', '', 'A', $enrolmentfee);
     }
@@ -1792,10 +1821,8 @@ sub GetPatronImage {
     my $sth = $dbh->prepare($query);
     $sth->execute($cardnumber);
     my $imagedata = $sth->fetchrow_hashref;
-    my $dberror = $sth->errstr;
     warn "Database error!" if $sth->errstr;
-    $sth->finish;
-    return $imagedata, $dberror;
+    return $imagedata, $sth->errstr;
 }
 
 =head2 PutPatronImage
@@ -1815,9 +1842,7 @@ sub PutPatronImage {
     my $sth = $dbh->prepare($query);
     $sth->execute($cardnumber,$mimetype,$imgfile,$imgfile);
     warn "Error returned inserting $cardnumber.$mimetype." if $sth->errstr;
-    my $dberror = $sth->errstr;
-    $sth->finish;
-    return $dberror;
+    return $sth->errstr;
 }
 
 =head2 RmPatronImage
@@ -1837,7 +1862,6 @@ sub RmPatronImage {
     $sth->execute($cardnumber);
     my $dberror = $sth->errstr;
     warn "Database error!" if $sth->errstr;
-    $sth->finish;
     return $dberror;
 }
 
@@ -1879,7 +1903,7 @@ sub GetBorrowersWhoHaveNotBorrowedSince {
     my $filterbranch = shift || 
                         ((C4::Context->preference('IndependantBranches') 
                              && C4::Context->userenv 
-                             && C4::Context->userenv->{flags}!=1 
+                             && C4::Context->userenv->{flags} % 2 !=1 
                              && C4::Context->userenv->{branch})
                          ? C4::Context->userenv->{branch}
                          : "");  
@@ -1931,7 +1955,7 @@ sub GetBorrowersWhoHaveNeverBorrowed {
     my $filterbranch = shift || 
                         ((C4::Context->preference('IndependantBranches') 
                              && C4::Context->userenv 
-                             && C4::Context->userenv->{flags}!=1 
+                             && C4::Context->userenv->{flags} % 2 !=1 
                              && C4::Context->userenv->{branch})
                          ? C4::Context->userenv->{branch}
                          : "");  
@@ -1981,7 +2005,7 @@ sub GetBorrowersWithIssuesHistoryOlderThan {
     my $filterbranch = shift || 
                         ((C4::Context->preference('IndependantBranches') 
                              && C4::Context->userenv 
-                             && C4::Context->userenv->{flags}!=1 
+                             && C4::Context->userenv->{flags} % 2 !=1 
                              && C4::Context->userenv->{branch})
                          ? C4::Context->userenv->{branch}
                          : "");