Bugzilla – Attachment 180305 Details for
Bug 38356
CheckPrevCheckout should also check current loans
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38356: Check patrons current checkouts separately
Bug-38356-Check-patrons-current-checkouts-separate.patch (text/plain), 9.73 KB, created by
Felicie
on 2025-04-02 11:57:19 UTC
(
hide
)
Description:
Bug 38356: Check patrons current checkouts separately
Filename:
MIME Type:
Creator:
Felicie
Created:
2025-04-02 11:57:19 UTC
Size:
9.73 KB
patch
obsolete
>From 79d5f5588da99ca4517ef8fd3c49d4071522c163 Mon Sep 17 00:00:00 2001 >From: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> >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 <felicie.thiery@biblibre.com> >--- > 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 @@ > <li class="needsconfirm highholds">High demand item. Loan period shortened to [% HIGHHOLDS.duration | html %] days (due [% HIGHHOLDS.returndate | $KohaDates %]). Check out anyway?</li> > [% END %] > >+ [% IF CURRENTISSUE %] >+ <li class="needsconfirm currentissue">Patron has this title currently checked out: <strong>[% biblio.title | html %] [% IF biblio.author %]by [% biblio.author | html %][% END %]</strong>. Check out anyway?</li> >+ [% END %] >+ > [% IF PREVISSUE %] > <li class="needsconfirm previssue">Patron has previously checked out this title: <strong>[% biblio.title | html %] [% IF biblio.author %]by [% biblio.author | html %][% END %]</strong>. Check out anyway?</li> > [% 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 38356
:
180239
|
180305
|
181883
|
181884
|
182064
|
182065
|
182066