From da33f365debdd09c8f9b8af2ca904d2f72d3c062 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 14 Feb 2018 15:15:39 -0300 Subject: [PATCH] Bug 20145: Do not insert 0000-00-00 in patron tests (and more) We should call Koha::Patron->is_expired in CanBookBeIssued instead of doing the same calculation. Tests have been adapted to pass with new SQL modes. We should not need to update the values in DB, we already have Bug 14717: Prevent 0000-00-00 dates in patron data (3.21.00.023) Test plan: prove t/db_dependent/Circulation/dateexpiry.t prove t/db_dependent/Koha/Patrons.t must return green Signed-off-by: Roch D'Amour Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart Conflicts: C4/Circulation.pm t/db_dependent/Koha/Patrons.t Signed-off-by: Nick Clemens (cherry picked from commit 9f075a4fdb2649493820ffed20a352add02c05ea) Signed-off-by: Fridolin Somers --- C4/Circulation.pm | 14 ++++---------- C4/Utils/DataTables/Members.pm | 3 ++- Koha/Patron.pm | 4 ++-- t/db_dependent/Circulation/dateexpiry.t | 4 ++-- t/db_dependent/Koha/Patrons.t | 8 ++------ 5 files changed, 12 insertions(+), 21 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 5cb6c16d7b..044aded02d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -720,6 +720,7 @@ sub CanBookBeIssued { # # BORROWER STATUS # + my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); if ( $borrower->{'category_type'} eq 'X' && ( $item->{barcode} )) { # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1 . &UpdateStats({ @@ -746,16 +747,9 @@ sub CanBookBeIssued { $issuingimpossible{DEBARRED} = 1; } } - if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') { + + if ( $patron->is_expired ) { $issuingimpossible{EXPIRED} = 1; - } else { - my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'sql', 'floating' ); - $expiry_dt->truncate( to => 'day'); - my $today = $now->clone()->truncate(to => 'day'); - $today->set_time_zone( 'floating' ); - if ( DateTime->compare($today, $expiry_dt) == 1 ) { - $issuingimpossible{EXPIRED} = 1; - } } # @@ -814,7 +808,7 @@ sub CanBookBeIssued { $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges ); } - my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); + $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); if ( my $debarred_date = $patron->is_debarred ) { # patron has accrued fine days or has a restriction. $count is a date if ($debarred_date eq '9999-12-31') { diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm index b2e4963a73..f4d043b23f 100644 --- a/C4/Utils/DataTables/Members.pm +++ b/C4/Utils/DataTables/Members.pm @@ -176,7 +176,8 @@ sub search { # FIXME Should be formatted from the template $patron->{fines} = sprintf("%.2f", $balance); - if($patron->{dateexpiry} and $patron->{dateexpiry} ne '0000-00-00') { + if( $patron->{dateexpiry} ) { + # FIXME We should not format the date here, do it in template-side instead $patron->{dateexpiry} = output_pref( { dt => dt_from_string( $patron->{dateexpiry}, 'iso'), dateonly => 1} ); } else { $patron->{dateexpiry} = ''; diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 555121ff65..314b90dd58 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -296,7 +296,7 @@ Returns 1 if the patron is expired or 0; sub is_expired { my ($self) = @_; return 0 unless $self->dateexpiry; - return 0 if $self->dateexpiry eq '0000-00-00'; + return 0 if $self->dateexpiry =~ '^9999'; return 1 if dt_from_string( $self->dateexpiry ) < dt_from_string->truncate( to => 'day' ); return 0; } @@ -316,7 +316,7 @@ sub is_going_to_expire { return 0 unless $delay; return 0 unless $self->dateexpiry; - return 0 if $self->dateexpiry eq '0000-00-00'; + return 0 if $self->dateexpiry =~ '^9999'; return 1 if dt_from_string( $self->dateexpiry )->subtract( days => $delay ) < dt_from_string->truncate( to => 'day' ); return 0; } diff --git a/t/db_dependent/Circulation/dateexpiry.t b/t/db_dependent/Circulation/dateexpiry.t index dbb7e0bda6..9ccb593c10 100644 --- a/t/db_dependent/Circulation/dateexpiry.t +++ b/t/db_dependent/Circulation/dateexpiry.t @@ -60,12 +60,12 @@ sub can_book_be_issued { $item = $builder->build( { source => 'Item' } ); $patron = $builder->build( { source => 'Borrower', - value => { dateexpiry => '0000-00-00' } + value => { dateexpiry => undef } } ); $patron->{flags} = C4::Members::patronflags( $patron ); ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( $patron, $item->{barcode} ); - is( $issuingimpossible->{EXPIRED}, 1, 'The patron should be considered as expired if dateexpiry is 0000-00-00' ); + is( not( exists $issuingimpossible->{EXPIRED} ), 1, 'The patron should not be considered as expired if dateexpiry is not set' ); my $tomorrow = dt_from_string->add_duration( DateTime::Duration->new( days => 1 ) ); $item = $builder->build( { source => 'Item' } ); diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 3d8be3ebc5..b7fb6f9933 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -185,13 +185,11 @@ subtest 'update_password' => sub { }; subtest 'is_expired' => sub { - plan tests => 5; + plan tests => 4; my $patron = $builder->build({ source => 'Borrower' }); $patron = Koha::Patrons->find( $patron->{borrowernumber} ); $patron->dateexpiry( undef )->store->discard_changes; is( $patron->is_expired, 0, 'Patron should not be considered expired if dateexpiry is not set'); - $patron->dateexpiry( '0000-00-00' )->store->discard_changes; - is( $patron->is_expired, 0, 'Patron should not be considered expired if dateexpiry is not 0000-00-00'); $patron->dateexpiry( dt_from_string )->store->discard_changes; is( $patron->is_expired, 0, 'Patron should not be considered expired if dateexpiry is today'); $patron->dateexpiry( dt_from_string->add( days => 1 ) )->store->discard_changes; @@ -203,13 +201,11 @@ subtest 'is_expired' => sub { }; subtest 'is_going_to_expire' => sub { - plan tests => 9; + plan tests => 8; my $patron = $builder->build({ source => 'Borrower' }); $patron = Koha::Patrons->find( $patron->{borrowernumber} ); $patron->dateexpiry( undef )->store->discard_changes; is( $patron->is_going_to_expire, 0, 'Patron should not be considered going to expire if dateexpiry is not set'); - $patron->dateexpiry( '0000-00-00' )->store->discard_changes; - is( $patron->is_going_to_expire, 0, 'Patron should not be considered going to expire if dateexpiry is not 0000-00-00'); t::lib::Mocks::mock_preference('NotifyBorrowerDeparture', 0); $patron->dateexpiry( dt_from_string )->store->discard_changes; -- 2.39.5