Incremental fix for Bug 3285 - Location on Add Subscription should be pull down
[koha.git] / C4 / Members.pm
index 7c7c48c..4c1d37c 100644 (file)
@@ -13,12 +13,13 @@ package C4::Members;
 # WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
 # A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
 #
-# You should have received a copy of the GNU General Public License along with
-# Koha; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
-# Suite 330, Boston, MA  02111-1307 USA
+# You should have received a copy of the GNU General Public License along
+# with Koha; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 
 use strict;
+#use warnings; FIXME - Bug 2505
 use C4::Context;
 use C4::Dates qw(format_date_in_iso);
 use Digest::MD5 qw(md5_base64);
@@ -27,6 +28,9 @@ use C4::Log; # logaction
 use C4::Overdues;
 use C4::Reserves;
 use C4::Accounts;
+use C4::Biblio;
+use C4::SQLHelper qw(InsertInTable UpdateInTable SearchInTable);
+use C4::Members::Attributes qw(SearchIdMatchingAttribute);
 
 our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,$debug);
 
@@ -37,6 +41,7 @@ BEGIN {
        @ISA = qw(Exporter);
        #Get data
        push @EXPORT, qw(
+               &Search
                &SearchMember 
                &GetMemberDetails
                &GetMember
@@ -51,6 +56,8 @@ BEGIN {
                &getzipnamecity 
                &getidcity
 
+                &GetFirstValidEmailAddress
+
                &GetAge 
                &GetCities 
                &GetRoadTypes 
@@ -62,6 +69,7 @@ BEGIN {
     &PutPatronImage
     &RmPatronImage
 
+               &IsMemberBlocked
                &GetMemberAccountRecords
                &GetBorNotifyAcctRecord
 
@@ -74,6 +82,11 @@ BEGIN {
                &GetBorrowersWithIssuesHistoryOlderThan
 
                &GetExpiryDate
+
+               &AddMessage
+               &DeleteMessage
+               &GetMessages
+               &GetMessagesCount
        );
 
        #Modify data
@@ -92,19 +105,20 @@ BEGIN {
                &AddMember
                &add_member_orgs
                &MoveMemberToDeleted
-               &ExtendMemberSubscriptionTo 
+               &ExtendMemberSubscriptionTo
        );
 
        #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
@@ -121,13 +135,10 @@ This module contains routines for adding, modifying and deleting members/patrons
 
 =head1 FUNCTIONS
 
-=over 2
+=head2 SearchMember
 
-=item SearchMember
-
-  ($count, $borrowers) = &SearchMember($searchstring, $type,$category_type,$filter,$showallbranches);
-
-=back
+  ($count, $borrowers) = &SearchMember($searchstring, $type, 
+                     $category_type, $filter, $showallbranches);
 
 Looks up patrons (borrowers) by name.
 
@@ -155,7 +166,6 @@ C<$count> is the number of elements in C<$borrowers>.
 
 #'
 #used by member enquiries from the intranet
-#called by member.pl and circ/circulation.pl
 sub SearchMember {
     my ($searchstring, $orderby, $type,$category_type,$filter,$showallbranches ) = @_;
     my $dbh   = C4::Context->dbh;
@@ -175,14 +185,13 @@ sub SearchMember {
     if (@$data){
         return ( scalar(@$data), $data );
     }
-    $sth->finish;
 
     if ( $type eq "simple" )    # simple search for one letter only
     {
         $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 +204,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");
           }      
         }     
@@ -220,16 +229,6 @@ sub SearchMember {
         }
         $query = $query . ") OR cardnumber LIKE ? ";
         push( @bind, $searchstring );
-        if (C4::Context->preference('ExtendedPatronAttributes')) {
-            $query .= "OR borrowernumber IN (
-SELECT borrowernumber
-FROM borrower_attributes
-JOIN borrower_attribute_types USING (code)
-WHERE staff_searchable = 1
-AND attribute like ?
-)";
-            push (@bind, $searchstring);
-        }
         $query .= "order by $orderby";
 
         # FIXME - .= <<EOT;
@@ -242,10 +241,60 @@ AND attribute like ?
     my @results;
     $data = $sth->fetchall_arrayref({});
 
-    $sth->finish;
     return ( scalar(@$data), $data );
 }
 
+=head2 Search
+
+  $borrowers_result_array_ref = &Search($filter,$orderby, $limit, 
+                       $columns_out, $search_on_fields,$searchtype);
+
+Looks up patrons (borrowers) on filter.
+
+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. 
+(mainly adults when creating a child.)
+
+C<$filter> can be
+   - a space-separated list of search terms. Implicit AND is done on them
+   - a hash ref containing fieldnames associated with queried value
+   - an array ref combining the two previous elements Implicit OR is done between each array element
+
+
+C<$orderby> is an arrayref of hashref. Contains the name of the field and 0 or 1 depending if order is ascending or descending
+
+C<$limit> is there to allow limiting number of results returned
+
+C<&columns_out> is an array ref to the fieldnames you want to see in the result list
+
+C<&search_on_fields> is an array ref to the fieldnames you want to limit search on when you are using string search
+
+C<&searchtype> is a string telling the type of search you want todo : start_with, exact or contains are allowed
+
+=cut
+
+sub Search {
+    my ($filter,$orderby, $limit, $columns_out, $search_on_fields,$searchtype) = @_;
+       my @filters;
+       if (ref($filter) eq "ARRAY"){
+               push @filters,@$filter;
+       }
+       else {
+               push @filters,$filter;
+       }
+    if (C4::Context->preference('ExtendedPatronAttributes')) {
+               my $matching_records = C4::Members::Attributes::SearchIdMatchingAttribute($filter);
+               push @filters,@$matching_records;
+    }
+       $searchtype||="start_with";
+       my $data=SearchInTable("borrowers",\@filters,$orderby,$limit,$columns_out,$search_on_fields,$searchtype);
+
+    return ( $data );
+}
+
 =head2 GetMemberDetails
 
 ($borrower) = &GetMemberDetails($borrowernumber, $cardnumber);
@@ -264,81 +313,11 @@ about the patron. Its keys act as flags :
         # Patron's card was reported lost
     }
 
-Each flag has a C<message> key, giving a human-readable explanation of
-the flag. If the state of a flag means that the patron should not be
+If the state of a flag means that the patron should not be
 allowed to borrow any more books, then it will have a C<noissues> key
 with a true value.
 
-The possible flags are:
-
-=head3 CHARGES
-
-=over 4
-
-=item Shows the patron's credit or debt, if any.
-
-=back
-
-=head3 GNA
-
-=over 4
-
-=item (Gone, no address.) Set if the patron has left without giving a
-forwarding address.
-
-=back
-
-=head3 LOST
-
-=over 4
-
-=item Set if the patron's card has been reported as lost.
-
-=back
-
-=head3 DBARRED
-
-=over 4
-
-=item Set if the patron has been debarred.
-
-=back
-
-=head3 NOTES
-
-=over 4
-
-=item Any additional notes about the patron.
-
-=back
-
-=head3 ODUES
-
-=over 4
-
-=item Set if the patron has overdue items. This flag has several keys:
-
-C<$flags-E<gt>{ODUES}{itemlist}> is a reference-to-array listing the
-overdue items. Its elements are references-to-hash, each describing an
-overdue item. The keys are selected fields from the issues, biblio,
-biblioitems, and items tables of the Koha database.
-
-C<$flags-E<gt>{ODUES}{itemlist}> is a string giving a text listing of
-the overdue items, one per line.
-
-=back
-
-=head3 WAITING
-
-=over 4
-
-=item Set if any items that the patron has reserved are available.
-
-C<$flags-E<gt>{WAITING}{itemlist}> is a reference-to-array listing the
-available items. Each element is a reference-to-hash whose keys are
-fields from the reserves table of the Koha database.
-
-=back
+See patronflags for more details.
 
 C<$borrower-E<gt>{authflags}> is a hash giving more detailed information
 about the top-level permissions flags set for the borrower.  For example,
@@ -354,11 +333,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 {
@@ -378,7 +357,6 @@ sub GetMemberDetails {
             $accessflagshash->{$flag} = 1;
         }
     }
-    $sth->finish;
     $borrower->{'flags'}     = $flags;
     $borrower->{'authflags'} = $accessflagshash;
 
@@ -394,37 +372,66 @@ sub GetMemberDetails {
 
 =head2 patronflags
 
- Not exported
+ $flags = &patronflags($patron);
 
- NOTE!: If you change this function, be sure to update the POD for
- &GetMemberDetails.
+This function is not exported.
 
- $flags = &patronflags($patron);
+The following will be set where applicable:
+ $flags->{CHARGES}->{amount}        Amount of debt
+ $flags->{CHARGES}->{noissues}      Set if debt amount >$5.00 (or syspref noissuescharge)
+ $flags->{CHARGES}->{message}       Message -- deprecated
+
+ $flags->{CREDITS}->{amount}        Amount of credit
+ $flags->{CREDITS}->{message}       Message -- deprecated
+
+ $flags->{  GNA  }                  Patron has no valid address
+ $flags->{  GNA  }->{noissues}      Set for each GNA
+ $flags->{  GNA  }->{message}       "Borrower has no valid address" -- deprecated
+
+ $flags->{ LOST  }                  Patron's card reported lost
+ $flags->{ LOST  }->{noissues}      Set for each LOST
+ $flags->{ LOST  }->{message}       Message -- deprecated
+
+ $flags->{DBARRED}                  Set if patron debarred, no access
+ $flags->{DBARRED}->{noissues}      Set for each DBARRED
+ $flags->{DBARRED}->{message}       Message -- deprecated
 
- $flags->{CHARGES}
-        {message}    Message showing patron's credit or debt
-       {noissues}    Set if patron owes >$5.00
-         {GNA}            Set if patron gone w/o address
-        {message}    "Borrower has no valid address"
-        {noissues}    Set.
-        {LOST}        Set if patron's card reported lost
-        {message}    Message to this effect
-        {noissues}    Set.
-        {DBARRED}        Set is patron is debarred
-        {message}    Message to this effect
-        {noissues}    Set.
-         {NOTES}        Set if patron has notes
-        {message}    Notes about patron
-         {ODUES}        Set if patron has overdue books
-        {message}    "Yes"
-        {itemlist}    ref-to-array: list of overdue books
-        {itemlisttext}    Text list of overdue items
-         {WAITING}        Set if there are items available that the
-                patron reserved
-        {message}    Message to this effect
-        {itemlist}    ref-to-array: list of available items
+ $flags->{ NOTES }
+ $flags->{ NOTES }->{message}       The note itself.  NOT deprecated
+
+ $flags->{ ODUES }                  Set if patron has overdue books.
+ $flags->{ ODUES }->{message}       "Yes"  -- deprecated
+ $flags->{ ODUES }->{itemlist}      ref-to-array: list of overdue books
+ $flags->{ ODUES }->{itemlisttext}  Text list of overdue items -- deprecated
+
+ $flags->{WAITING}                  Set if any of patron's reserves are available
+ $flags->{WAITING}->{message}       Message -- deprecated
+ $flags->{WAITING}->{itemlist}      ref-to-array: list of available items
+
+=over 
+
+=item C<$flags-E<gt>{ODUES}-E<gt>{itemlist}> is a reference-to-array listing the
+overdue items. Its elements are references-to-hash, each describing an
+overdue item. The keys are selected fields from the issues, biblio,
+biblioitems, and items tables of the Koha database.
+
+=item C<$flags-E<gt>{ODUES}-E<gt>{itemlisttext}> is a string giving a text listing of
+the overdue items, one per line.  Deprecated.
+
+=item C<$flags-E<gt>{WAITING}-E<gt>{itemlist}> is a reference-to-array listing the
+available items. Each element is a reference-to-hash whose keys are
+fields from the reserves table of the Koha database.
+
+=back
+
+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.
 
 =cut
+
+# TODO: use {anonymous => hashes} instead of a dozen %flaginfo
 # FIXME rename this function.
 sub patronflags {
     my %flags;
@@ -433,9 +440,9 @@ sub patronflags {
     my ($amount) = GetMemberAccountRecords( $patroninformation->{'borrowernumber'});
     if ( $amount > 0 ) {
         my %flaginfo;
-        my $noissuescharge = C4::Context->preference("noissuescharge");
+        my $noissuescharge = C4::Context->preference("noissuescharge") || 5;
         $flaginfo{'message'} = sprintf "Patron owes \$%.02f", $amount;
-        $flaginfo{'amount'} = sprintf "%.02f",$amount;
+        $flaginfo{'amount'}  = sprintf "%.02f", $amount;
         if ( $amount > $noissuescharge ) {
             $flaginfo{'noissues'} = 1;
         }
@@ -444,6 +451,7 @@ sub patronflags {
     elsif ( $amount < 0 ) {
         my %flaginfo;
         $flaginfo{'message'} = sprintf "Patron has credit of \$%.02f", -$amount;
+        $flaginfo{'amount'}  = sprintf "%.02f", $amount;
         $flags{'CREDITS'} = \%flaginfo;
     }
     if (   $patroninformation->{'gonenoaddress'}
@@ -472,12 +480,11 @@ sub patronflags {
         && $patroninformation->{'borrowernotes'} )
     {
         my %flaginfo;
-        $flaginfo{'message'} = "$patroninformation->{'borrowernotes'}";
+        $flaginfo{'message'} = $patroninformation->{'borrowernotes'};
         $flags{'NOTES'}      = \%flaginfo;
     }
-    my ( $odues, $itemsoverdue ) =
-      checkoverdues( $patroninformation->{'borrowernumber'}, $dbh );
-    if ( $odues > 0 ) {
+    my ( $odues, $itemsoverdue ) = checkoverdues($patroninformation->{'borrowernumber'});
+    if ( $odues && $odues > 0 ) {
         my %flaginfo;
         $flaginfo{'message'}  = "Yes";
         $flaginfo{'itemlist'} = $itemsoverdue;
@@ -485,7 +492,7 @@ sub patronflags {
             @$itemsoverdue )
         {
             $flaginfo{'itemlisttext'} .=
-              "$_->{'date_due'} $_->{'barcode'} $_->{'title'} \n";
+              "$_->{'date_due'} $_->{'barcode'} $_->{'title'} \n";  # newline is display layer
         }
         $flags{'ODUES'} = \%flaginfo;
     }
@@ -503,48 +510,140 @@ sub patronflags {
 
 =head2 GetMember
 
-  $borrower = &GetMember($information, $type);
+  $borrower = &GetMember(%information);
+
+Retrieve the first patron record meeting on criteria listed in the
+C<%information> hash, which should contain one or more
+pairs of borrowers column names and values, e.g.,
 
-Looks up information about a patron (borrower) by either card number
-,firstname, or borrower number, depending on $type value.
-If C<$type> == 'cardnumber', C<&GetBorrower>
-searches by cardnumber then by firstname if not found in cardnumber; 
-otherwise, it searches by borrowernumber.
+   $borrower = GetMember(borrowernumber => id);
 
 C<&GetBorrower> returns a reference-to-hash whose keys are the fields of
 the C<borrowers> table in the Koha database.
 
+FIXME: GetMember() is used throughout the code as a lookup
+on a unique key such as the borrowernumber, but this meaning is not
+enforced in the routine itself.
+
 =cut
 
 #'
 sub GetMember {
-    my ( $information, $type ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth;
-    my $select = "
-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' ) ){
-        $information = uc $information;
-        $sth = $dbh->prepare("$select WHERE $type=?");
-    } else {
-        $sth = $dbh->prepare("$select WHERE borrowernumber=?");
+    my ( %information ) = @_;
+    if (exists $information{borrowernumber} && !defined $information{borrowernumber}) {
+        #passing mysql's kohaadmin?? Makes no sense as a query
+        return;
     }
-    $sth->execute($information);
-    my $data = $sth->fetchrow_hashref;
-    $sth->finish;
-    ($data) and return ($data);
+    my $dbh = C4::Context->dbh;
+    my $select =
+    q{SELECT borrowers.*, categories.category_type, categories.description
+    FROM borrowers 
+    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE };
+    my $more_p = 0;
+    my @values = ();
+    for (keys %information ) {
+        if ($more_p) {
+            $select .= ' AND ';
+        }
+        else {
+            $more_p++;
+        }
 
-    if ($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);
+        if (defined $information{$_}) {
+            $select .= "$_ = ?";
+            push @values, $information{$_};
+        }
+        else {
+            $select .= "$_ IS NULL";
+        }
     }
-    return undef;        
+    $debug && warn $select, " ",values %information;
+    my $sth = $dbh->prepare("$select");
+    $sth->execute(map{$information{$_}} keys %information);
+    my $data = $sth->fetchall_arrayref({});
+    #FIXME interface to this routine now allows generation of a result set
+    #so whole array should be returned but bowhere in the current code expects this
+    if (@{$data} ) {
+        return $data->[0];
+    }
+
+    return;
+}
+
+
+=head2 IsMemberBlocked
+
+  my ($block_status, $count) = IsMemberBlocked( $borrowernumber );
+
+Returns whether a patron has overdue items that may result
+in a block or whether the patron has active fine days
+that would block circulation privileges.
+
+C<$block_status> can have the following values:
+
+1 if the patron has outstanding fine days, in which case C<$count> is the number of them
+
+-1 if the patron has overdue items, in which case C<$count> is the number of them
+
+0 if the patron has no overdue items or outstanding fine days, in which case C<$count> is 0
+
+Outstanding fine days are checked before current overdue items
+are.
+
+FIXME: this needs to be split into two functions; a potential block
+based on the number of current overdue items could be orthogonal
+to a block based on whether the patron has any fine days accrued.
+
+=cut
+
+sub IsMemberBlocked {
+    my $borrowernumber = shift;
+    my $dbh            = C4::Context->dbh;
+
+    # does patron have current fine days?
+       my $strsth=qq{
+            SELECT
+            ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due) ) AS blockingdate,
+            DATEDIFF(ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due)),NOW()) AS blockedcount
+            FROM old_issues
+       };
+    if(C4::Context->preference("item-level_itypes")){
+        $strsth.=
+               qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber)
+            LEFT JOIN issuingrules ON (issuingrules.itemtype=items.itype)}
+    }else{
+        $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) };
+    }
+       $strsth.=
+        qq{ WHERE finedays IS NOT NULL
+            AND  date_due < returndate
+            AND borrowernumber = ?
+            ORDER BY blockingdate DESC, blockedcount DESC
+            LIMIT 1};
+       my $sth=$dbh->prepare($strsth);
+    $sth->execute($borrowernumber);
+    my $row = $sth->fetchrow_hashref;
+    my $blockeddate  = $row->{'blockeddate'};
+    my $blockedcount = $row->{'blockedcount'};
+
+    return (1, $blockedcount) if $blockedcount > 0;
+
+    # if he have late issues
+    $sth = $dbh->prepare(
+        "SELECT COUNT(*) as latedocs
+         FROM issues
+         WHERE borrowernumber = ?
+         AND date_due < curdate()"
+    );
+    $sth->execute($borrowernumber);
+    my $latedocs = $sth->fetchrow_hashref->{'latedocs'};
+
+    return (-1, $latedocs) if $latedocs > 0;
+
+    return (0, 0);
 }
 
 =head2 GetMemberIssuesAndFines
@@ -571,99 +670,63 @@ sub GetMemberIssuesAndFines {
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
     my $issue_count = $sth->fetchrow_arrayref->[0];
-    $sth->finish;
 
     $sth = $dbh->prepare(
         "SELECT COUNT(*) FROM issues 
          WHERE borrowernumber = ? 
-         AND date_due < now()"
+         AND date_due < curdate()"
     );
     $sth->execute($borrowernumber);
     my $overdue_count = $sth->fetchrow_arrayref->[0];
-    $sth->finish;
 
     $sth = $dbh->prepare("SELECT SUM(amountoutstanding) FROM accountlines WHERE borrowernumber = ?");
     $sth->execute($borrowernumber);
     my $total_fines = $sth->fetchrow_arrayref->[0];
-    $sth->finish;
 
     return ($overdue_count, $issue_count, $total_fines);
 }
 
-=head2
+sub columns(;$) {
+    return @{C4::Context->dbh->selectcol_arrayref("SHOW columns from borrowers")};
+}
 
 =head2 ModMember
 
-  &ModMember($borrowernumber);
+  my $success = ModMember(borrowernumber => $borrowernumber,
+                                            [ field => value ]... );
 
 Modify borrower's data.  All date fields should ALREADY be in ISO format.
 
+return :
+true on success, or false on failure
+
 =cut
 
-#'
 sub ModMember {
     my (%data) = @_;
-    my $dbh = C4::Context->dbh;
-    my $iso_re = C4::Dates->new()->regexp('iso');
-    foreach (qw(dateofbirth dateexpiry dateenrolled)) {
-        if (my $tempdate = $data{$_}) {                                 # assignment, not comparison
-            ($tempdate =~ /$iso_re/) and next;                          # Congatulations, you sent a valid ISO date.
-            warn "ModMember given $_ not in ISO format ($tempdate)";
-            if (my $tempdate2 = format_date_in_iso($tempdate)) {        # assignment, not comparison
-                $data{$_} = $tempdate2;
-            } else {
-                warn "ModMember cannot convert '$tempdate' (from syspref)";
-            }
-        }
-    }
-    if (!$data{'dateofbirth'}){
-        undef $data{'dateofbirth'};
-    }
-    my $qborrower=$dbh->prepare("SHOW columns from borrowers");
-    $qborrower->execute;
-    my %hashborrowerfields;  
-    while (my ($field)=$qborrower->fetchrow){
-      $hashborrowerfields{$field}=1;
-    }  
-    my $query = "UPDATE borrowers SET \n";
-    my $sth;
-    my @parameters;  
-    
     # test to know if you must update or not the borrower password
-    if ( $data{'password'} eq '****' ) {
-        delete $data{'password'};
-    } else {
-        $data{'password'} = md5_base64( $data{'password'} )  if ($data{'password'} ne "");
-        delete $data{'password'} if ($data{password} eq "");
-    }
-    foreach (keys %data){  
-        if ($_ ne 'borrowernumber' and $_ ne 'flags' and $hashborrowerfields{$_}){
-          $query .= " $_=?, "; 
-          push @parameters,$data{$_};
+    if (exists $data{password}) {
+        if ($data{password} eq '****' or $data{password} eq '') {
+            delete $data{password};
+        } else {
+            $data{password} = md5_base64($data{password});
         }
     }
-    $query =~ s/, $//;
-    $query .= " WHERE borrowernumber=?";
-    push @parameters, $data{'borrowernumber'};
-    $debug and print STDERR "$query (executed w/ arg: $data{'borrowernumber'})";
-    $sth = $dbh->prepare($query);
-    $sth->execute(@parameters);
-    $sth->finish;
-
+       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 ( $borrowercategory->{'category_type'} eq ('A' || 'S') ) {
+    if ( exists  $borrowercategory->{'category_type'} && $borrowercategory->{'category_type'} eq ('A' || 'S') ) {
         # is adult check guarantees;
         UpdateGuarantees(%data);
     }
-    logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "$query (executed w/ arg: $data{'borrowernumber'})") 
+    logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ arg: $data{'borrowernumber'})") 
         if C4::Context->preference("BorrowersLog");
-}
 
+    return $execute_success;
+}
 
-=head2
 
 =head2 AddMember
 
@@ -678,95 +741,24 @@ Returns the borrowernumber
 sub AddMember {
     my (%data) = @_;
     my $dbh = C4::Context->dbh;
-    $data{'userid'} = '' unless $data{'password'};
+    $data{'password'} = '!' if (not $data{'password'} and $data{'userid'});
     $data{'password'} = md5_base64( $data{'password'} ) if $data{'password'};
-    
-    # WE SHOULD NEVER PASS THIS SUBROUTINE ANYTHING OTHER THAN ISO DATES
-    # IF YOU UNCOMMENT THESE LINES YOU BETTER HAVE A DARN COMPELLING REASON
-#    $data{'dateofbirth'}  = format_date_in_iso( $data{'dateofbirth'} );
-#    $data{'dateenrolled'} = format_date_in_iso( $data{'dateenrolled'});
-#    $data{'dateexpiry'}   = format_date_in_iso( $data{'dateexpiry'}  );
-    # This query should be rewritten to use "?" at execute.
-    if (!$data{'dateofbirth'}){
-        undef ($data{'dateofbirth'});
-    }
-    my $query =
-        "insert into borrowers set cardnumber=" . $dbh->quote( $data{'cardnumber'} )
-      . ",surname="     . $dbh->quote( $data{'surname'} )
-      . ",firstname="   . $dbh->quote( $data{'firstname'} )
-      . ",title="       . $dbh->quote( $data{'title'} )
-      . ",othernames="  . $dbh->quote( $data{'othernames'} )
-      . ",initials="    . $dbh->quote( $data{'initials'} )
-      . ",streetnumber=". $dbh->quote( $data{'streetnumber'} )
-      . ",streettype="  . $dbh->quote( $data{'streettype'} )
-      . ",address="     . $dbh->quote( $data{'address'} )
-      . ",address2="    . $dbh->quote( $data{'address2'} )
-      . ",zipcode="     . $dbh->quote( $data{'zipcode'} )
-      . ",city="        . $dbh->quote( $data{'city'} )
-      . ",phone="       . $dbh->quote( $data{'phone'} )
-      . ",email="       . $dbh->quote( $data{'email'} )
-      . ",mobile="      . $dbh->quote( $data{'mobile'} )
-      . ",phonepro="    . $dbh->quote( $data{'phonepro'} )
-      . ",opacnote="    . $dbh->quote( $data{'opacnote'} )
-      . ",guarantorid=" . $dbh->quote( $data{'guarantorid'} )
-      . ",dateofbirth=" . $dbh->quote( $data{'dateofbirth'} )
-      . ",branchcode="  . $dbh->quote( $data{'branchcode'} )
-      . ",categorycode=" . $dbh->quote( $data{'categorycode'} )
-      . ",dateenrolled=" . $dbh->quote( $data{'dateenrolled'} )
-      . ",contactname=" . $dbh->quote( $data{'contactname'} )
-      . ",borrowernotes=" . $dbh->quote( $data{'borrowernotes'} )
-      . ",dateexpiry="  . $dbh->quote( $data{'dateexpiry'} )
-      . ",contactnote=" . $dbh->quote( $data{'contactnote'} )
-      . ",B_address="   . $dbh->quote( $data{'B_address'} )
-      . ",B_zipcode="   . $dbh->quote( $data{'B_zipcode'} )
-      . ",B_city="      . $dbh->quote( $data{'B_city'} )
-      . ",B_phone="     . $dbh->quote( $data{'B_phone'} )
-      . ",B_email="     . $dbh->quote( $data{'B_email'} )
-      . ",password="    . $dbh->quote( $data{'password'} )
-      . ",userid="      . $dbh->quote( $data{'userid'} )
-      . ",sort1="       . $dbh->quote( $data{'sort1'} )
-      . ",sort2="       . $dbh->quote( $data{'sort2'} )
-      . ",contacttitle=" . $dbh->quote( $data{'contacttitle'} )
-      . ",emailpro="    . $dbh->quote( $data{'emailpro'} )
-      . ",contactfirstname=" . $dbh->quote( $data{'contactfirstname'} )
-      . ",sex="         . $dbh->quote( $data{'sex'} )
-      . ",fax="         . $dbh->quote( $data{'fax'} )
-      . ",relationship=" . $dbh->quote( $data{'relationship'} )
-      . ",B_streetnumber=" . $dbh->quote( $data{'B_streetnumber'} )
-      . ",B_streettype=" . $dbh->quote( $data{'B_streettype'} )
-      . ",gonenoaddress=" . $dbh->quote( $data{'gonenoaddress'} )
-      . ",lost="        . $dbh->quote( $data{'lost'} )
-      . ",debarred="    . $dbh->quote( $data{'debarred'} )
-      . ",ethnicity="   . $dbh->quote( $data{'ethnicity'} )
-      . ",ethnotes="    . $dbh->quote( $data{'ethnotes'} ) 
-      . ",altcontactsurname="   . $dbh->quote( $data{'altcontactsurname'} ) 
-      . ",altcontactfirstname="     . $dbh->quote( $data{'altcontactfirstname'} ) 
-      . ",altcontactaddress1="  . $dbh->quote( $data{'altcontactaddress1'} ) 
-      . ",altcontactaddress2="  . $dbh->quote( $data{'altcontactaddress2'} ) 
-      . ",altcontactaddress3="  . $dbh->quote( $data{'altcontactaddress3'} ) 
-      . ",altcontactzipcode="   . $dbh->quote( $data{'altcontactzipcode'} ) 
-      . ",altcontactphone="     . $dbh->quote( $data{'altcontactphone'} ) ;
-    $debug and print STDERR "AddMember SQL: ($query)\n";
-    my $sth = $dbh->prepare($query);
-    #   print "Executing SQL: $query\n";
-    $sth->execute();
-    $sth->finish;
-    $data{'borrowernumber'} = $dbh->{'mysql_insertid'};     # unneeded w/ autoincrement ?  
+       $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
-    $sth = $dbh->prepare("SELECT enrolmentfee FROM categories WHERE categorycode=?");
+    my $sth = $dbh->prepare("SELECT enrolmentfee FROM categories WHERE categorycode=?");
     $sth->execute($data{'categorycode'});
     my ($enrolmentfee) = $sth->fetchrow;
-    if ($enrolmentfee) {
+    if ($enrolmentfee && $enrolmentfee > 0) {
         # insert fee in patron debts
         manualinvoice($data{'borrowernumber'}, '', '', 'A', $enrolmentfee);
     }
     return $data{'borrowernumber'};
 }
 
+
 sub Check_Userid {
     my ($uid,$member) = @_;
     my $dbh = C4::Context->dbh;
@@ -784,6 +776,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 ) = @_;
@@ -826,76 +833,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 ) {
-
-                # 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 );
-
-                # mult each char 1-7 by its corresponding weighting
-                $sum += $temp1 * $temp2;
-            }
-
-            my $rem = ( $sum % 11 );
-            $rem = 'X' if $rem == 10;
-
-            $cardnumber = "V$cardnumber$rem";
+    ($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;
         }
-        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");
+        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];
 
-            $sth->execute;
+            # sequence left to right, 1 char at a time
+            my $temp2 = substr( $cardnumber, $i, 1 );
 
-            my ($result) = $sth->fetchrow;
-            $sth->finish;
-            $cardnumber = $result + 1;
+            # mult each char 1-7 by its corresponding weighting
+            $sum += $temp1 * $temp2;
         }
+
+        my $rem = ( $sum % 11 );
+        $rem = 'X' if $rem == 10;
+
+        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(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
@@ -926,7 +922,6 @@ sub GetGuarantees {
 
     my @dat;
     my $data = $sth->fetchall_arrayref({}); 
-    $sth->finish;
     return ( scalar(@$data), $data );
 }
 
@@ -935,77 +930,86 @@ 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
 
 #'
 sub UpdateGuarantees {
-    my (%data) = @_;
+    my %data = shift;
     my $dbh = C4::Context->dbh;
     my ( $count, $guarantees ) = GetGuarantees( $data{'borrowernumber'} );
-    for ( my $i = 0 ; $i < $count ; $i++ ) {
-
-        # FIXME
-        # It looks like the $i is only being returned to handle walking through
-        # the array, which is probably better done as a foreach loop.
-        #
+    foreach my $guarantee (@$guarantees){
         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'}'
+              SET address=?,fax=?,B_city=?,mobile=?,city=?,phone=?
+              WHERE borrowernumber=?
         |;
-        my $sth3 = $dbh->prepare($guaquery);
-        $sth3->execute;
-        $sth3->finish;
+        my $sth = $dbh->prepare($guaquery);
+        $sth->execute($data{'address'},$data{'fax'},$data{'B_city'},$data{'mobile'},$data{'city'},$data{'phone'},$guarantee->{'borrowernumber'});
     }
 }
 =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
 
-  ($count, $issues) = &GetAllIssues($borrowernumber, $sortkey, $limit);
+  $issues = &GetAllIssues($borrowernumber, $sortkey, $limit);
 
 Looks up what the patron with the given borrowernumber has borrowed,
 and sorts the results.
@@ -1016,11 +1020,9 @@ C<biblioitems>, or C<items> table in the Koha database.
 
 C<$limit> is the maximum number of results to return.
 
-C<&GetAllIssues> 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>, C<biblioitems>, and
-C<items> tables of the Koha database. C<$count> is the number of
-elements in C<$issues>
+C<&GetAllIssues> an arrayref, C<$issues>, of hashrefs, the keys of which
+are the fields from the C<issues>, C<biblio>, C<biblioitems>, and
+C<items> tables of the Koha database.
 
 =cut
 
@@ -1030,16 +1032,15 @@ sub GetAllIssues {
 
     #FIXME: sanity-check order and limit
     my $dbh   = C4::Context->dbh;
-    my $count = 0;
     my $query =
-  "SELECT *,items.timestamp AS itemstimestamp 
+  "SELECT *, issues.timestamp as issuestimestamp, 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.timestamp as issuestimestamp, 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
@@ -1050,48 +1051,15 @@ sub GetAllIssues {
         $query .= " limit $limit";
     }
 
-    #print $query;
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber, $borrowernumber);
     my @result;
     my $i = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
-        $result[$i] = $data;
-        $i++;
-        $count++;
-    }
-
-    # get all issued items for borrowernumber from oldissues table
-    # large chunk of older issues data put into table oldissues
-    # to speed up db calls for issuing items
-    if ( C4::Context->preference("ReadingHistory") ) {
-        # FIXME oldissues (not to be confused with old_issues) is
-        # apparently specific to HLT.  Not sure if the ReadingHistory
-        # syspref is still required, as old_issues by design
-        # is no longer checked with each loan.
-        my $query2 = "SELECT * FROM oldissues
-                      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=? 
-                      ORDER BY $order";
-        if ( $limit != 0 ) {
-            $limit = $limit - $count;
-            $query2 .= " limit $limit";
-        }
-
-        my $sth2 = $dbh->prepare($query2);
-        $sth2->execute($borrowernumber);
-
-        while ( my $data2 = $sth2->fetchrow_hashref ) {
-            $result[$i] = $data2;
-            $i++;
-        }
-        $sth2->finish;
+        push @result, $data;
     }
-    $sth->finish;
 
-    return ( $i, \@result );
+    return \@result;
 }
 
 
@@ -1129,11 +1097,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(1000 * $data->{'amountoutstanding'}); # convert float to integer to avoid round-off errors
     }
