Bug 9809: Update AddReserve prototype to remove constraint parameter
authorKyle M Hall <kyle@bywatersolutions.com>
Tue, 30 Jun 2015 12:19:29 +0000 (08:19 -0400)
committerTomas Cohen Arazi <tomascohen@theke.io>
Wed, 26 Aug 2015 13:26:43 +0000 (10:26 -0300)
Test Plan:
1) Apply this patch set
2) prove t/db_dependent/Circulation.t
3) prove t/db_dependent/Holds.t
4) prove t/db_dependent/Holds/LocalHoldsPriority.t
5) prove t/db_dependent/Holds/RevertWaitingStatus.t
6) prove t/db_dependent/HoldsQueue.t
7) prove t/db_dependent/Reserves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

AMENDED: An else branch in reserve/placerequest.pl was removed. This had
the effect of making it no longer possible to place an any hold in the
staff client.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified placing a biblio level and an item level hold.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

C4/ILSDI/Services.pm
C4/Reserves.pm
C4/SIP/ILS/Transaction/Hold.pm
opac/opac-reserve.pl
reserve/placerequest.pl
serials/routing-preview.pl

index 5d3f882..e69c070 100644 (file)
@@ -627,7 +627,7 @@ sub HoldTitle {
     #    $constraint, $bibitems,  $priority, $resdate, $expdate, $notes,
     #    $title,      $checkitem, $found
     my $priority= C4::Reserves::CalculatePriority( $biblionumber );
-    AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, $priority, undef, undef, undef, $title, undef, undef );
+    AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, undef, undef );
 
     # Hashref building
     my $out;
@@ -704,7 +704,7 @@ sub HoldItem {
     #    $constraint, $bibitems,  $priority, $resdate, $expdate, $notes,
     #    $title,      $checkitem, $found
     my $priority= C4::Reserves::CalculatePriority( $biblionumber );
-    AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, $priority, undef, undef, undef, $title, $itemnumber, undef );
+    AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, $itemnumber, undef );
 
     # Hashref building
     my $out;
index 4e27748..f0fc845 100644 (file)
@@ -146,21 +146,20 @@ BEGIN {
 
 =head2 AddReserve
 
-    AddReserve($branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
+    AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
 
 =cut
 
 sub AddReserve {
     my (
         $branch,    $borrowernumber, $biblionumber,
-        $constraint, $bibitems,  $priority, $resdate, $expdate, $notes,
+        $bibitems,  $priority, $resdate, $expdate, $notes,
         $title,      $checkitem, $found
     ) = @_;
     my $fee =
-          GetReserveFee($borrowernumber, $biblionumber, $constraint,
+          GetReserveFee($borrowernumber, $biblionumber,
             $bibitems );
     my $dbh     = C4::Context->dbh;
-    my $const   = lc substr( $constraint, 0, 1 );
     $resdate = format_date_in_iso( $resdate ) if ( $resdate );
     $resdate = C4::Dates->today( 'iso' ) unless ( $resdate );
     if ($expdate) {
@@ -194,20 +193,18 @@ sub AddReserve {
             "Reserve Charge - $title", $fee );
     }
 
-    #if ($const eq 'a'){
     my $query = qq{
         INSERT INTO reserves
-            (borrowernumber,biblionumber,reservedate,branchcode,constrainttype,
+            (borrowernumber,biblionumber,reservedate,branchcode,
             priority,reservenotes,itemnumber,found,waitingdate,expirationdate)
         VALUES
              (?,?,?,?,?,
-             ?,?,?,?,?,?)
+             ?,?,?,?,?)
              };
     my $sth = $dbh->prepare($query);
     $sth->execute(
-        $borrowernumber, $biblionumber, $resdate, $branch,
-        $const,          $priority,     $notes,   $checkitem,
-        $found,          $waitingdate, $expdate
+        $borrowernumber, $biblionumber, $resdate, $branch,      $priority,
+        $notes,          $checkitem,    $found,   $waitingdate, $expdate
     );
     my $reserve_id = $sth->{mysql_insertid};
 
@@ -240,9 +237,6 @@ sub AddReserve {
         }
     }
 
-    #}
-    ($const eq "o" || $const eq "e") or return $reserve_id;
-
     return $reserve_id;
 }
 
@@ -302,7 +296,6 @@ sub GetReservesFromBiblionumber {
                 biblionumber,
                 borrowernumber,
                 reservedate,
-                constrainttype,
                 found,
                 itemnumber,
                 reservenotes,
@@ -659,18 +652,17 @@ sub GetOtherReserves {
 
 =head2 GetReserveFee
 
-  $fee = GetReserveFee($borrowernumber,$biblionumber,$constraint,$biblionumber);
+  $fee = GetReserveFee($borrowernumber,$biblionumber,$biblionumber);
 
 Calculate the fee for a reserve
 
 =cut
 
 sub GetReserveFee {
-    my ($borrowernumber, $biblionumber, $constraint, $bibitems ) = @_;
+    my ($borrowernumber, $biblionumber, $bibitems ) = @_;
 
     #check for issues;
     my $dbh   = C4::Context->dbh;
-    my $const = lc substr( $constraint, 0, 1 );
     my $query = qq{
       SELECT * FROM borrowers
     LEFT JOIN categories ON borrowers.categorycode = categories.categorycode
@@ -693,30 +685,19 @@ sub GetReserveFee {
         );
         $sth1->execute($biblionumber);
         while ( my $data1 = $sth1->fetchrow_hashref ) {
-            if ( $const eq "a" ) {
-                push @biblioitems, $data1;
-            }
-            else {
-                my $found = 0;
-                my $x     = 0;
-                while ( $x < $cntitems ) {
-                    if ( @$bibitems->{'biblioitemnumber'} ==
-                        $data->{'biblioitemnumber'} )
-                    {
-                        $found = 1;
-                    }
-                    $x++;
-                }
-                if ( $const eq 'o' ) {
-                    if ( $found == 1 ) {
-                        push @biblioitems, $data1;
-                    }
-                }
-                else {
-                    if ( $found == 0 ) {
-                        push @biblioitems, $data1;
-                    }
+            my $found = 0;
+            my $x     = 0;
+            while ( $x < $cntitems ) {
+                if ( @$bibitems->{'biblioitemnumber'} ==
+                    $data->{'biblioitemnumber'} )
+                {
+                    $found = 1;
                 }
+                $x++;
+            }
+
+            if ( $found == 0 ) {
+                push @biblioitems, $data1;
             }
         }
         my $cntitemsfound = @biblioitems;
@@ -1795,7 +1776,7 @@ sub _FixPriority {
 
     # get whats left
     my $query = "
-        SELECT reserve_id, borrowernumber, reservedate, constrainttype
+        SELECT reserve_id, borrowernumber, reservedate
         FROM   reserves
         WHERE  biblionumber   = ?
           AND  ((found <> 'W' AND found <> 'T') OR found IS NULL)
@@ -1857,13 +1838,10 @@ sub _FixPriority {
 
   @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead, $ignore_borrowers);
 
-Looks for an item-specific match first, then for a title-level match, returning the
-first match found.  If neither, then we look for a 3rd kind of match based on
-reserve constraints.
+Looks for a holds-queue based item-specific match first, then for a holds-queue title-level match, returning the
+first match found.  If neither, then we look for non-holds-queue based holds.
 Lookahead is the number of days to look in advance.
 
-TODO: add more explanation about reserve constraints
-
 C<&_Findgroupreserve> returns :
 C<@results> is an array of references-to-hash whose keys are mostly
 fields from the reserves table of the Koha database, plus
@@ -2238,7 +2216,7 @@ sub MergeHolds {
         );
         my $upd_sth = $dbh->prepare(
 "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ?
-        AND reservedate = ? AND constrainttype = ? AND (itemnumber = ? or itemnumber is NULL) "
+        AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) "
         );
         $sth->execute( $to_biblio, 'W', 'T' );
         my $priority = 1;
@@ -2246,7 +2224,7 @@ sub MergeHolds {
             $upd_sth->execute(
                 $priority,                    $to_biblio,
                 $reserve->{'borrowernumber'}, $reserve->{'reservedate'},
-                $reserve->{'constrainttype'}, $reserve->{'itemnumber'}
+                $reserve->{'itemnumber'}
             );
             $priority++;
         }
index 0e00d3d..fa2e5ba 100644 (file)
@@ -38,36 +38,36 @@ sub queue_position {
 }
 
 sub do_hold {
-       my $self = shift;
-       unless ($self->{patron}) {
-               $self->screen_msg('do_hold called with undefined patron');
-               $self->ok(0);
-               return $self;
-       }
-       my $borrower = GetMember( 'cardnumber'=>$self->{patron}->id);
-       unless ($borrower) {
-               $self->screen_msg('No borrower matches cardnumber "' . $self->{patron}->id . '".');
-               $self->ok(0);
-               return $self;
-       }
-       my $bib = GetBiblioFromItemNumber(undef, $self->{item}->id);
-       unless ($bib) {
-               $self->screen_msg('No biblio record matches barcode "' . $self->{item}->id . '".');
-               $self->ok(0);
-               return $self;
-       }
-       my $branch = ($self->pickup_location || $self->{patron}->branchcode);
-       unless ($branch) {
-               $self->screen_msg('No branch specified (or found w/ patron).');
-               $self->ok(0);
-               return $self;
-       }
-       my $bibno = $bib->{biblionumber};
-       AddReserve($branch, $borrower->{borrowernumber}, 
-                               $bibno, 'a', GetBiblioItemByBiblioNumber($bibno)) ;
-               # unfortunately no meaningful return value
-       $self->ok(1);
-       return $self;
+    my $self = shift;
+    unless ( $self->{patron} ) {
+        $self->screen_msg('do_hold called with undefined patron');
+        $self->ok(0);
+        return $self;
+    }
+    my $borrower = GetMember( 'cardnumber' => $self->{patron}->id );
+    unless ($borrower) {
+        $self->screen_msg( 'No borrower matches cardnumber "' . $self->{patron}->id . '".' );
+        $self->ok(0);
+        return $self;
+    }
+    my $bib = GetBiblioFromItemNumber( undef, $self->{item}->id );
+    unless ($bib) {
+        $self->screen_msg( 'No biblio record matches barcode "' . $self->{item}->id . '".' );
+        $self->ok(0);
+        return $self;
+    }
+    my $branch = ( $self->pickup_location || $self->{patron}->branchcode );
+    unless ($branch) {
+        $self->screen_msg('No branch specified (or found w/ patron).');
+        $self->ok(0);
+        return $self;
+    }
+    my $bibno = $bib->{biblionumber};
+    AddReserve( $branch, $borrower->{borrowernumber}, $bibno, GetBiblioItemByBiblioNumber($bibno) );
+
+    # unfortunately no meaningful return value
+    $self->ok(1);
+    return $self;
 }
 
 sub drop_hold {
index 68abc0a..6e6e928 100755 (executable)
@@ -285,7 +285,7 @@ if ( $query->param('place_reserve') ) {
         if ($canreserve) {
             AddReserve(
                 $branch,      $borrowernumber,
-                $biblioNum,   'a',
+                $biblioNum,
                 [$biblioNum], $rank,
                 $startdate,   $expiration_date,
                 $notes,       $biblioData->{title},
index 1fc2436..6cbaa7b 100755 (executable)
@@ -38,10 +38,6 @@ my $input = CGI->new();
 checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet');
 
 my @bibitems=$input->param('biblioitem');
-# FIXME I think reqbib does not exist anymore, it's used in line 82, to AddReserve of contraint type 'o'
-#       I bet it's a 2.x feature, reserving a given biblioitem, that is useless in Koha 3.0
-#       we can remove this line, the AddReserve of constrainttype 'o',
-#       and probably remove the reserveconstraint table as well, I never could fill anything in this table.
 my @reqbib=$input->param('reqbib'); 
 my $biblionumber=$input->param('biblionumber');
 my $borrowernumber=$input->param('borrowernumber');
@@ -109,19 +105,11 @@ if ($type eq 'str8' && $borrower){
 
         if ($multi_hold) {
             my $bibinfo = $bibinfos{$biblionumber};
-            AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',[$biblionumber],
+            AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,[$biblionumber],
                        $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found);
         } else {
-            if ($input->param('request') eq 'any'){
-                # place a request on 1st available
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found);
-            } elsif ($reqbib[0] ne ''){
-                # FIXME : elsif probably never reached, (see top of the script)
-                # place a request on a given item
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found);
-            } else {
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found);
-            }
+            # place a request on 1st available
+            AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found);
         }
     }
 
index 1fd5814..13bca6c 100755 (executable)
@@ -79,7 +79,6 @@ if($ok){
                                $count--;
                        }
                }
-               my $const = 'o';
                my $notes;
                my $title = $subs->{'bibliotitle'};
         for my $routing ( @routinglist ) {
@@ -95,7 +94,7 @@ if($ok){
                     branchcode     => $branch
                 });
             } else {
-                AddReserve($branch,$routing->{borrowernumber},$biblio,$const,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title);
+                AddReserve($branch,$routing->{borrowernumber},$biblio,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title);
         }
     }
        }