Bugzilla – Attachment 144774 Details for
Bug 29145
Allow patrons to have overdue items that would not result in debarment when removing overdue debarments
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29145: Add tests and modify sysprefs
Bug-29145-Add-tests-and-modify-sysprefs.patch (text/plain), 14.60 KB, created by
David Gustafsson
on 2022-12-21 16:42:31 UTC
(
hide
)
Description:
Bug 29145: Add tests and modify sysprefs
Filename:
MIME Type:
Creator:
David Gustafsson
Created:
2022-12-21 16:42:31 UTC
Size:
14.60 KB
patch
obsolete
>From 743765b14a0c7e0d053aa2bfb21f78e88ed8cf2d Mon Sep 17 00:00:00 2001 >From: David Gustafsson <david.gustafsson@ub.gu.se> >Date: Wed, 21 Sep 2022 16:09:51 +0200 >Subject: [PATCH] Bug 29145: Add tests and modify sysprefs > >Add tests, remove ODueDebarmentRemovalAllowUnrestricted syspref and instead >modify AutoRemoveOverduesRestrictions to have a third option >--- > C4/Circulation.pm | 22 ++--- > Koha/Patron.pm | 4 +- > ..._AutoRemoveOverduesRestrictions-syspref.pl | 13 +++ > ...ue-debarment-removal-allow-unrestricted.pl | 14 --- > installer/data/mysql/mandatory/sysprefs.sql | 2 +- > .../admin/preferences/circulation.pref | 15 ++-- > .../Circulation/MarkIssueReturned.t | 89 +++++++++++++++++-- > 7 files changed, 116 insertions(+), 43 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl > delete mode 100644 installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 0d691fd26a..ce33dde3e6 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2496,6 +2496,7 @@ sub MarkIssueReturned { > > # Retrieve the issue > my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return; >+ my $issue_branchcode = $issue->branchcode; > > return unless $issue->borrowernumber == $borrowernumber; # If the item is checked out to another patron we do not return it > >@@ -2542,18 +2543,17 @@ sub MarkIssueReturned { > # The reason this is here, and not in Koha::Patron->has_overdues() is > # to make sure it will not cause any side effects elsewhere, since this > # is only relevant for removal of debarments. >- my $has_overdue_ignore_unrestricted = 0; >- if(C4::Context->preference('ODueDebarmentRemovalAllowUnrestricted')) { >- $has_overdue_ignore_unrestricted = 1; >- } >+ my $has_overdue_ignore_unrestricted = C4::Context->preference('AutoRemoveOverduesRestrictions') eq 'when_no_overdue_causing_debarment'; > >- # Remove any OVERDUES related debarment if the borrower has no overdues >- if ( C4::Context->preference('AutoRemoveOverduesRestrictions') >- && $patron->debarred >- && !$patron->has_overdues({ >- ignore_unrestricted => $has_overdue_ignore_unrestricted, >- issue_branch => $issue->{'branchcode'} }) >- && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) } >+ # Possibly remove any OVERDUES related debarment >+ if ( >+ C4::Context->preference('AutoRemoveOverduesRestrictions') ne 'no' >+ && $patron->debarred >+ && !$patron->has_overdues({ >+ ignore_unrestricted => $has_overdue_ignore_unrestricted, >+ issue_branchcode => $issue_branchcode >+ }) >+ && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) } > ) { > DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); > } >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 2cb28dd86b..ffa771104a 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1002,11 +1002,11 @@ sub _get_overdue_restrict_delay { > > my $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = ? AND categorycode = ?"; > >- my $rqoverduerules = $dbh->prepare($query); >+ my $rqoverduerules = $dbh->prepare($query); > $rqoverduerules->execute($branchcode, $categorycode); > > # We get default rules if there is no rule for this branch >- if($rqoverduerules->rows == 0){ >+ if($rqoverduerules->rows == 0) { > $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = '' AND categorycode = ?"; > > $rqoverduerules = $dbh->prepare($query); >diff --git a/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl b/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl >new file mode 100644 >index 0000000000..ca210104c4 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl >@@ -0,0 +1,13 @@ >+use Modern::Perl; >+ >+return { >+ bug_number => "", >+ description => "Change type of AutoRemoveOverduesRestrictions system preference", >+ up => sub { >+ my ($args) = @_; >+ my ($dbh, $out) = @$args{qw(dbh out)}; >+ # Do you stuffs here >+ $dbh->do(q{UPDATE `systempreferences` SET `type` = 'Choice', `options` = 'no|when_no_overdue|when_no_overdue_causing_debarment', `explanation` = 'Defines if and on what conditions OVERDUES debarments should automatically be lifted when overdue items are returned by the patron.', `value` = CASE `value` WHEN '1' THEN 'when_no_overdue' WHEN '0' THEN 'no' ELSE `value` END WHERE variable = 'AutoRemoveOverduesRestrictions'}); >+ say $out "Type of AutoRemoveOverduesRestrictions system prefernce has ben changed"; >+ }, >+} >diff --git a/installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl b/installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl >deleted file mode 100644 >index 3a019999d1..0000000000 >--- a/installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl >+++ /dev/null >@@ -1,14 +0,0 @@ >-use Modern::Perl; >- >-return { >- bug_number => "", >- description => "Add system preference", >- up => sub { >- my ($args) = @_; >- my ($dbh, $out) = @$args{qw(dbh out)}; >- # Do you stuffs here >- $dbh->do(q{INSERT IGNORE INTO systempreferences (`variable`, `value`, `options`, `explanation`, `type`) VALUES ('ODueDebarmentRemovalAllowUnrestricted', '0', null, 'Allow removal of OVERDUES debarment when overdues still exist, but has not reached restricting delay', 'YesNo')}); >- # Print useful stuff here >- say $out "System preference added"; >- }, >-} >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index beebd27a6f..1a277a25bd 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -86,7 +86,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('AutoLinkBiblios','0',NULL,'If enabled, link biblio to authorities on creation and edit','YesNo'), > ('AutomaticWrongTransfer','0',NULL,'Defines whether transfers should be automatically confirmed at checkin if modal dismissed','YesNo'), > ('autoMemberNum','0','','If ON, patron number is auto-calculated','YesNo'), >-('AutoRemoveOverduesRestrictions','0',NULL,'Defines whether an OVERDUES debarment should be lifted automatically if all overdue items are returned by the patron.','YesNo'), >+('AutoRemoveOverduesRestrictions','no','no|when_no_overdue|when_no_overdue_causing_debarment', 'Defines if and on what conditions OVERDUES debarments should automatically be lifted when overdue items are returned by the patron.','Choice'), > ('AutoRenewalNotices','cron','cron|preferences|never','How should Koha determine whether to end autorenewal notices','Choice'), > ('AutoResumeSuspendedHolds','1',NULL,'Allow suspended holds to be automatically resumed by a set date.','YesNo'), > ('AutoReturnCheckedOutItems', '0', '', 'If disabled, librarian must confirm return of checked out item when checking out to another.', 'YesNo'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >index e38185fcdc..8e1320bf72 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >@@ -669,12 +669,6 @@ Circulation: > 1: Store > 0: "Don't store" > - 'the last patron to return an item. This setting is independent of the <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=opacreadinghistory">opacreadinghistory</a> and <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=AnonymousPatron">AnonymousPatron</a> system preferences.' >- - >- - pref: ODueDebarmentRemovalAllowUnrestricted >- choices: >- yes: Allow >- no: Do not allow >- - removal of Overdue debarments when patron has overdue items but none are old enough to have reached restricting delay. Used in combination with AutoRemoveOverduesRestrictions. > Holds policy: > - > - pref: EnableItemGroups >@@ -1000,11 +994,14 @@ Circulation: > 0: "Don't cumulate" > - the restriction periods. > - >+ - When returning items > - pref: AutoRemoveOverduesRestrictions >+ type: choice > choices: >- 1: "Do" >- 0: "Don't" >- - allow OVERDUES restrictions triggered by sent notices to be cleared automatically when all overdue items are returned by a patron. >+ when_no_overdue: "if patron has no remaining overdue items" >+ when_no_overdue_causing_debarment: "if patron has no remaining items that is cause for debarment" >+ no: "don't" >+ - remove OVERDUES restriction triggered by sent notices. The difference between removing restriction when no remaining overdue items exists and doing so only when any of the items would result in debarment is that the latter option will respect possible grace periods of overdue rules also on returns and not only when generating overdue notices. > - > - If a patron is restricted, > - pref: RestrictionBlockRenewing >diff --git a/t/db_dependent/Circulation/MarkIssueReturned.t b/t/db_dependent/Circulation/MarkIssueReturned.t >index 654ac37451..21a4227d46 100755 >--- a/t/db_dependent/Circulation/MarkIssueReturned.t >+++ b/t/db_dependent/Circulation/MarkIssueReturned.t >@@ -173,20 +173,37 @@ subtest 'Manually pass a return date' => sub { > }; > > subtest 'AutoRemoveOverduesRestrictions' => sub { >- plan tests => 2; >+ plan tests => 5; > > $schema->storage->txn_begin; > >- t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 1); >- >+ my $dbh = C4::Context->dbh; > my $patron = $builder->build_object({ class => 'Koha::Patrons' }); >- t::lib::Mocks::mock_userenv( { branchcode => $patron->branchcode } ); >+ my $categorycode = $patron->categorycode; >+ my $branchcode = $patron->branchcode; >+ >+ $dbh->do(qq{ >+ INSERT INTO `overduerules` ( >+ `categorycode`, >+ `delay1`, >+ `letter1`, >+ `debarred1`, >+ `delay2`, >+ `letter2`, >+ `debarred2` >+ ) >+ VALUES ('$categorycode', 6, 'ODUE', 0, 10, 'ODUE2', 1) >+ }); >+ >+ t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue'); >+ >+ t::lib::Mocks::mock_userenv( { branchcode => $branchcode } ); > my $item_1 = $builder->build_sample_item; > my $item_2 = $builder->build_sample_item; > my $item_3 = $builder->build_sample_item; > my $five_days_ago = dt_from_string->subtract( days => 5 ); >- my $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # overdue >- my $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue >+ my $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # overdue, but would not trigger debarment >+ my $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue, but would not trigger debarment > my $checkout_3 = AddIssue( $patron->unblessed, $item_3->barcode ); # not overdue > > Koha::Patron::Debarments::AddUniqueDebarment( >@@ -207,5 +224,65 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { > $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); > is( scalar @$debarments, 0, 'OVERDUES debarment is removed if patron does not have overdues' ); > >+ t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue_causing_debarment'); >+ >+ my $eleven_days_ago = dt_from_string->subtract( days => 11 ); >+ >+ $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $eleven_days_ago ); # overdue and would trigger debarment >+ $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue, but would not trigger debarment >+ >+ Koha::Patron::Debarments::AddUniqueDebarment( >+ { >+ borrowernumber => $patron->borrowernumber, >+ type => 'OVERDUES', >+ comment => "OVERDUES_PROCESS simulation", >+ } >+ ); >+ >+ C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_1->itemnumber ); >+ >+ $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); >+ is( scalar @$debarments, 0, 'OVERDUES debarment is removed if remaning items would not result in patron debarment' ); >+ >+ $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $eleven_days_ago ); # overdue and would trigger debarment >+ >+ Koha::Patron::Debarments::AddUniqueDebarment( >+ { >+ borrowernumber => $patron->borrowernumber, >+ type => 'OVERDUES', >+ comment => "OVERDUES_PROCESS simulation", >+ } >+ ); >+ >+ C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber ); >+ >+ $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); >+ is( $debarments->[0]->{type}, 'OVERDUES', 'OVERDUES debarment is not removed if patron still has overdues that would trigger debarment' ); >+ >+ my $thirteen_days_ago = dt_from_string->subtract( days => 13 ); >+ >+ # overdue and would trigger debarment >+ $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $thirteen_days_ago ); >+ >+ # $chechout_1 should now not trigger debarment with this new rule for specific branchcode >+ $dbh->do(qq{ >+ INSERT INTO `overduerules` ( >+ `branchcode`, >+ `categorycode`, >+ `delay1`, >+ `letter1`, >+ `debarred1`, >+ `delay2`, >+ `letter2`, >+ `debarred2` >+ ) >+ VALUES ('$branchcode', '$categorycode', 6, 'ODUE', 0, 12, 'ODUE2', 1) >+ }); >+ >+ C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber ); >+ >+ $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); >+ is( scalar @$debarments, 0, 'OVERDUES debarment is removed if remaning items would not result in patron debarment' ); >+ > $schema->storage->txn_rollback; > }; >-- >2.35.1
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 29145
:
125495
|
140819
|
140820
|
141822
|
141823
|
142394
|
142395
|
142396
|
142814
|
142815
|
142816
|
142817
|
143213
|
144773
|
144774
|
144775
|
144776
|
144777
|
144778
|
144779
|
144780
|
144781
|
144782
|
145720
|
145749
|
145750
|
145751
|
153804
|
153805
|
153806
|
153807
|
153808
|
153809
|
153810
|
153811
|
153812
|
153813
|
153814
|
156392
|
156393
|
156394
|
156395
|
156396
|
156397
|
156398
|
156399
|
156400
|
156401
|
156402
|
156403
|
156404