-    $sth->finish;
+    $total /= 1000;
     return ( $total, \@acctlines,$numlines);
 }
 
@@ -1173,9 +1144,9 @@ sub GetBorNotifyAcctRecord {
     while ( my $data = $sth->fetchrow_hashref ) {
         $acctlines[$numlines] = $data;
         $numlines++;
-        $total += $data->{'amountoutstanding'};
+        $total += int(100 * $data->{'amountoutstanding'});
     }
-    $sth->finish;
+    $total /= 100;
     return ( $total, \@acctlines, $numlines );
 }
 
@@ -1202,15 +1173,18 @@ 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;
     ( $data[0] ) and return $data[0], $data[1];
     return 0;
 }
@@ -1232,7 +1206,6 @@ sub checkcardnumber {
     else {
         return 0;
     }
-    $sth->finish();
 }  
 
 
@@ -1270,6 +1243,33 @@ sub getidcity {
     return $data;
 }
 
+=head2 GetFirstValidEmailAddress
+
+  $email = GetFirstValidEmailAddress($borrowernumber);
+
+Return the first valid email address for a borrower, given the borrowernumber.  For now, the order 
+is defined as email, emailpro, B_email.  Returns the empty string if the borrower has no email 
+addresses.
+
+=cut
+
+sub GetFirstValidEmailAddress {
+    my $borrowernumber = shift;
+    my $dbh = C4::Context->dbh;
+    my $sth = $dbh->prepare( "SELECT email, emailpro, B_email FROM borrowers where borrowernumber = ? ");
+    $sth->execute( $borrowernumber );
+    my $data = $sth->fetchrow_hashref;
+
+    if ($data->{'email'}) {
+       return $data->{'email'};
+    } elsif ($data->{'emailpro'}) {
+       return $data->{'emailpro'};
+    } elsif ($data->{'B_email'}) {
+       return $data->{'B_email'};
+    } else {
+       return '';
+    }
+}
 
 =head2 GetExpiryDate 
 
