From 79d5f5588da99ca4517ef8fd3c49d4071522c163 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Wed, 12 Mar 2025 14:37:11 +0200 Subject: [PATCH] Bug 38356: Check patrons current checkouts separately If syspref CheckPrevCheckout is enabled, Koha checks if patron has previously checked out item within timeperiod set in syspref CheckPrevCheckoutDelay. This however doesn't take into account cases where item is currently checked out for patron and they are trying to check out another item from same record (can be done if syspref AllowMultipleIssuesOnABiblio is enabled). We should always check if item is currently checked out for the patron if CheckPrevCheckout is enabled. To test: 1. Enable syspref CheckPrevCheckout and set CheckPrevCheckoutDelay as e.g. 10. Also enable syspref AllowMultipleIssuesOnABiblio. 2. Find a record that has been checked out for patron over 10 days ago. 3. Attempt to check out item from same record for same patron. => Note that you have to confirm if you want to check out item for the patron. 4. Find record that is currently checked out for a patron. 5. Attempt to check out item from same record for same patron. => Note that item is checked out for patron without confirm message. 6. Apply this patch and restart services if needed. 7. Repeat steps 2. and 3. => Confirm that you still have to confirm if you want to check out item for the patron. 8. Repeat steps 4. and 5. => New confirm message "Patron has this title currently checked out:..." should be displayed and you have to confirm if you want to check out item for the patron. Also prove t/db_dependent/Patron/Borrower_PrevCheckout.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Felicie --- C4/Circulation.pm | 9 +- Koha/Patron.pm | 10 +-- .../prog/en/modules/circ/circulation.tt | 4 + t/db_dependent/Patron/Borrower_PrevCheckout.t | 88 +++++++++++++++++-- 4 files changed, 99 insertions(+), 12 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 31794b7c02..fbb6f08ba2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1060,8 +1060,13 @@ sub CanBookBeIssued { # $patron = Koha::Patrons->find( $patron->borrowernumber ) ; # FIXME Refetch just in case, to avoid regressions. But must not be needed - if ( $patron->wants_check_for_previous_checkout && $patron->do_check_for_previous_checkout($item_unblessed) ) { - $needsconfirmation{PREVISSUE} = 1; + my $check_previous_checkout = $patron->do_check_for_previous_checkout($item_unblessed); + if ( $patron->wants_check_for_previous_checkout && $check_previous_checkout ) { + if ( $check_previous_checkout eq "currentlycheckedout" ) { + $needsconfirmation{CURRENTISSUE} = 1; + } else { + $needsconfirmation{PREVISSUE} = 1; + } } # diff --git a/Koha/Patron.pm b/Koha/Patron.pm index fd7cf85fc2..4f1e884ed2 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -858,6 +858,10 @@ sub do_check_for_previous_checkout { itemnumber => \@item_nos, }; + # Check current issues table + my $issues = Koha::Checkouts->search($criteria); + return "currentlycheckedout" if $issues->count; # 0 || N + my $delay = C4::Context->preference('CheckPrevCheckoutDelay') || 0; if ($delay) { my $dtf = Koha::Database->new->schema->storage->datetime_parser; @@ -865,13 +869,9 @@ sub do_check_for_previous_checkout { $criteria->{'returndate'} = { '>' => $dtf->format_datetime($newer_than), }; } - # Check current issues table - my $issues = Koha::Checkouts->search($criteria); - return 1 if $issues->count; # 0 || N - # Check old issues table my $old_issues = Koha::Old::Checkouts->search($criteria); - return $old_issues->count; # 0 || N + return $old_issues->count; # 0 || N } =head3 is_debarred diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index d81f3522f6..50cf698834 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -272,6 +272,10 @@
  • High demand item. Loan period shortened to [% HIGHHOLDS.duration | html %] days (due [% HIGHHOLDS.returndate | $KohaDates %]). Check out anyway?
  • [% END %] + [% IF CURRENTISSUE %] +
  • Patron has this title currently checked out: [% biblio.title | html %] [% IF biblio.author %]by [% biblio.author | html %][% END %]. Check out anyway?
  • + [% END %] + [% IF PREVISSUE %]
  • Patron has previously checked out this title: [% biblio.title | html %] [% IF biblio.author %]by [% biblio.author | html %][% END %]. Check out anyway?
  • [% END %] diff --git a/t/db_dependent/Patron/Borrower_PrevCheckout.t b/t/db_dependent/Patron/Borrower_PrevCheckout.t index cb5480d5e7..7d643d5a08 100755 --- a/t/db_dependent/Patron/Borrower_PrevCheckout.t +++ b/t/db_dependent/Patron/Borrower_PrevCheckout.t @@ -8,7 +8,7 @@ use Koha::DateUtils qw( dt_from_string ); use Koha::Patrons; use Test::NoWarnings; -use Test::More tests => 62; +use Test::More tests => 64; use_ok('Koha::Patron'); @@ -315,13 +315,13 @@ my $cpvPmappings = [ msg => "Same item, same patron [1]", item => $item_1, patron => $patron, - result => 1, + result => "currentlycheckedout", }, { msg => "Diff item, same bib, same patron [1]", item => $item_2, patron => $patron, - result => 1, + result => "currentlycheckedout", }, { msg => "Diff item, diff bib, same patron [0]", @@ -354,6 +354,47 @@ test_it( $cpvPmappings, "PostIssue" ); # Return item_1 from patron: BAIL_OUT("Return Failed") unless AddReturn( $item_1->{barcode}, $patron->{branchcode} ); +#Since currently checked in item now return status "currentlycheckedout" we need use +#same test scenarions for returned item as above but without "currentlycheckedout" +$cpvPmappings = [ + { + msg => "Same item, same patron [1]", + item => $item_1, + patron => $patron, + result => 1, + }, + { + msg => "Diff item, same bib, same patron [1]", + item => $item_2, + patron => $patron, + result => 1, + }, + { + msg => "Diff item, diff bib, same patron [0]", + item => $item_d, + patron => $patron, + result => 0, + }, + { + msg => "Same item, diff patron [0]", + item => $item_1, + patron => $patron_d, + result => 0, + }, + { + msg => "Diff item, same bib, diff patron [0]", + item => $item_2, + patron => $patron_d, + result => 0, + }, + { + msg => "Diff item, diff bib, diff patron [0]", + item => $item_d, + patron => $patron_d, + result => 0, + }, +]; + # Then: test_it( $cpvPmappings, "PostReturn" ); @@ -364,7 +405,18 @@ test_it( $cpvPmappings, "PostReturn" ); # whetherthe different combinational outcomes of the above return values in # CanBookBeIssued result in the approriate $needsconfirmation. -# We want to test: +# We want to test when item is currently issued to the patron: +# - DESCRIPTION [RETURNVALUE (0/1)] +# - patron, !wants_check_for_previous_checkout, !do_check_for_previous_checkout +# [!$issuingimpossible,!$needsconfirmation->{CURRENTISSUE}] +# - patron, wants_check_for_previous_checkout, !do_check_for_previous_checkout +# [!$issuingimpossible,!$needsconfirmation->{CURRENTISSUE}] +# - patron, !wants_check_for_previous_checkout, do_check_for_previous_checkout +# [!$issuingimpossible,!$needsconfirmation->{CURRENTISSUE}] +# - patron, wants_check_for_previous_checkout, do_check_for_previous_checkout +# [!$issuingimpossible,$needsconfirmation->{CURRENTISSUE}] + +# And we also need to test when item has been previously issued to the patron: # - DESCRIPTION [RETURNVALUE (0/1)] # - patron, !wants_check_for_previous_checkout, !do_check_for_previous_checkout # [!$issuingimpossible,!$needsconfirmation->{PREVISSUE}] @@ -424,6 +476,32 @@ my $CBBI_mappings = [ }, ]; +# Tests +map { + t::lib::Mocks::mock_preference( 'checkprevcheckout', $_->{syspref} ); + my ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( $patron, $_->{item}->{barcode} ); + is( $needsconfirmation->{CURRENTISSUE}, $_->{result}, $_->{msg} ); +} @{$CBBI_mappings}; + +# Return $prev_item from patron: +BAIL_OUT("Return Failed") unless AddReturn( $prev_item->{barcode}, $patron->{branchcode} ); + +# Mappings +$CBBI_mappings = [ + { + syspref => 'hardno', + item => $prev_item, + result => undef, + msg => "patron, !wants_check_for_previous_checkout, do_check_for_previous_checkout" + }, + { + syspref => 'hardyes', + item => $prev_item, + result => 1, + msg => "patron, wants_check_for_previous_checkout, do_check_for_previous_checkout" + }, +]; + # Tests map { t::lib::Mocks::mock_preference( 'checkprevcheckout', $_->{syspref} ); @@ -456,7 +534,7 @@ subtest 'Check previous checkouts for serial' => sub { AddIssue( $patron, $item1->barcode ); is( - $patron->do_check_for_previous_checkout( $item1->unblessed ), 1, + $patron->do_check_for_previous_checkout( $item1->unblessed ), "currentlycheckedout", 'Check only one item if bibliographic record is serial' ); is( -- 2.39.5