Attachment 118678 Details for Bug 26498 – Bug 26498: (QA follow-up) Add handling for update

[patch] Bug 26498: (QA follow-up) Add handling for update

Bug-26498-QA-follow-up-Add-handling-for-update.patch (text/plain), 6.59 KB, created by Martin Renvoize (ashimema) on 2021-03-23 15:52:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-03-23 15:52:38 UTC

Size: 6.59 KB

patch

obsolete

>From 56c7b2d6cdf487706c53721990aa92cc9c3a0698 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>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.
>---
> 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.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 26498: 114057 | 114524 | 114663 | 117127 | 117472 | 117542 | 117543 | 118676 | 118677 | 118678 | 119278