@@ -1282,16 +1282,20 @@ Return date is also in ISO format.
 
 sub GetExpiryDate {
     my ( $categorycode, $dateenrolled ) = @_;
-    my $enrolmentperiod = 12;   # reasonable default
+    my $enrolments;
     if ($categorycode) {
         my $dbh = C4::Context->dbh;
-        my $sth = $dbh->prepare("select enrolmentperiod from categories where categorycode=?");
+        my $sth = $dbh->prepare("SELECT enrolmentperiod,enrolmentperioddate FROM categories WHERE categorycode=?");
         $sth->execute($categorycode);
-        $enrolmentperiod = $sth->fetchrow;
+        $enrolments = $sth->fetchrow_hashref;
     }
     # die "GetExpiryDate: for enrollmentperiod $enrolmentperiod (category '$categorycode') starting $dateenrolled.\n";
-    my @date = split /-/,$dateenrolled;
-    return sprintf("%04d-%02d-%02d", Add_Delta_YM(@date,0,$enrolmentperiod));
+    my @date = split (/-/,$dateenrolled);
+    if($enrolments->{enrolmentperiod}){
+        return sprintf("%04d-%02d-%02d", Add_Delta_YM(@date,0,$enrolments->{enrolmentperiod}));
+    }else{
+        return $enrolments->{enrolmentperioddate};
+    }
 }
 
 =head2 checkuserpassword (OUEST-PROVENCE)
