Skip to content

Commit

Permalink
Bug 9809: Update AddReserve prototype to remove constraint parameter
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Signed-off-by: Kyle M Hall <[email protected]>

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 <[email protected]>
Verified placing a biblio level and an item level hold.

Signed-off-by: Jonathan Druart <[email protected]>
Signed-off-by: Tomas Cohen Arazi <[email protected]>
  • Loading branch information
@kylemhall @tomascohen
kylemhall authored and tomascohen committed Aug 26, 2015
1 parent a379525 commit ad32394
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 98 deletions.
4 changes: 2 additions & 2 deletions C4/ILSDI/Services.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
74 changes: 26 additions & 48 deletions C4/Reserves.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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};

Expand Down Expand Up @@ -240,9 +237,6 @@ sub AddReserve {
}
}

#}
($const eq "o" || $const eq "e") or return $reserve_id;

return $reserve_id;
}

Expand Down Expand Up @@ -302,7 +296,6 @@ sub GetReservesFromBiblionumber {
biblionumber,
borrowernumber,
reservedate,
constrainttype,
found,
itemnumber,
reservenotes,
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2238,15 +2216,15 @@ 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;
while ( my $reserve = $sth->fetchrow_hashref() ) {
$upd_sth->execute(
$priority, $to_biblio,
$reserve->{'borrowernumber'}, $reserve->{'reservedate'},
$reserve->{'constrainttype'}, $reserve->{'itemnumber'}
$reserve->{'itemnumber'}
);
$priority++;
}
Expand Down
60 changes: 30 additions & 30 deletions C4/SIP/ILS/Transaction/Hold.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion opac/opac-reserve.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ sub get_out {
if ($canreserve) {
AddReserve(
$branch, $borrowernumber,
$biblioNum, 'a',
$biblioNum,
[$biblioNum], $rank,
$startdate, $expiration_date,
$notes, $biblioData->{title},
Expand Down
18 changes: 3 additions & 15 deletions reserve/placerequest.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@
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');
Expand Down Expand Up @@ -109,19 +105,11 @@

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);
}
}

Expand Down
3 changes: 1 addition & 2 deletions serials/routing-preview.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
$count--;
}
}
my $const = 'o';
my $notes;
my $title = $subs->{'bibliotitle'};
for my $routing ( @routinglist ) {
Expand All @@ -95,7 +94,7 @@
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);
}
}
}
Expand Down

0 comments on commit ad32394

Please sign in to comment.