@@ -, +, @@
---
C4/Circulation.pm | 60 +++++++++---------
C4/Letters.pm | 2 +-
C4/Reserves.pm | 61 ++++++++++---------
Koha/Hold.pm | 13 ----
Koha/REST/V1/Holds.pm | 10 +--
Koha/Schema/Result/ClubHoldsToPatronHold.pm | 6 +-
Koha/Schema/Result/Hold.pm | 14 ++---
.../atomicupdate/bug_25260_holds_table.perl | 16 ++---
installer/data/mysql/kohastructure.sql | 10 +--
t/db_dependent/Circulation.t | 4 +-
t/db_dependent/Hold.t | 4 +-
t/db_dependent/Items/MoveItemFromBiblio.t | 6 +-
t/db_dependent/Koha/Holds.t | 2 +-
t/db_dependent/Koha/Patrons.t | 2 +-
t/db_dependent/Letters/TemplateToolkit.t | 4 +-
t/db_dependent/Reserves.t | 22 +++----
16 files changed, 112 insertions(+), 124 deletions(-)
--- a/C4/Circulation.pm
+++ a/C4/Circulation.pm
@@ -1104,47 +1104,47 @@ sub CanBookBeIssued {
{
# The item is on reserve and waiting, but has been
# reserved by some other patron.
- $needsconfirmation{RESERVE_WAITING} = 1;
- $needsconfirmation{'resfirstname'} = $patron->firstname;
- $needsconfirmation{'ressurname'} = $patron->surname;
- $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+ $needsconfirmation{RESERVE_WAITING} = 1;
+ $needsconfirmation{'resfirstname'} = $patron->firstname;
+ $needsconfirmation{'ressurname'} = $patron->surname;
+ $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
$needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
- $needsconfirmation{'resbranchcode'} = $res->{pickup_library_id};
- $needsconfirmation{'reswaitingdate'} = $res->{waiting_date};
- $needsconfirmation{'reserve_id'} = $res->{id};
+ $needsconfirmation{'resbranchcode'} = $res->{pickup_library_id};
+ $needsconfirmation{'reswaitingdate'} = $res->{waiting_date};
+ $needsconfirmation{'reserve_id'} = $res->{hold_id};
}
elsif ( $restype eq "Reserved" ) {
# The item is on reserve for someone else.
- $needsconfirmation{RESERVED} = 1;
- $needsconfirmation{'resfirstname'} = $patron->firstname;
- $needsconfirmation{'ressurname'} = $patron->surname;
- $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+ $needsconfirmation{RESERVED} = 1;
+ $needsconfirmation{'resfirstname'} = $patron->firstname;
+ $needsconfirmation{'ressurname'} = $patron->surname;
+ $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
$needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
- $needsconfirmation{'resbranchcode'} = $patron->branchcode;
- $needsconfirmation{'resreservedate'} = $res->{created_date};
- $needsconfirmation{'reserve_id'} = $res->{id};
+ $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+ $needsconfirmation{'resreservedate'} = $res->{created_date};
+ $needsconfirmation{'reserve_id'} = $res->{hold_id};
}
elsif ( $restype eq "Transferred" ) {
# The item is determined hold being transferred for someone else.
- $needsconfirmation{TRANSFERRED} = 1;
- $needsconfirmation{'resfirstname'} = $patron->firstname;
- $needsconfirmation{'ressurname'} = $patron->surname;
- $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+ $needsconfirmation{TRANSFERRED} = 1;
+ $needsconfirmation{'resfirstname'} = $patron->firstname;
+ $needsconfirmation{'ressurname'} = $patron->surname;
+ $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
$needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
- $needsconfirmation{'resbranchcode'} = $patron->branchcode;
- $needsconfirmation{'resreservedate'} = $res->{created_date};
- $needsconfirmation{'reserve_id'} = $res->{id};
+ $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+ $needsconfirmation{'resreservedate'} = $res->{created_date};
+ $needsconfirmation{'reserve_id'} = $res->{hold_id};
}
elsif ( $restype eq "Processing" ) {
# The item is determined hold being processed for someone else.
- $needsconfirmation{PROCESSING} = 1;
- $needsconfirmation{'resfirstname'} = $patron->firstname;
- $needsconfirmation{'ressurname'} = $patron->surname;
- $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+ $needsconfirmation{PROCESSING} = 1;
+ $needsconfirmation{'resfirstname'} = $patron->firstname;
+ $needsconfirmation{'ressurname'} = $patron->surname;
+ $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
$needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
- $needsconfirmation{'resbranchcode'} = $patron->branchcode;
- $needsconfirmation{'resreservedate'} = $res->{created_date};
- $needsconfirmation{'reserve_id'} = $res->{id};
+ $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+ $needsconfirmation{'resreservedate'} = $res->{created_date};
+ $needsconfirmation{'reserve_id'} = $res->{hold_id};
}
}
}
@@ -2864,10 +2864,6 @@ sub CanBookBeRenewed {
}
my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
- use Data::Printer colored => 1;
- p( $resfound );
- p( $resrec );
- p( $possible_reserves );
# If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
if ( $resfound && $resrec->{non_priority} ) {
--- a/C4/Letters.pm
+++ a/C4/Letters.pm
@@ -1669,7 +1669,7 @@ sub _get_tt_params {
module => 'Koha::Holds',
singular => 'hold',
plural => 'holds',
- pk => 'id',
+ pk => 'hold_id',
},
serial => {
module => 'Koha::Serials',
--- a/C4/Reserves.pm
+++ a/C4/Reserves.pm
@@ -633,7 +633,7 @@ sub GetOtherReserves {
#minus priorities of others reservs
ModReserveMinusPriority(
$itemnumber,
- $checkreserves->{id},
+ $checkreserves->{hold_id},
);
#launch the subroutine dotransfer
@@ -651,7 +651,7 @@ sub GetOtherReserves {
$messages->{'waiting'} = 1;
ModReserveMinusPriority(
$itemnumber,
- $checkreserves->{id},
+ $checkreserves->{hold_id},
);
ModReserveStatus($itemnumber,'waiting');
}
@@ -1094,7 +1094,7 @@ whose keys are fields from the reserves table in the Koha database.
sub ModReserveFill {
my ($res) = @_;
- my $reserve_id = $res->{'id'};
+ my $reserve_id = $res->{hold_id};
my $hold = Koha::Holds->find($reserve_id);
# get the priority on this record....
@@ -1210,7 +1210,7 @@ sub ModReserveAffect {
$hold->set_processing();
} else {
$hold->set_waiting($desk_id);
- _koha_notify_reserve( $hold->id ) unless $already_on_shelf;
+ _koha_notify_reserve( $hold->hold_id ) unless $already_on_shelf;
# Complete transfer if one exists
my $transfer = $hold->item->get_transfer;
$transfer->receive if $transfer;
@@ -1282,7 +1282,7 @@ sub ModReserveMinusPriority {
my $query = "
UPDATE holds
SET priority = 0 , item_id = ?
- WHERE id = ?
+ WHERE hold_id = ?
";
my $sth_upd = $dbh->prepare($query);
$sth_upd->execute( $item_id, $reserve_id );
@@ -1462,7 +1462,7 @@ sub ToggleLowestPriority {
my $dbh = C4::Context->dbh;
- my $sth = $dbh->prepare( "UPDATE holds SET lowest_priority = NOT lowest_priority WHERE id = ?");
+ my $sth = $dbh->prepare( "UPDATE holds SET lowest_priority = NOT lowest_priority WHERE hold_id = ?");
$sth->execute( $reserve_id );
_FixPriority({ reserve_id => $reserve_id, rank => '999999' });
@@ -1600,7 +1600,7 @@ sub _FixPriority {
my $query = "
UPDATE holds
SET priority = 0
- WHERE id = ?
+ WHERE hold_id = ?
AND status IN ('waiting', 'in_transit', 'processing')
";
my $sth = $dbh->prepare($query);
@@ -1610,7 +1610,7 @@ sub _FixPriority {
# get whats left
my $query = "
- SELECT id
+ SELECT hold_id
FROM holds
WHERE biblio_id = ?
AND completed = 0
@@ -1628,7 +1628,7 @@ sub _FixPriority {
my $i;
my $key = -1; # to allow for 0 to be a valid result
for ( $i = 0 ; $i < @priority ; $i++ ) {
- if ( $reserve_id && $reserve_id == $priority[$i]->{'id'} ) {
+ if ( $reserve_id && $reserve_id == $priority[$i]->{hold_id} ) {
$key = $i; # save the index
last;
}
@@ -1646,23 +1646,23 @@ sub _FixPriority {
$query = "
UPDATE holds
SET priority = ?
- WHERE id = ?
+ WHERE hold_id = ?
";
$sth = $dbh->prepare($query);
for ( my $j = 0 ; $j < @priority ; $j++ ) {
$sth->execute(
$j + 1,
- $priority[$j]->{'id'}
+ $priority[$j]->{hold_id}
);
}
- $sth = $dbh->prepare( "SELECT id FROM holds WHERE lowest_priority = 1 ORDER BY priority" );
+ $sth = $dbh->prepare( "SELECT hold_id FROM holds WHERE lowest_priority = 1 ORDER BY priority" );
$sth->execute();
unless ( $ignoreSetLowestRank ) {
while ( my $res = $sth->fetchrow_hashref() ) {
_FixPriority({
- reserve_id => $res->{'id'},
+ reserve_id => $res->{hold_id},
rank => '999999',
ignoreSetLowestRank => 1
});
@@ -1710,12 +1710,12 @@ sub _Findgroupreserve {
holds.timestamp AS timestamp,
biblioitems.biblioitemnumber AS biblioitemnumber,
holds.item_id AS item_id,
- holds.id AS id,
+ holds.hold_id AS hold_id,
holds.item_type AS item_type,
holds.non_priority AS non_priority
FROM holds
JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber)
- JOIN hold_fill_targets ON (holds.id=hold_fill_targets.reserve_id)
+ JOIN hold_fill_targets ON (holds.hold_id=hold_fill_targets.reserve_id)
WHERE status='placed'
AND priority > 0
AND completed = 0
@@ -1740,19 +1740,19 @@ sub _Findgroupreserve {
holds.patron_id AS patron_id,
holds.created_date AS created_date,
holds.pickup_library_id AS pickup_library_id,
- holds.completed_date AS completed_date,
+ holds.completed_date AS completed_date,
holds.status AS status,
holds.notes AS notes,
holds.priority AS priority,
holds.timestamp AS timestamp,
biblioitems.biblioitemnumber AS biblioitemnumber,
holds.item_id AS item_id,
- holds.id AS id,
+ holds.hold_id AS hold_id,
holds.item_type AS item_type,
holds.non_priority AS non_priority
FROM holds
JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber)
- JOIN hold_fill_targets ON (holds.id=hold_fill_targets.reserve_id)
+ JOIN hold_fill_targets ON (holds.hold_id=hold_fill_targets.reserve_id)
WHERE status='placed'
AND priority > 0
AND completed = 0
@@ -1783,7 +1783,7 @@ sub _Findgroupreserve {
holds.priority AS priority,
holds.timestamp AS timestamp,
holds.item_id AS item_id,
- holds.id AS id,
+ holds.hold_id AS hold_id,
holds.item_type AS item_type,
holds.non_priority AS non_priority
FROM holds
@@ -1996,7 +1996,7 @@ sub MoveReserve {
RevertWaitingStatus({ itemnumber => $itemnumber });
}
elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item
- my $hold = Koha::Holds->find( $res->{id} );
+ my $hold = Koha::Holds->find( $res->{hold_id} );
$hold->cancel;
}
}
@@ -2026,18 +2026,23 @@ sub MergeHolds {
# Reorder by date
# don't reorder those already waiting
- $sth = $dbh->prepare(
- "SELECT id FROM holds WHERE biblio_id = ? AND status = 'placed' AND completed = 0 ORDER BY created_date ASC"
- );
- my $upd_sth = $dbh->prepare(
- "UPDATE holds SET priority = ? WHERE id = ? "
- );
+ $sth = $dbh->prepare(q{
+ SELECT hold_id
+ FROM holds
+ WHERE biblio_id = ? AND status = 'placed' AND completed = 0
+ ORDER BY created_date ASC
+ });
+ my $upd_sth = $dbh->prepare(q{
+ UPDATE holds
+ SET priority = ?
+ WHERE hold_id = ?
+ });
$sth->execute( $to_biblio );
my $priority = 1;
while ( my $reserve = $sth->fetchrow_hashref() ) {
$upd_sth->execute(
$priority,
- $reserve->{'id'}
+ $reserve->{hold_id}
);
$priority++;
}
@@ -2068,7 +2073,7 @@ sub RevertWaitingStatus {
my $hold = Koha::Holds->search(
{
item_id => $item_id,
- status => 'waiting',
+ status => 'waiting',
}
)->next;
--- a/Koha/Hold.pm
+++ a/Koha/Hold.pm
@@ -632,19 +632,6 @@ sub _set_default_expiration_date {
dt_from_string( $self->created_date )->add( $timeunit => $period ) );
}
-=head3 to_api_mapping
-
-This method returns the mapping for representing a Koha::Hold object
-on the API.
-
-=cut
-
-sub to_api_mapping {
- return {
- id => 'hold_id'
- };
-}
-
=head2 Internal methods
=head3 _type
--- a/Koha/REST/V1/Holds.pm
+++ a/Koha/REST/V1/Holds.pm
@@ -248,7 +248,7 @@ sub edit {
return try {
my $hold_id = $c->validation->param('hold_id');
- my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next;
+ my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next;
unless ($hold) {
return $c->render(
@@ -313,7 +313,7 @@ sub delete {
my $c = shift->openapi->valid_input or return;
my $hold_id = $c->validation->param('hold_id');
- my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next;
+ my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next;
unless ($hold) {
return $c->render( status => 404, openapi => { error => "Hold not found." } );
@@ -342,7 +342,7 @@ sub suspend {
my $c = shift->openapi->valid_input or return;
my $hold_id = $c->validation->param('hold_id');
- my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next;
+ my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next;
my $body = $c->req->json;
my $end_date = ($body) ? $body->{end_date} : undef;
@@ -390,7 +390,7 @@ sub resume {
my $c = shift->openapi->valid_input or return;
my $hold_id = $c->validation->param('hold_id');
- my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next;
+ my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next;
my $body = $c->req->json;
unless ($hold) {
@@ -416,7 +416,7 @@ sub update_priority {
my $c = shift->openapi->valid_input or return;
my $hold_id = $c->validation->param('hold_id');
- my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next;
+ my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next;
unless ($hold) {
return $c->render(
--- a/Koha/Schema/Result/ClubHoldsToPatronHold.pm
+++ a/Koha/Schema/Result/ClubHoldsToPatronHold.pm
@@ -135,7 +135,7 @@ Related object: L Notes:
-<
+<
Notes: -
[% hold.id %]=[% hold.notes %]+
[% hold.hold_id %]=[% hold.notes %]EOF --- a/t/db_dependent/Reserves.t +++ a/t/db_dependent/Reserves.t @@ -123,7 +123,7 @@ my ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber, $barcod is($status, "Reserved", "CheckReserves Test 1"); -ok(exists($reserve->{id}), 'CheckReserves() include reserve_id in its response'); +ok(exists($reserve->{hold_id}), 'CheckReserves() include reserve_id in its response'); ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber); is($status, "Reserved", "CheckReserves Test 2"); @@ -238,7 +238,7 @@ $itemnum_fpl = $builder->build_sample_item( # Ensure that priorities are numbered correcly when a hold is moved to waiting # (bug 11947) -$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); +$dbh->do("DELETE FROM holds WHERE biblio_id=?",undef,($bibnum2)); AddReserve( { branchcode => $branch_3, @@ -267,7 +267,7 @@ ModReserveAffect($itemnum_cpl, $requesters{$branch_3}, 0); # Now it should have different priorities. my $biblio = Koha::Biblios->find( $bibnum2 ); -my $holds = $biblio->holds({ completed => 0 }, { order_by => 'id' });; +my $holds = $biblio->holds({ completed => 0 }, { order_by => 'hold_id' });; is($holds->next->priority, 0, 'Item is correctly waiting'); is($holds->next->priority, 1, 'Item is correctly priority 1'); is($holds->next->priority, 2, 'Item is correctly priority 2'); @@ -277,7 +277,7 @@ is( @reserves, 1, 'GetWaiting got only the waiting reserve' ); is( $reserves[0]->patron_id(), $requesters{$branch_3}, 'GetWaiting got the reserve for the correct borrower' ); -$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); +$dbh->do("DELETE FROM holds WHERE biblio_id=?",undef,($bibnum2)); AddReserve( { branchcode => $branch_3, @@ -533,7 +533,7 @@ is( CanReserveBeCanceledFromOpac(), undef, 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' ); is( - CanReserveBeCanceledFromOpac( $canres->{id} ), + CanReserveBeCanceledFromOpac( $canres->{hold_id} ), undef, 'CanReserveBeCanceledFromOpac should return undef if called without the id' ); @@ -543,18 +543,18 @@ is( 'CanReserveBeCanceledFromOpac should return undef if called without borrowernumber' ); -my $cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); +my $cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}); is($cancancel, 1, 'Can user cancel its own reserve'); -$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_2}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_2}); is($cancancel, 0, 'Other user cant cancel reserve'); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 1); -$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}); is($cancancel, 0, 'Reserve in transfer status cant be canceled'); $dbh->do('DELETE FROM holds', undef, ($bibnum)); -is( CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}), undef, +is( CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}), undef, 'Cannot cancel a deleted hold' ); AddReserve( @@ -568,7 +568,7 @@ AddReserve( (undef, $canres, undef) = CheckReserves($item->itemnumber); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); -$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}); is($cancancel, 0, 'Reserve in waiting status cant be canceled'); # End of tests for bug 12876 @@ -1140,7 +1140,7 @@ subtest 'CheckReserves additional tests' => sub { my ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item->itemnumber ); is( $status, 'Transferred', "We found a reserve" ); - is( $matched_reserve->{id}, $reserve1->id, 'We got the Transit reserve' ); + is( $matched_reserve->{hold_id}, $reserve1->id, 'We got the Transit reserve' ); is( scalar @$possible_reserves, 2, 'We do get both reserves' ); my $patron_B = $builder->build_object({ class => "Koha::Patrons" }); --