@@ -1351,7 +1355,6 @@ sub GetborCatFromCatType {
         push @codes, $data->{'categorycode'};
         $labels{ $data->{'categorycode'} } = $data->{'description'};
     }
-    $sth->finish;
     return ( \@codes, \%labels );
 }
 
@@ -1361,8 +1364,9 @@ 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.
 
 =cut
@@ -1380,14 +1384,13 @@ sub GetBorrowercategory {
         $sth->execute($catcode);
         my $data =
         $sth->fetchrow_hashref;
-        $sth->finish();
         return $data;
     } 
     return;  
 }    # sub getborrowercategory
 
 =head2 GetBorrowercategoryList
+
   $arrayref_hashref = &GetBorrowercategoryList;
 If no category code provided, the function returns all the categories.
 
@@ -1404,7 +1407,6 @@ sub GetBorrowercategoryList {
     $sth->execute;
     my $data =
     $sth->fetchall_arrayref({});
-    $sth->finish();
     return $data;
 }    # sub getborrowercategory
 
@@ -1431,7 +1433,6 @@ sub ethnicitycategories {
         push @codes, $data->{'code'};
         $labels{ $data->{'code'} } = $data->{'name'};
     }
-    $sth->finish;
     return ( \@codes, \%labels );
 }
 
@@ -1454,7 +1455,6 @@ sub fixEthnicity {
     my $sth       = $dbh->prepare("Select name from ethnicity where code = ?");
     $sth->execute($ethnicity);
     my $data = $sth->fetchrow_hashref;
-    $sth->finish;
     return $data->{'name'};
 }    # sub fixEthnicity
 
