Skip to content

GitLab

Commit f5da9491 authored by Benjamin Rokseth's avatar Benjamin Rokseth Committed by Benjamin Rokseth
Browse files

koha: attempt to fix fillhold issue 

koha: Deichman::Patron::Reserve->Update more solid

make sure not to update with undef or invalid refs
parent 586ac4c2
Hide whitespace changes
Inline Side-by-side
koha/Core/Circulation.pm
View file @ f5da9491
......@@ -12,6 +12,8 @@ use DateTime::Duration;
use DateTime::Format::Strptime;
use Deichman::Item;
use Deichman::Patron;
use Deichman::Patron::Reserve;
use Koha::Holds;
use Koha::Items;
use Koha::Item;
......@@ -129,16 +131,16 @@ sub Checkin {
}
}
# HOLDS?
# HOLDS? GET FIRST PENDING AND SEE IF ITEM CAN FILL IT
my $hold = $self->getPendingHold();
if ($hold and $self->canItemFillHold()) {
if ($hold->branchcode ne $self->{library}->branchcode) {
$self->{messages}->{NeedsTransfer} = $hold->branchcode;
if ($hold and $self->canItemFillHold($hold)) {
if ($hold->{res}->{branchcode} ne $self->{library}->branchcode) {
$self->{messages}->{NeedsTransfer} = $hold->{res}->{branchcode};
}
# Weird message return yes, but this is how Koha templates expect it
my $status = $hold->is_waiting ? "Waiting" : "Reserved";
my $status = $hold->isWaiting() ? "Waiting" : "Reserved";
$self->{messages}->{ResFound} = {
%{$hold->unblessed},
%{$hold->{res}},
%{$self->{item}->unblessed},
ResFound => $status,
};
......@@ -181,7 +183,6 @@ sub Checkin {
$self->{checkout}->patron and $self->{item}->last_returned_by( $self->{checkout}->patron );
}
my $deleted = $self->{checkout}->delete();
if ($deleted != 1) {
$self->{error} = Core::Exception::Circulation::Checkin::Failed->new();
$self->{messages}->{WasReturned} = 0;
......@@ -230,7 +231,7 @@ sub Checkin {
datelastseen => _today(),
itemlost => 0,
itemlost_on => undef,
notforloan => $self->{notforloan} == -1 ? 0 : $self->{notforloan}, # Unset 'til klargjøring' status
notforloan => $self->{item}->notforloan == -1 ? 0 : $self->{item}->notforloan, # Unset 'til klargjøring' status
})->store;
my $dt = Time::HiRes::time() - $t0;
......@@ -297,11 +298,9 @@ sub Checkout {
my $hold = $self->getPendingHold();
if ($hold) {
# Right patron - fill hold
if ($hold->borrowernumber == $self->{patron}->borrowernumber) {
$hold->set({found => "F", priority => 0});
# Merge tables - PLEASE!
Koha::Old::Hold->new( $hold->unblessed() )->store();
$hold->delete();
if ($hold->{res}->{borrowernumber} == $self->{patron}->borrowernumber) {
$hold->SetFound();
$hold->Delete() # This moves it to old_reserves Merge tables - PLEASE!
} else {
# TODO: Checkout should be aborted here?
$self->{error} = Core::Exception::Circulation::Checkout::HoldForAnother->new();
......@@ -748,19 +747,19 @@ sub canItemBeIssued {
unless ( $ignoreReserves ) {
my $hold = $self->getPendingHold();
if ($hold) {
if ($hold->is_waiting) {
if ($hold->isWaiting()) {
$needsconfirmation->{RESERVE_WAITING} = 1;
$needsconfirmation->{reswaitingdate} = $hold->waitingdate;
$needsconfirmation->{reswaitingdate} = $hold->{waitingdate};
} else {
$needsconfirmation->{RESERVED} = 1;
}
$needsconfirmation->{resfirstname} = $hold->borrower->firstname;
$needsconfirmation->{ressurname} = $hold->borrower->surname;
$needsconfirmation->{rescardnumber} = $hold->borrower->cardnumber;
$needsconfirmation->{resborrowernumber} = $hold->borrower->borrowernumber;
$needsconfirmation->{resbranchcode} = $hold->borrower->branchcode;
$needsconfirmation->{resreservedate} = $hold->reservedate;
my $p = Deichman::Patron->new()->Get($hold->{res}->{borrowernumber});
$needsconfirmation->{resfirstname} = $p->{patron}->{firstname};
$needsconfirmation->{ressurname} = $p->{patron}->{surname};
$needsconfirmation->{rescardnumber} = $p->{patron}->{cardnumber};
$needsconfirmation->{resborrowernumber} = $hold->{res}->{borrowernumber};
$needsconfirmation->{resbranchcode} = $hold->{res}->{branchcode};
$needsconfirmation->{resreservedate} = $hold->{res}->{reservedate};
}
}
......@@ -875,31 +874,14 @@ sub canItemBeReturned {
=head
Check if item can fill a pending hold
reservable means:
- item is not on item level hold
- item not found (Waiting or in Transfer)
- item not lost, withdrawn
- item notforloan not code > 0
- item not damaged (unless AllowHoldsOnDamagedItems)
- number of reservable items is greater than number of pending holds
- something something onshelfholds
ref: C4::Reserves::CanItemBeReserved L282
=cut
sub canItemFillHold {
my $self = shift;
if ($self->{item}->itype =~ /10DLAAN|DAGSLAAN|TOUKESLAAN|UKESLAAN/) {
return;
}
my $reservable;
try {
my $it = Deichman::Item->new()->Get($self->{item}->itemnumber);
my ($available, $reserves) = $it->reservableItems();
$reservable = $available > 0;
} catch {
warn $_;
};
return $reservable;
my ($self, $hold) = @_;
# item level hold for this item
($self->{item}->itemnumber != $hold->{res}->{itemnumber}) and return 1;
# item type disallowed
($self->{item}->itype =~ /10DLAAN|DAGSLAAN|TOUKESLAAN|UKESLAAN/) and return 0;
return 1;
}
=head
get soonest renewal date from date and circ rule
......@@ -1034,9 +1016,26 @@ sub getIssuingCharges {
=cut
sub getPendingHold {
my $self = shift;
my $itemholds = $self->{item}->current_holds;
my $biblioholds = Koha::Holds->search({biblionumber => $self->{item}->biblionumber, found => undef, suspend => 0}, { order_by => { -asc => "priority" }});
my $hold = $itemholds->count ? $itemholds->next : $biblioholds->count ? $biblioholds->next : undef;
my $hold;
try {
my $item = Deichman::Item->new()->Get($self->{item}->itemnumber);
$item->GetItemHolds();
$item->GetBiblioHolds();
if (scalar @{ $item->{itemHolds} } > 0) {
my $ih = $item->{itemHolds}->[0];
$hold = Deichman::Patron::Reserve->new()->Get($ih->{reserve_id});
}
if (scalar @{ $item->{biblioHolds} } > 0) {
my $bh = $item->{biblioHolds}->[0];
$hold = Deichman::Patron::Reserve->new()->Get($bh->{reserve_id});
}
} catch {
warn $_;
};
# my $itemholds = $self->{item}->current_holds;
# my $biblioholds = Koha::Holds->search({biblionumber => $self->{item}->biblionumber, found => undef, suspend => 0}, { order_by => { -asc => "priority" }});
# my $hold = $itemholds->count ? $itemholds->next : $biblioholds->count ? $biblioholds->next : undef;
return $hold;
}
......
koha/Deichman/Item.pm
View file @ f5da9491
......@@ -75,7 +75,7 @@ sub GetItemHolds {
my ($self) = @_;
return unless $self->{item};
my $dbh = $self->dbh;
my $q = "SELECT * FROM reserves WHERE itemnumber = ? AND found IS NULL AND suspend = 0";
my $q = "SELECT * FROM reserves WHERE itemnumber = ? AND suspend = 0";
my $sth = $dbh->prepare($q);
$sth->execute($self->{item}->{itemnumber}) or Deichman::Exception::Item->throw($dbh->errstr);
my @itemHolds;
......@@ -90,7 +90,7 @@ sub GetBiblioHolds {
my ($self) = @_;
return unless $self->{item};
my $dbh = $self->dbh;
my $q = "SELECT * FROM reserves WHERE biblionumber = ? AND found IS NULL AND suspend = 0 ORDER BY priority ASC";
my $q = "SELECT * FROM reserves WHERE biblionumber = ? AND suspend = 0 ORDER BY priority ASC";
my $sth = $dbh->prepare($q);
$sth->execute($self->{item}->{biblionumber}) or Deichman::Exception::Item->throw($dbh->errstr);
my @biblioHolds;
......@@ -101,6 +101,10 @@ sub GetBiblioHolds {
return $self;
}
=head
Getter for reservable items count (not used):
ref: C4::Reserves::CanItemBeReserved L282
=cut
sub reservableItems {
my ($self) = @_;
return unless $self->{item};
......
koha/Deichman/Patron/Reserve.pm
View file @ f5da9491
......@@ -18,7 +18,7 @@ sub new {
sub Read {
my ($self) = @_;
my $q = "SELECT * FROM reserves WHERE id = ?";
my $q = "SELECT * FROM reserves WHERE reserve_id = ?";
my $dbh = $self->dbh;
my $sth = $dbh->prepare($q);
$sth->execute($self->{res}->{reserve_id}) or Deichman::Exception::Reserve->throw($dbh->errstr);
......@@ -29,7 +29,7 @@ sub Read {
sub Get {
my ($self, $id) = @_;
my $q = "SELECT * FROM reserves WHERE id = ?";
my $q = "SELECT * FROM reserves WHERE reserve_id = ?";
my $dbh = $self->dbh;
my $sth = $dbh->prepare($q);
$sth->execute($id) or Deichman::Exception::Reserve->throw($dbh->errstr);
......@@ -64,10 +64,18 @@ sub Add {
}
sub Update {
my ($self, $reserve) = @_;
$reserve or Deichman::Exception::Reserve::ArgumentError->throw();
my ($self, $res) = @_;
$res ||= $self->{res};
# Get original and merge in new props from updated reserve
# TODO: this should be part of dbObject method
my $orig = Deichman::Patron::Reserve->new()->Get($res->{reserve_id});
my $update = $orig->{res};
foreach my $key (keys %{$update}) {
$res->{$key} or delete $update->{$key};
$update->{$key} = $res->{$key} if $res->{$key};
}
my $dbh = $self->dbh;
my $obj = $self->dbObject($reserve);
my $obj = $self->dbObject($update);
my $q = "UPDATE reserves SET "
. $obj->{updateString}
. " WHERE reserve_id = ?";
......@@ -76,15 +84,30 @@ sub Update {
return $self->Read;
}
# Please merge tables!
sub Delete {
my ($self) = @_;
$self->{res} or Deichman::Exception::Reserve::NotFound->throw();
my $obj = $self->dbObject($self->{res});
my $dbh = $self->dbh;
my $q = "DELETE FROM reserves WHERE reserve_id = ?";
my $q = "INSERT INTO old_reserves ($obj->{insertFields}) VALUES ($obj->{insertBinds})";
my $sth = $dbh->prepare($q);
$sth->execute( @{$obj->{insertValues}} ) or Deichman::Exception::Reserve->throw($dbh->errstr);
$q = "DELETE FROM reserves WHERE reserve_id = ?";
$sth = $dbh->prepare($q);
$sth->execute($self->{res}->{reserve_id}) or Deichman::Exception::Reserve->throw($dbh->errstr);
return $self;
}
sub SetFound {
my ($self) = @_;
$self->{res} or Deichman::Exception::Reserve::NotFound->throw();
$self->{res}->{found} = "F";
$self->{res}->{priority} = 0;
$self->Update();
return $self;
}
# Util functions
sub isWaiting {
my ($self) = @_;
......
koha/circ/returns.pl
View file @ f5da9491
......@@ -623,14 +623,16 @@ foreach ( sort { $a <=> $b } keys %returneditems ) {
} else {
$ri{return_overdue} = 1 if (DateTime->compare($duedate, $dropboxdate) == -1);
}
$ri{borrowernumber} = $patron->borrowernumber;
$ri{borcnum} = $patron->cardnumber;
$ri{borfirstname} = $patron->firstname;
$ri{borsurname} = $patron->surname;
$ri{bortitle} = $patron->title;
$ri{bornote} = $patron->borrowernotes;
$ri{borcategorycode}= $patron->categorycode;
$ri{borissuescount} = $patron->checkouts->count;
if ($patron) {
$ri{borrowernumber} = $patron->borrowernumber;
$ri{borcnum} = $patron->cardnumber;
$ri{borfirstname} = $patron->firstname;
$ri{borsurname} = $patron->surname;
$ri{bortitle} = $patron->title;
$ri{bornote} = $patron->borrowernotes;
$ri{borcategorycode}= $patron->categorycode;
$ri{borissuescount} = $patron->checkouts->count;
}
}
else {
$ri{borrowernumber} = $riborrowernumber{$_};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment