From fc99557e96142d3d3b9c65b5113dd7a13394c79f 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 Signed-off-by: Lucas Gass --- 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 60dc4dc765e..fdfbd8b78e5 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1066,8 +1066,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 56df7f9b8da..55e06599574 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -865,6 +865,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; @@ -872,13 +876,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 ad2dcc64125..556cee4e4df 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 cb5480d5e74..7d643d5a089 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