@@ -1486,6 +1486,7 @@ sub GetAge{
 }    # sub get_age
 
 =head2 get_institutions
+
   $insitutions = get_institutions();
 
 Just returns a list of all the borrowers of type I, borrownumber and name
@@ -1504,7 +1505,6 @@ sub get_institutions {
     while ( my $data = $sth->fetchrow_hashref() ) {
         $orgs{ $data->{'borrowernumber'} } = $data;
     }
-    $sth->finish();
     return ( \%orgs );
 
 }    # sub get_institutions
@@ -1527,53 +1527,34 @@ sub add_member_orgs {
     foreach my $otherborrowernumber (@$otherborrowers) {
         $sth->execute( $borrowernumber, $otherborrowernumber );
     }
-    $sth->finish();
 
 }    # sub add_member_orgs
 
-=head2 GetCities (OUEST-PROVENCE)
+=head2 GetCities
 
-  ($id_cityarrayref, $city_hashref) = &GetCities();
+  $cityarrayref = GetCities();
 
-Looks up the different city and zip in the database. Returns two
-elements: a reference-to-array, which lists the zip city
-codes, and a reference-to-hash, which maps the name of the city.
-WHERE =>OUEST PROVENCE OR EXTERIEUR
+  Returns an array_ref of the entries in the cities table
+  If there are entries in the table an empty row is returned
+  This is currently only used to populate a popup in memberentry
 
 =cut
 
 sub GetCities {
 
-    #my ($type_city) = @_;
     my $dbh   = C4::Context->dbh;
-    my $query = qq|SELECT cityid,city_zipcode,city_name 
-        FROM cities 
-        ORDER BY city_name|;
-    my $sth = $dbh->prepare($query);
-
-    #$sth->execute($type_city);
-    $sth->execute();
-    my %city;
-    my @id;
-    #    insert empty value to create a empty choice in cgi popup
-    push @id, " ";
-    $city{""} = "";
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push @id, $data->{'city_zipcode'}."|".$data->{'city_name'};
-        $city{ $data->{'city_zipcode'}."|".$data->{'city_name'} } = $data->{'city_name'};
-    }
-
-#test to know if the table contain some records if no the function return nothing
-    my $id = @id;
-    $sth->finish;
-    if ( $id == 1 ) {
-        # all we have is the one blank row
-        return ();
-    }
-    else {
-        unshift( @id, "" );
-        return ( \@id, \%city );
-    }
+    my $city_arr = $dbh->selectall_arrayref(
+        q|SELECT cityid,city_zipcode,city_name FROM cities ORDER BY city_name|,
+        { Slice => {} });
+    if ( @{$city_arr} ) {
+        unshift @{$city_arr}, {
+            city_zipcode => q{},
+            city_name    => q{},
+            cityid       => q{},
+        };
+    }
+
+    return  $city_arr;
 }
 
 =head2 GetSortDetails (OUEST-PROVENCE)
@@ -1601,37 +1582,37 @@ 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
 
-DelMember($borrowernumber);
+    DelMember($borrowernumber);
 
 This function remove directly a borrower whitout writing it on deleteborrower.
 + Deletes reserves for the borrower
@@ -1649,7 +1630,6 @@ sub DelMember {
           WHERE borrowernumber=?|;
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
-    $sth->finish;
     $query = "
        DELETE
        FROM borrowers
@@ -1657,7 +1637,6 @@ sub DelMember {
    ";
     $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
-    $sth->finish;
     logaction("MEMBERS", "DELETE", $borrowernumber, "") if C4::Context->preference("BorrowersLog");
     return $sth->rows;
 }
@@ -1674,10 +1653,9 @@ Returns ISO date.
 sub ExtendMemberSubscriptionTo {
     my ( $borrowerid,$date) = @_;
     my $dbh = C4::Context->dbh;
-    my $borrower = GetMember($borrowerid,'borrowernumber');
+    my $borrower = GetMember('borrowernumber'=>$borrowerid);
     unless ($date){
       $date=POSIX::strftime("%Y-%m-%d",localtime());
-      my $borrower = GetMember($borrowerid,'borrowernumber');
       $date = GetExpiryDate( $borrower->{'categorycode'}, $date );
     }
     my $sth = $dbh->do(<<EOF);
@@ -1689,7 +1667,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);
     }
