Bugzilla – Attachment 182941 Details for
Bug 40058
Move RevertWaitingStatus to Koha::Hold->revert_waiting()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40058: Add Koha::Hold->revert_waiting()
Bug-40058-Add-KohaHold-revertwaiting.patch (text/plain), 11.54 KB, created by
David Nind
on 2025-06-04 11:22:36 UTC
(
hide
)
Description:
Bug 40058: Add Koha::Hold->revert_waiting()
Filename:
MIME Type:
Creator:
David Nind
Created:
2025-06-04 11:22:36 UTC
Size:
11.54 KB
patch
obsolete
>From 0401803e6f3875ff356f36def2c98b04665abfae Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Tue, 3 Jun 2025 10:30:12 -0300 >Subject: [PATCH] Bug 40058: Add Koha::Hold->revert_waiting() > >This patch adds a new method to the `Koha::Hold` class. The method >follows the original implementation in C4::Reserves. > >Tests are added to cover all the code paths. > >To test: >1. Apply this patch >2. Run: > $ ktd --shell > k$ prove t/db_dependent/Koha/Hold.t >=> SUCCESS: Tests pass :-D >3. Sign off :-D > >Signed-off-by: David Nind <david@davidnind.com> >--- > Koha/Hold.pm | 53 +++++++++ > t/db_dependent/Koha/Hold.t | 237 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 289 insertions(+), 1 deletion(-) > >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 8f0cd75ac2..d6a784ddfa 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -889,6 +889,59 @@ sub fill { > return $self; > } > >+=head3 revert_waiting >+ >+ $hold->revert_waiting(); >+ >+Reverts a 'waiting' hold back to a regular hold with a priority of 1. >+ >+=cut >+ >+sub revert_waiting { >+ my ( $self, $params ) = @_; >+ >+ Koha::Exceptions::InvalidStatus->throw( invalid_status => 'hold_not_waiting' ) >+ unless $self->is_waiting; >+ >+ $self->_result->result_source->schema->txn_do( >+ sub { >+ my $original = C4::Context->preference('HoldsLog') ? $self->unblessed : undef; >+ >+ # Increment the priority of all other non-waiting >+ # reserves for this bib record >+ my $holds = Koha::Holds->search( >+ { >+ biblionumber => $self->biblionumber, >+ priority => { '>' => 0 } >+ } >+ )->update( >+ { priority => \'priority + 1' }, >+ { no_triggers => 1 } >+ ); >+ >+ ## Fix up the currently waiting reserve >+ $self->set( >+ { >+ priority => 1, >+ found => undef, >+ waitingdate => undef, >+ expirationdate => $self->patron_expiration_date, >+ itemnumber => $self->item_level_hold ? $self->itemnumber : undef, >+ } >+ )->store( { hold_reverted => 1 } ); >+ >+ logaction( 'HOLDS', 'MODIFY', $self->id, $self, undef, $original ) >+ if C4::Context->preference('HoldsLog'); >+ >+ C4::Reserves::_FixPriority( { biblionumber => $self->biblionumber } ); >+ >+ Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [ $self->biblionumber ] } ) >+ if C4::Context->preference('RealTimeHoldsQueue'); >+ } >+ ); >+ return $self; >+} >+ > =head3 sub change_type > > $hold->change_type # to record level >diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t >index 4b84be5744..5a8020fae6 100755 >--- a/t/db_dependent/Koha/Hold.t >+++ b/t/db_dependent/Koha/Hold.t >@@ -20,7 +20,7 @@ > use Modern::Perl; > > use Test::NoWarnings; >-use Test::More tests => 15; >+use Test::More tests => 16; > > use Test::Exception; > use Test::MockModule; >@@ -29,6 +29,8 @@ use t::lib::Mocks; > use t::lib::TestBuilder; > use t::lib::Mocks; > >+use C4::Reserves qw(AddReserve); >+ > use Koha::ActionLogs; > use Koha::DateUtils qw(dt_from_string); > use Koha::Holds; >@@ -1161,3 +1163,236 @@ subtest 'change_type() tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'revert_waiting() tests' => sub { >+ >+ plan tests => 3; >+ >+ subtest 'item-level holds tests' => sub { >+ >+ plan tests => 13; >+ >+ $schema->storage->txn_begin; >+ >+ my $item = $builder->build_sample_item; >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { branchcode => $item->homebranch } >+ } >+ ); >+ >+ # Create item-level hold >+ my $hold = Koha::Holds->find( >+ AddReserve( >+ { >+ branchcode => $item->homebranch, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $item->biblionumber, >+ priority => 1, >+ itemnumber => $item->itemnumber, >+ } >+ ) >+ ); >+ >+ is( $hold->item_level_hold, 1, 'item_level_hold should be set when AddReserve is called with a specific item' ); >+ >+ my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); >+ $mock->mock( >+ 'enqueue', >+ sub { >+ my ( $self, $args ) = @_; >+ is_deeply( >+ $args->{biblio_ids}, >+ [ $hold->biblionumber ], >+ "AlterPriority triggers a holds queue update for the related biblio" >+ ); >+ } >+ ); >+ >+ t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 1 ); >+ t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); >+ >+ # Mark it waiting >+ $hold->set_waiting(); >+ >+ isnt( $hold->waitingdate, undef, "'waitingdate' set" ); >+ is( $hold->priority, 0, "'priority' set to 0" ); >+ ok( $hold->is_waiting, 'Hold set to waiting' ); >+ >+ # Revert the waiting status >+ $hold->revert_waiting(); >+ >+ is( $hold->waitingdate, undef, "'waitingdate' reset" ); >+ ok( !$hold->is_waiting, 'Hold no longer set to waiting' ); >+ is( $hold->priority, 1, "'priority' set to 1" ); >+ >+ is( >+ $hold->itemnumber, $item->itemnumber, >+ 'Itemnumber should not be removed when the waiting status is revert' >+ ); >+ >+ my $log = >+ Koha::ActionLogs->search( { module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id } )->next; >+ my $expected = sprintf q{'timestamp' => '%s'}, $hold->timestamp; >+ like( $log->info, qr{$expected}, 'Timestamp logged is the current one' ); >+ my $log_count = >+ Koha::ActionLogs->search( { module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id } )->count; >+ >+ t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 0 ); >+ t::lib::Mocks::mock_preference( 'HoldsLog', 0 ); >+ >+ $hold->set_waiting(); >+ >+ # Revert the waiting status, RealTimeHoldsQueue => shouldn't add a test >+ $hold->revert_waiting(); >+ >+ my $log_count_after = >+ Koha::ActionLogs->search( { module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id } )->count; >+ is( $log_count, $log_count_after, "No logging is added for ->revert_waiting() when HoldsLog is disabled" ); >+ >+ # Set as 'processing' to test the exception behavior >+ $hold->found('P'); >+ throws_ok { $hold->revert_waiting() } >+ 'Koha::Exceptions::InvalidStatus', >+ "Hold is not in 'waiting' status, exception thrown"; >+ >+ is( $@->invalid_status, 'hold_not_waiting', "'invalid_status' set the right value" ); >+ >+ $schema->storage->txn_rollback; >+ }; >+ >+ subtest 'biblio-level hold tests' => sub { >+ >+ plan tests => 8; >+ >+ $schema->storage->txn_begin; >+ >+ my $item = $builder->build_sample_item; >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { branchcode => $item->homebranch } >+ } >+ ); >+ >+ # Create biblio-level hold >+ my $hold = Koha::Holds->find( >+ AddReserve( >+ { >+ branchcode => $item->homebranch, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $item->biblionumber, >+ priority => 1, >+ } >+ ) >+ ); >+ >+ is( >+ $hold->item_level_hold, 0, >+ 'item_level_hold should not be set when AddReserve is called without a specific item' >+ ); >+ >+ # Mark it waiting >+ $hold->set( { itemnumber => $item->itemnumber } )->set_waiting(); >+ $hold->set_waiting(); >+ >+ isnt( $hold->waitingdate, undef, "'waitingdate' set" ); >+ is( $hold->priority, 0, "'priority' set to 0" ); >+ ok( $hold->is_waiting, 'Hold set to waiting' ); >+ >+ # Revert the waiting status >+ $hold->revert_waiting(); >+ >+ is( $hold->waitingdate, undef, "'waitingdate' reset" ); >+ ok( !$hold->is_waiting, 'Hold no longer set to waiting' ); >+ is( $hold->priority, 1, "'priority' set to 1" ); >+ is( $hold->itemnumber, undef, "'itemnumber' unset" ); >+ >+ $schema->storage->txn_rollback; >+ }; >+ >+ subtest 'priority shift tests' => sub { >+ >+ plan tests => 4; >+ >+ $schema->storage->txn_begin; >+ >+ # Create the items and patrons we need >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $itype = $builder->build_object( { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >+ my $item = $builder->build_sample_item( >+ { >+ itype => $itype->itemtype, >+ library => $library->branchcode >+ } >+ ); >+ my $patron_1 = $builder->build_object( { class => "Koha::Patrons" } ); >+ my $patron_2 = $builder->build_object( { class => "Koha::Patrons" } ); >+ my $patron_3 = $builder->build_object( { class => "Koha::Patrons" } ); >+ my $patron_4 = $builder->build_object( { class => "Koha::Patrons" } ); >+ >+ # Place a hold on the title for both patrons >+ my $hold = Koha::Holds->find( >+ C4::Reserves::AddReserve( >+ { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron_1->borrowernumber, >+ biblionumber => $item->biblionumber, >+ priority => 1, >+ itemnumber => $item->itemnumber, >+ } >+ ) >+ ); >+ >+ C4::Reserves::AddReserve( >+ { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron_2->borrowernumber, >+ biblionumber => $item->biblionumber, >+ priority => 1, >+ itemnumber => $item->itemnumber, >+ } >+ ); >+ >+ C4::Reserves::AddReserve( >+ { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron_3->borrowernumber, >+ biblionumber => $item->biblionumber, >+ priority => 1, >+ itemnumber => $item->itemnumber, >+ } >+ ); >+ >+ C4::Reserves::AddReserve( >+ { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron_4->borrowernumber, >+ biblionumber => $item->biblionumber, >+ priority => 1, >+ itemnumber => $item->itemnumber, >+ } >+ ); >+ >+ is( $item->biblio->holds->count, 4, '4 holds on the biblio' ); >+ >+ $hold->set_waiting()->discard_changes(); >+ is( $hold->priority, 0, "'priority' set to 0" ); >+ >+ $hold->revert_waiting()->discard_changes(); >+ is( $hold->priority, 1, "'priority' set to 1" ); >+ >+ my $holds = $item->biblio->holds; >+ is_deeply( >+ [ >+ $holds->next->priority, $holds->next->priority, >+ $holds->next->priority, $holds->next->priority, >+ ], >+ [ 1, 2, 3, 4 ], >+ 'priorities have been reordered' >+ ); >+ >+ $schema->storage->txn_rollback; >+ }; >+}; >-- >2.39.5
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 40058
:
182924
|
182925
|
182926
| 182941 |
182942
|
182943