From 2eb0ac6a3e08b86eba140f5135ebc717eccc70ae Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 23 Mar 2021 15:50:54 +0000 Subject: [PATCH] Bug 26498: (QA follow-up) Add handling for update This follow-up adds handling for updates as well as the create action as highlighted by Jonathans comments. We also adds tests for these cases. Signed-off-by: Jonathan Druart --- Koha/Hold.pm | 40 +++++++++++++++--- t/db_dependent/Hold.t | 98 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 114 insertions(+), 24 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index b8e9d2e09d..46aee1aace 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -572,18 +572,48 @@ expirationdate for holds. sub store { my ($self) = @_; - if ( C4::Context->preference('DefaultHoldExpirationdate') - and ( not defined $self->expirationdate or $self->expirationdate eq '' ) ){ + if ( !$self->in_storage ) { + if ( + C4::Context->preference('DefaultHoldExpirationdate') + and ( not defined $self->expirationdate + or $self->expirationdate eq '' ) + ) + { + $self->_set_default_expirationdate; + } + } + else { - my $period = C4::Context->preference('DefaultHoldExpirationdatePeriod'); - my $timeunit = C4::Context->preference('DefaultHoldExpirationdateUnitOfTime'); + my %updated_columns = $self->_result->get_dirty_columns; + return $self->SUPER::store unless %updated_columns; - $self->expirationdate( dt_from_string( $self->reservedate )->add( $timeunit => $period ) ); + if ( exists $updated_columns{reservedate} ) { + if ( + C4::Context->preference('DefaultHoldExpirationdate') + and ( not exists $updated_columns{expirationdate} + or exists $updated_columns{expirationdate} + and $updated_columns{expirationdate} eq '' ) + ) + { + $self->_set_default_expirationdate; + } + } } $self = $self->SUPER::store; } +sub _set_default_expirationdate { + my $self = shift; + + my $period = C4::Context->preference('DefaultHoldExpirationdatePeriod'); + my $timeunit = + C4::Context->preference('DefaultHoldExpirationdateUnitOfTime'); + + $self->expirationdate( + dt_from_string( $self->reservedate )->add( $timeunit => $period ) ); +} + =head3 _move_to_old my $is_moved = $hold->_move_to_old; diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index c86807e3a9..365557e2b8 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -142,28 +142,88 @@ ok( !$hold->is_at_destination(), "Waiting hold where hold branchcode is not the $item->holdingbranch( $branches[1]->{branchcode} ); ok( $hold->is_at_destination(), "Waiting hold where hold branchcode is the same as the item's holdingbranch is at destination" ); -# Test setting expiration date -t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdate', 1 ); -t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdatePeriod', 2 ); -t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdateUnitOfTime', 'years' ); +$schema->storage->txn_rollback(); -my $hold_2 = Koha::Hold->new( - { - biblionumber => $biblionumber, - itemnumber => $item->id(), - reservedate => '2020-11-30', - waitingdate => '2020-11-30', - borrowernumber => $borrower->{borrowernumber}, - branchcode => $branches[1]->{branchcode}, - suspend => 0, - } -); -$hold_2->store(); +subtest "store() tests" => sub { -my $expected_date = dt_from_string( $hold_2->reservedate )->add( years => 2); -is($hold_2->expirationdate->ymd, $expected_date->ymd,'Expiration date is correctly set.'); + plan tests => 5; -$schema->storage->txn_rollback(); + $schema->storage->txn_begin(); + + my $item = $builder->build_sample_item; + my $biblio = $item->biblio; + my $borrower = $builder->build_object( { class => 'Koha::Patrons' } ); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + + # Test setting expiration date + t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdate', 1 ); + t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdatePeriod', 2 ); + t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdateUnitOfTime', + 'years' ); + + my $hold = Koha::Hold->new( + { + biblionumber => $biblio->biblionumber, + itemnumber => $item->id, + reservedate => '2020-11-30', + waitingdate => '2020-11-30', + borrowernumber => $borrower->borrowernumber, + branchcode => $library->branchcode, + suspend => 0, + } + )->store; + $hold->discard_changes; + + my $expected_date = + dt_from_string( $hold->reservedate )->add( years => 2 ); + is( $hold->expirationdate, + $expected_date->ymd, 'Default expiration date for new hold is correctly set.' ); + + my $passed_date = + dt_from_string( $hold->reservedate )->add( months => 2 ); + $hold = Koha::Hold->new( + { + biblionumber => $biblio->biblionumber, + itemnumber => $item->id, + reservedate => '2020-11-30', + expirationdate => $passed_date->ymd, + waitingdate => '2020-11-30', + borrowernumber => $borrower->borrowernumber, + branchcode => $library->branchcode, + suspend => 0, + } + )->store; + $hold->discard_changes; + + is( $hold->expirationdate, + $passed_date->ymd, 'Passed expiration date for new hold is correctly set.' ); + + $passed_date->add( months => 1 ); + $hold->expirationdate($passed_date->ymd)->store(); + $hold->discard_changes; + + is( $hold->expirationdate, + $passed_date->ymd, 'Passed expiration date when updating hold is correctly set.' ); + + $hold->reservedate($passed_date->ymd)->store(); + $hold->discard_changes; + + is( $hold->expirationdate, + $passed_date->add( years => 2 )->ymd, 'Default expiration date correctly set on reservedate update.' ); + + $hold->set( + { + reservedate => $passed_date->ymd, + expirationdate => $passed_date->add( weeks => 2 )->ymd + } + )->store(); + $hold->discard_changes; + + is( $hold->expirationdate, + $passed_date->ymd, 'Passed expiration date when updating hold correctly set (Passing both reservedate and expirationdate.' ); + + $schema->storage->txn_rollback(); +}; subtest "delete() tests" => sub { -- 2.39.5