From 73a29574e4a23a3905c557a8eb9a14e58a7d3ed4 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 17 Apr 2020 18:44:35 -0300 Subject: [PATCH] Bug 25260: Adapt Koha::Hold(s) Signed-off-by: Tomas Cohen Arazi --- Koha/Hold.pm | 131 ++++++++++++++++++++------------------------------ Koha/Holds.pm | 22 ++++++--- 2 files changed, 68 insertions(+), 85 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 6896f5e94d..cde045455a 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -64,11 +64,11 @@ sub age { my $age; if ( $use_calendar ) { - my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); - $age = $calendar->days_between( dt_from_string( $self->reservedate ), $today ); + my $calendar = Koha::Calendar->new( branchcode => $self->pickup_library_id ); + $age = $calendar->days_between( dt_from_string( $self->placing_date ), $today ); } else { - $age = $today->delta_days( dt_from_string( $self->reservedate ) ); + $age = $today->delta_days( dt_from_string( $self->placing_date ) ); } $age = $age->in_units( 'days' ); @@ -88,27 +88,25 @@ sub suspend_hold { my $date = $dt ? $dt->clone()->truncate( to => 'day' )->datetime : undef; if ( $self->is_found ) { # We can't suspend found holds - if ( $self->is_waiting ) { - Koha::Exceptions::Hold::CannotSuspendFound->throw( status => 'W' ); - } - elsif ( $self->is_in_transit ) { - Koha::Exceptions::Hold::CannotSuspendFound->throw( status => 'T' ); + if ( $self->is_waiting or + $self->is_in_transit ) { + Koha::Exceptions::Hold::CannotSuspendFound->throw( status => $self->status ); } else { Koha::Exceptions::Hold::CannotSuspendFound->throw( 'Unhandled data exception on found hold (id=' . $self->id - . ', found=' - . $self->found + . ', status=' + . $self->status . ')' ); } } - $self->suspend(1); - $self->suspend_until($date); + $self->suspended(1); + $self->suspended_until_date($date); $self->store(); - logaction( 'HOLDS', 'SUSPEND', $self->reserve_id, Dumper( $self->unblessed ) ) + logaction( 'HOLDS', 'SUSPEND', $self->id, Dumper( $self->unblessed ) ) if C4::Context->preference('HoldsLog'); return $self; @@ -123,12 +121,12 @@ my $hold = $hold->resume(); sub resume { my ( $self ) = @_; - $self->suspend(0); - $self->suspend_until( undef ); + $self->suspended(0); + $self->suspended_until_date( undef ); $self->store(); - logaction( 'HOLDS', 'RESUME', $self->reserve_id, Dumper($self->unblessed) ) + logaction( 'HOLDS', 'RESUME', $self->id, Dumper($self->unblessed) ) if C4::Context->preference('HoldsLog'); return $self; @@ -145,7 +143,7 @@ sub delete { my $deleted = $self->SUPER::delete($self); - logaction( 'HOLDS', 'DELETE', $self->reserve_id, Dumper($self->unblessed) ) + logaction( 'HOLDS', 'DELETE', $self->id, Dumper($self->unblessed) ) if C4::Context->preference('HoldsLog'); return $deleted; @@ -161,24 +159,24 @@ sub set_waiting { $self->priority(0); if ($transferToDo) { - $self->found('T')->store(); + $self->status('in_transit')->store(); return $self; } my $today = dt_from_string(); my $values = { - found => 'W', - waitingdate => $today->ymd, + status => 'waiting', + waiting_date => $today->ymd, }; my $requested_expiration; - if ($self->expirationdate) { - $requested_expiration = dt_from_string($self->expirationdate); + if ($self->expiration_date) { + $requested_expiration = dt_from_string($self->expiration_date); } my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); - my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); + my $calendar = Koha::Calendar->new( branchcode => $self->pickup_library_id ); my $expirationdate = $today->clone; $expirationdate->add(days => $max_pickup_delay); @@ -190,7 +188,7 @@ sub set_waiting { # If patron's requested expiration date is prior to the # calculated one, we keep the patron's one. my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; - $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; + $values->{expiration_date} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; $self->set($values)->store(); @@ -206,9 +204,13 @@ Returns true if hold is a waiting or in transit sub is_found { my ($self) = @_; - return 0 unless $self->found(); - return 1 if $self->found() eq 'W'; - return 1 if $self->found() eq 'T'; + if ( $self->status eq 'waiting' + or $self->status eq 'in_transit' ) + { + return 1; + } + + return 0; } =head3 is_waiting @@ -220,8 +222,7 @@ Returns true if hold is a waiting hold sub is_waiting { my ($self) = @_; - my $found = $self->found; - return $found && $found eq 'W'; + return $self->status eq 'waiting'; } =head3 is_in_transit @@ -233,8 +234,7 @@ Returns true if hold is a in_transit hold sub is_in_transit { my ($self) = @_; - return 0 unless $self->found(); - return $self->found() eq 'T'; + return $self->status eq 'in_transit'; } =head3 is_cancelable_from_opac @@ -250,7 +250,7 @@ This is used from the OPAC. sub is_cancelable_from_opac { my ($self) = @_; - return 1 unless $self->is_found(); + return 1 unless $self->is_found() or $self->completed; return 0; # if ->is_in_transit or if ->is_waiting } @@ -265,7 +265,7 @@ the hold item's holding branch sub is_at_destination { my ($self) = @_; - return $self->is_waiting() && ( $self->branchcode() eq $self->item()->holdingbranch() ); + return $self->is_waiting() && ( $self->pickup_library_id() eq $self->item()->holdingbranch() ); } =head3 biblio @@ -277,7 +277,7 @@ Returns the related Koha::Biblio object for this hold sub biblio { my ($self) = @_; - $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); + $self->{_biblio} ||= Koha::Biblios->find( $self->biblio_id ); return $self->{_biblio}; } @@ -291,7 +291,7 @@ Returns the related Koha::Item object for this Hold sub item { my ($self) = @_; - $self->{_item} ||= Koha::Items->find( $self->itemnumber() ); + $self->{_item} ||= Koha::Items->find( $self->item_id ); return $self->{_item}; } @@ -305,7 +305,7 @@ Returns the related Koha::Library object for this Hold sub branch { my ($self) = @_; - $self->{_branch} ||= Koha::Libraries->find( $self->branchcode() ); + $self->{_branch} ||= Koha::Libraries->find( $self->pickup_library_id ); return $self->{_branch}; } @@ -320,7 +320,7 @@ Returns the related Koha::Patron object for this Hold sub borrower { my ($self) = @_; - $self->{_borrower} ||= Koha::Patrons->find( $self->borrowernumber() ); + $self->{_borrower} ||= Koha::Patrons->find( $self->patron_id ); return $self->{_borrower}; } @@ -334,7 +334,7 @@ my $bool = $hold->is_suspended(); sub is_suspended { my ( $self ) = @_; - return $self->suspend(); + return $self->suspended(); } @@ -354,19 +354,23 @@ sub cancel { my ( $self, $params ) = @_; $self->_result->result_source->schema->txn_do( sub { - $self->cancellationdate( dt_from_string->strftime( '%Y-%m-%d %H:%M:%S' ) ); - $self->priority(0); - $self->_move_to_old; - $self->SUPER::delete(); # Do not add a DELETE log + $self->set( + { + completion_date => dt_from_string->strftime( '%Y-%m-%d %H:%M:%S' ), + priority => 0, + completed => 1, + status => 'cancelled' + } + )->store; # now fix the priority on the others.... - C4::Reserves::_FixPriority({ biblionumber => $self->biblionumber }); + C4::Reserves::_FixPriority({ biblionumber => $self->biblio_id }); # and, if desired, charge a cancel fee my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); if ( $charge && $params->{'charge_cancel_fee'} ) { my $account = - Koha::Account->new( { patron_id => $self->borrowernumber } ); + Koha::Account->new( { patron_id => $self->patron_id } ); $account->add_debit( { amount => $charge, @@ -374,32 +378,18 @@ sub cancel { interface => C4::Context->interface, library_id => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef, type => 'RESERVE_EXPIRED', - item_id => $self->itemnumber + item_id => $self->item_id } ); } - C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, Dumper($self->unblessed) ) + C4::Log::logaction( 'HOLDS', 'CANCEL', $self->id, Dumper($self->unblessed) ) if C4::Context->preference('HoldsLog'); } ); return $self; } -=head3 _move_to_old - -my $is_moved = $hold->_move_to_old; - -Move a hold to the old_reserve table following the same pattern as Koha::Patron->move_to_deleted - -=cut - -sub _move_to_old { - my ($self) = @_; - my $hold_infos = $self->unblessed; - return Koha::Old::Hold->new( $hold_infos )->store; -} - =head3 to_api_mapping This method returns the mapping for representing a Koha::Hold object @@ -409,24 +399,7 @@ on the API. sub to_api_mapping { return { - reserve_id => 'hold_id', - borrowernumber => 'patron_id', - reservedate => 'hold_date', - biblionumber => 'biblio_id', - branchcode => 'pickup_library_id', - notificationdate => undef, - reminderdate => undef, - cancellationdate => 'cancelation_date', - reservenotes => 'notes', - found => 'status', - itemnumber => 'item_id', - waitingdate => 'waiting_date', - expirationdate => 'expiration_date', - lowestPriority => 'lowest_priority', - suspend => 'suspended', - suspend_until => 'suspended_until', - itemtype => 'item_type', - item_level_hold => 'item_level', + id => 'hold_id', }; } @@ -437,7 +410,7 @@ sub to_api_mapping { =cut sub _type { - return 'Reserve'; + return 'Hold'; } =head1 AUTHORS diff --git a/Koha/Holds.pm b/Koha/Holds.pm index 604953d4c9..af1ce7af5f 100644 --- a/Koha/Holds.pm +++ b/Koha/Holds.pm @@ -33,7 +33,7 @@ Koha::Holds - Koha Hold object set class =head1 API -=head2 Class Methods +=head2 Class methods =cut @@ -46,7 +46,12 @@ returns a set of holds that are waiting from an existing set sub waiting { my ( $self ) = @_; - return $self->search( { found => 'W' } ); + return $self->search( + { + status => 'waiting', + completed => 0 + } + ); } =head3 unfilled @@ -58,7 +63,12 @@ returns a set of holds that are unfilled from an existing set sub unfilled { my ( $self ) = @_; - return $self->search( { found => undef } ); + return $self->search( + { + status => 'placed', + completed => 0 + } + ); } =head3 forced_hold_level @@ -83,10 +93,10 @@ that would prevent one or the other. sub forced_hold_level { my ($self) = @_; - my $item_level_count = $self->search( { itemnumber => { '!=' => undef } } )->count(); + my $item_level_count = $self->search( { item_id => { '!=' => undef } } )->count(); return 'item' if $item_level_count > 0; - my $record_level_count = $self->search( { itemnumber => undef } )->count(); + my $record_level_count = $self->search( { item_id => undef } )->count(); return 'record' if $record_level_count > 0; return; @@ -97,7 +107,7 @@ sub forced_hold_level { =cut sub _type { - return 'Reserve'; + return 'Hold'; } =head3 object_class -- 2.25.0