Bugzilla – Attachment 103990 Details for
Bug 25260
Merge 'reserves' and 'old_reserves' into a new 'holds' table
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25260: Adapt Koha::Hold(s)
Bug-25260-Adapt-KohaHolds.patch (text/plain), 12.46 KB, created by
Agustín Moyano
on 2020-04-29 21:21:38 UTC
(
hide
)
Description:
Bug 25260: Adapt Koha::Hold(s)
Filename:
MIME Type:
Creator:
Agustín Moyano
Created:
2020-04-29 21:21:38 UTC
Size:
12.46 KB
patch
obsolete
>From 61a6e2ed15e006cb76ddc8d1a4a0b67f554d73d8 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Fri, 17 Apr 2020 18:44:35 -0300 >Subject: [PATCH] Bug 25260: Adapt Koha::Hold(s) > >Sponsored-by: ByWater Solutions >Sponsored-by: Theke Solutions >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25260
:
103984
|
103985
|
103986
|
103987
|
103988
|
103989
|
103990
|
103991
|
103992
|
103993
|
104053
|
104054
|
104055
|
104056
|
104057
|
117745
|
117746
|
117747
|
117748
|
117749
|
117750
|
117751
|
122761
|
122762
|
122763
|
122764
|
122765
|
122766
|
122767
|
122787
|
122788
|
122789
|
122790
|
122791
|
122792
|
122793
|
122976
|
122977
|
122978
|
122979
|
122980
|
122981
|
122982
|
122983
|
123071
|
123072
|
123073
|
123074
|
123075
|
123076
|
123077
|
123078
|
123079
|
123080
|
123257
|
123258
|
123259
|
123260
|
123261
|
123396