Attachment 41897 Details for Bug 9809 – Bug 9809 - Update AddReserve prototype to remove constraint parameter

[patch] Bug 9809 - Update AddReserve prototype to remove constraint parameter

Bug-9809---Update-AddReserve-prototype-to-remove-c.patch (text/plain), 14.63 KB, created by Jonathan Druart on 2015-08-25 11:37:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2015-08-25 11:37:20 UTC

Size: 14.63 KB

patch

obsolete

>From 8e25a672f3b267a5d8ef727521ef3d9b41267fb0 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Tue, 30 Jun 2015 08:19:29 -0400
>Subject: [PATCH] Bug 9809 - Update AddReserve prototype to remove constraint
> parameter
>
>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>
>---
> C4/ILSDI/Services.pm           |  4 +--
> C4/Reserves.pm                 | 74 +++++++++++++++---------------------------
> C4/SIP/ILS/Transaction/Hold.pm | 60 +++++++++++++++++-----------------
> opac/opac-reserve.pl           |  2 +-
> reserve/placerequest.pl        | 18 ++--------
> serials/routing-preview.pl     |  3 +-
> 6 files changed, 63 insertions(+), 98 deletions(-)
>
>diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm
>index 5d3f882..e69c070 100644
>--- a/C4/ILSDI/Services.pm
>+++ b/C4/ILSDI/Services.pm
>@@ -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;
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 4e27748..f0fc845 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -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++;
>         }
>diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm
>index 0e00d3d..fa2e5ba 100644
>--- a/C4/SIP/ILS/Transaction/Hold.pm
>+++ b/C4/SIP/ILS/Transaction/Hold.pm
>@@ -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 {
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index 68abc0a..6e6e928 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -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},
>diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl
>index 1fc2436..6cbaa7b 100755
>--- a/reserve/placerequest.pl
>+++ b/reserve/placerequest.pl
>@@ -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);
>         }
>     }
> 
>diff --git a/serials/routing-preview.pl b/serials/routing-preview.pl
>index 1fd5814..13bca6c 100755
>--- a/serials/routing-preview.pl
>+++ b/serials/routing-preview.pl
>@@ -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);
>         }
>     }
> 	}
>-- 
>2.1.0

Actions: View | Diff | Splinter Review

Attachments on bug 9809: 37638 | 40733 | 40734 | 40735 | 40736 | 40737 | 40738 | 41754 | 41755 | 41756 | 41757 | 41758 | 41759 | 41760 | 41761 | 41767 | 41768 | 41769 | 41770 | 41771 | 41772 | 41773 | 41774 | 41809 | 41810 | 41811 | 41812 | 41813 | 41814 | 41815 | 41816 | 41817 | 41845 | 41848 | 41849 | 41850 | 41851 | 41852 | 41892 | 41893 | 41894 | 41895 | 41896 | 41897 | 41898 | 41899 | 41900 | 41901 | 41902 | 41903