@@ -1728,7 +1706,6 @@ ORDER BY road_type|;
 
 #test to know if the table contain some records if no the function return nothing
     my $id = @id;
-    $sth->finish;
     if ( $id eq 0 ) {
         return ();
     }
@@ -1749,7 +1726,7 @@ Looks up the different title . Returns array  with all borrowers title
 =cut
 
 sub GetTitles {
-    my @borrowerTitle = split /,|\|/,C4::Context->preference('BorrowersTitles');
+    my @borrowerTitle = split (/,|\|/,C4::Context->preference('BorrowersTitles'));
     unshift( @borrowerTitle, "" );
     my $count=@borrowerTitle;
     if ($count == 1){
@@ -1772,14 +1749,12 @@ sub GetPatronImage {
     my ($cardnumber) = @_;
     warn "Cardnumber passed to GetPatronImage is $cardnumber" if $debug;
     my $dbh = C4::Context->dbh;
-    my $query = "SELECT mimetype, imagefile FROM patronimage WHERE cardnumber = ?;";
+    my $query = 'SELECT mimetype, imagefile FROM patronimage WHERE cardnumber = ?';
     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
@@ -1799,9 +1774,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
@@ -1821,7 +1794,6 @@ sub RmPatronImage {
     $sth->execute($cardnumber);
     my $dberror = $sth->errstr;
     warn "Database error!" if $sth->errstr;
-    $sth->finish;
     return $dberror;
 }
 
@@ -1850,39 +1822,47 @@ WHERE roadtypeid=?|;
 
 =head2 GetBorrowersWhoHaveNotBorrowedSince
 
-&GetBorrowersWhoHaveNotBorrowedSince($date)
+  &GetBorrowersWhoHaveNotBorrowedSince($date)
 
 this function get all borrowers who haven't borrowed since the date given on input arg.
-      
+
 =cut
 
 sub GetBorrowersWhoHaveNotBorrowedSince {
-### TODO : It could be dangerous to delete Borrowers who have just been entered and who have not yet borrowed any book. May be good to add a dateexpiry or dateenrolled filter.      
-       
-                my $filterdate = shift||POSIX::strftime("%Y-%m-%d",localtime());
+    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}!=1 
+                             && 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(issues.timestamp) as latestissue
+        SELECT borrowers.borrowernumber,
+               max(old_issues.timestamp) as latestissue,
+               max(issues.timestamp) as currentissue
         FROM   borrowers
         JOIN   categories USING (categorycode)
-        LEFT JOIN issues ON borrowers.borrowernumber = issues.borrowernumber
+        LEFT JOIN old_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) 
    ";
     my @query_params;
     if ($filterbranch && $filterbranch ne ""){ 
         $query.=" AND borrowers.branchcode= ?";
         push @query_params,$filterbranch;
-    }    
+    }
+    if($filterexpiry){
+        $query .= " AND dateexpiry < ? ";
+        push @query_params,$filterdate;
+    }
     $query.=" GROUP BY borrowers.borrowernumber";
     if ($filterdate){ 
-        $query.=" HAVING latestissue <? OR latestissue IS NULL";
+        $query.=" HAVING (latestissue < ? OR latestissue IS NULL) 
+                  AND currentissue IS NULL";
         push @query_params,$filterdate;
     }
     warn $query if $debug;
@@ -1903,9 +1883,9 @@ sub GetBorrowersWhoHaveNotBorrowedSince {
 
 =head2 GetBorrowersWhoHaveNeverBorrowed
 
-$results = &GetBorrowersWhoHaveNeverBorrowed
+  $results = &GetBorrowersWhoHaveNeverBorrowed
 
-this function get all borrowers who have never borrowed.
+This function get all borrowers who have never borrowed.
 
 I<$result> is a ref to an array which all elements are a hasref.
 
@@ -1915,7 +1895,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}
                          : "");  
