Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 30947: Simplify CanBookBeIssued date handling

Browse source 
1) This removes support for passing string dates to CanBookBeIssued. The
function didn't publicly even document support for string dates, only
DateTime objects.

2) We get a $duedate always at least from CalcDateDue so having

 $issuingimpossible{INVALID_DATE} = output_pref($duedate);

was unneccesary and thus removed.

3) The check "duedate cannot be before now" was needlessly complex: if
the due date really cannot be before now we should check seconds too
and warn the librarian! Thus the truncation to minutes can be dropped
safely.

To test:
 1) prove t/db_dependent/Circulation.t
 2) prove t/db_dependent/Illrequests.t
 3) Enable OnSiteCheckouts and disable SpecifyDueDate syspref. Create
 on-site checkout for any patron and verify the due date is your
 current date at 23:59, you can check the exact minute with sql:
  > select * from issues

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Joonas Kylmälä 2022-06-11 10:50:06 +00:00 committed by Tomas Cohen Arazi
parent 3025373b69
commit 9b4b43656e
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
4 changed files with 12 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
C4/Circulation.pm
View file

@ -794,20 +794,11 @@ sub CanBookBeIssued {
my $patron_unblessed = $patron->unblessed;
my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) );
#
# DUE DATE is OK ? -- should already have checked.
#
if ($duedate && ref $duedate ne 'DateTime') {
$duedate = dt_from_string($duedate);
}
my $now = dt_from_string();
unless ( $duedate ) {
my $issuedate = $now->clone();
$duedate = CalcDateDue( $issuedate, $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
# Offline circ calls AddIssue directly, doesn't run through here
# So issuingimpossible should be ok.
$duedate ||= CalcDateDue( $now->clone(), $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
if (DateTime->compare($duedate,$now) == -1 ) { # duedate cannot be before now
$needsconfirmation{INVALID_DATE} = output_pref($duedate);
}
my $fees = Koha::Charges::Fees->new(
@ -819,16 +810,6 @@ sub CanBookBeIssued {
}
);
if ($duedate) {
my $today = $now->clone();
$today->truncate( to => 'minute');
if (DateTime->compare($duedate,$today) == -1 ) { # duedate cannot be before now
$needsconfirmation{INVALID_DATE} = output_pref($duedate);
}
} else {
$issuingimpossible{INVALID_DATE} = output_pref($duedate);
}
#
# BORROWER STATUS
#

2
Koha/Illrequest.pm
View file

@ -1222,7 +1222,7 @@ sub check_out {
scalar $target_item->barcode
);
if ($params->{duedate} && length $params->{duedate} > 0) {
push @issue_args, $params->{duedate};
push @issue_args, dt_from_string($params->{duedate});
}
# Check if we can check out
my ( $error, $confirm, $alerts, $messages ) =

5
circ/circulation.pl
View file

@ -196,8 +196,9 @@ my ($datedue,$invalidduedate);
my $duedatespec_allow = C4::Context->preference('SpecifyDueDate');
if( $onsite_checkout && !$duedatespec_allow ) {
$datedue = output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' });
$datedue .= ' 23:59:00';
$datedue = dt_from_string()->truncate(to => 'day');
$datedue->set_hour(23);
$datedue->set_minute(59);
} elsif( $duedatespec_allow ) {
if ( $duedatespec ) {
$datedue = eval { dt_from_string( $duedatespec ) };

8
t/db_dependent/Circulation.t
View file

@ -3837,9 +3837,9 @@ subtest 'CanBookBeIssued | is_overdue' => sub {
}
);
my $now = dt_from_string;
my $five_days_go = output_pref({ dt => $now->clone->add( days => 5 ), dateonly => 1});
my $ten_days_go = output_pref({ dt => $now->clone->add( days => 10), dateonly => 1 });
my $now = dt_from_string()->truncate( to => 'day' );
my $five_days_go = $now->clone->add( days => 5 );
my $ten_days_go = $now->clone->add( days => 10);
my $library = $builder->build( { source => 'Branch' } );
my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $patron_category->{categorycode} } } );
@ -3851,7 +3851,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub {
my $issue = AddIssue( $patron->unblessed, $item->barcode, $five_days_go ); # date due was 10d ago
my $actualissue = Koha::Checkouts->find( { itemnumber => $item->itemnumber } );
is( output_pref({ str => $actualissue->date_due, dateonly => 1}), $five_days_go, "First issue works");
is( output_pref({ str => $actualissue->date_due, dateonly => 1}), output_pref({ str => $five_days_go, dateonly => 1}), "First issue works");
my ($issuingimpossible, $needsconfirmation) = CanBookBeIssued($patron,$item->barcode,$ten_days_go, undef, undef, undef);
is( $needsconfirmation->{RENEW_ISSUE}, 1, "This is a renewal");
is( $needsconfirmation->{TOO_MANY}, undef, "Not too many, is a renewal");