@@ -1950,7 +1930,7 @@ sub GetBorrowersWhoHaveNeverBorrowed {
 
 =head2 GetBorrowersWithIssuesHistoryOlderThan
 
-$results = &GetBorrowersWithIssuesHistoryOlderThan($date)
+  $results = &GetBorrowersWithIssuesHistoryOlderThan($date)
 
 this function get all borrowers who has an issue history older than I<$date> given on input arg.
 
@@ -1965,7 +1945,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}
                          : "");  
@@ -1995,7 +1975,7 @@ sub GetBorrowersWithIssuesHistoryOlderThan {
 
 =head2 GetBorrowersNamesAndLatestIssue
 
-$results = &GetBorrowersNamesAndLatestIssueList(@borrowernumbers)
+  $results = &GetBorrowersNamesAndLatestIssueList(@borrowernumbers)
 
 this function get borrowers Names and surnames and Issue information.
 
@@ -2018,6 +1998,149 @@ sub GetBorrowersNamesAndLatestIssue {
     my $results = $sth->fetchall_arrayref({});
     return $results;
 }
+
+=head2 DebarMember
+
+  my $success = DebarMember( $borrowernumber );
+
+marks a Member as debarred, and therefore unable to checkout any more
+items.
+
+return :
+true on success, false on failure
+
+=cut
+
+sub DebarMember {
+    my $borrowernumber = shift;
+
+    return unless defined $borrowernumber;
+    return unless $borrowernumber =~ /^\d+$/;
+
+    return ModMember( borrowernumber => $borrowernumber,
+                      debarred       => 1 );
+    
+}
+
+=head2 AddMessage
+
+  AddMessage( $borrowernumber, $message_type, $message, $branchcode );
+
+Adds a message to the messages table for the given borrower.
+
+Returns:
+  True on success
+  False on failure
+
+=cut
+
+sub AddMessage {
+    my ( $borrowernumber, $message_type, $message, $branchcode ) = @_;
+
+    my $dbh  = C4::Context->dbh;
+
+    if ( ! ( $borrowernumber && $message_type && $message && $branchcode ) ) {
+      return;
+    }
+
+    my $query = "INSERT INTO messages ( borrowernumber, branchcode, message_type, message ) VALUES ( ?, ?, ?, ? )";
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $branchcode, $message_type, $message );
+
+    return 1;
+}
+
+=head2 GetMessages
+
+  GetMessages( $borrowernumber, $type );
+
+$type is message type, B for borrower, or L for Librarian.
+Empty type returns all messages of any type.
+
+Returns all messages for the given borrowernumber
+
+=cut
+
+sub GetMessages {
+    my ( $borrowernumber, $type, $branchcode ) = @_;
+
+    if ( ! $type ) {
+      $type = '%';
+    }
+
+    my $dbh  = C4::Context->dbh;
+
+    my $query = "SELECT
+                  branches.branchname,
+                  messages.*,
+                  DATE_FORMAT( message_date, '%m/%d/%Y' ) AS message_date_formatted,
+                  messages.branchcode LIKE '$branchcode' AS can_delete
+                  FROM messages, branches
+                  WHERE borrowernumber = ?
+                  AND message_type LIKE ?
+                  AND messages.branchcode = branches.branchcode
+                  ORDER BY message_date DESC";
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $type ) ;
+    my @results;
+
+    while ( my $data = $sth->fetchrow_hashref ) {
+        push @results, $data;
+    }
+    return \@results;
+
+}
+
+=head2 GetMessages
+
+  GetMessagesCount( $borrowernumber, $type );
+
+$type is message type, B for borrower, or L for Librarian.
+Empty type returns all messages of any type.
+
+Returns the number of messages for the given borrowernumber
+
+=cut
+
+sub GetMessagesCount {
+    my ( $borrowernumber, $type, $branchcode ) = @_;
+
+    if ( ! $type ) {
+      $type = '%';
+    }
+
+    my $dbh  = C4::Context->dbh;
+
+    my $query = "SELECT COUNT(*) as MsgCount FROM messages WHERE borrowernumber = ? AND message_type LIKE ?";
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $borrowernumber, $type ) ;
+    my @results;
+
+    my $data = $sth->fetchrow_hashref;
+    my $count = $data->{'MsgCount'};
+
+    return $count;
+}
+
+
+
+=head2 DeleteMessage
+
+  DeleteMessage( $message_id );
+
+=cut
+
+sub DeleteMessage {
+    my ( $message_id ) = @_;
+
+    my $dbh = C4::Context->dbh;
+
+    my $query = "DELETE FROM messages WHERE message_id = ?";
+    my $sth = $dbh->prepare($query);
+    $sth->execute( $message_id );
+
+}
+
 END { }    # module clean-up code here (global destructor)
 
 1;