Bugzilla – Attachment 189750 Details for
Bug 23415
Notify patron fines when renewing
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23415: (follow-up) Rename sysprefs and add self-checkout control
4c8fc0c.patch (text/plain), 14.50 KB, created by
Martin Renvoize (ashimema)
on 2025-11-20 09:42:12 UTC
(
hide
)
Description:
Bug 23415: (follow-up) Rename sysprefs and add self-checkout control
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-11-20 09:42:12 UTC
Size:
14.50 KB
patch
obsolete
>From 4c8fc0c53b4843058fd3182dad18c3f025183954 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Thu, 20 Nov 2025 09:30:15 +0000 >Subject: [PATCH] Bug 23415: (follow-up) Rename sysprefs and add self-checkout > control >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This follow-up patch addresses feedback from the QA process: > >1. System preference naming consistency > Renamed the following preferences to remove "OPAC" prefix and align > with the FineNoRenewals naming convention: > - OPACFineNoRenewalsIncludeCredits â FineNoRenewalsIncludeCredits > - OPACFineNoRenewalsBlockAutoRenew â FineNoRenewalsBlockAutoRenew > >2. Self-checkout backward compatibility > Added new system preference FineNoRenewalsBlockSelfCheckRenew to > control whether fine limits block renewals via web-based self-checkout. > > Previously, OPACFineNoRenewals did not block self-checkout renewals. > With the rename to FineNoRenewals, this behavior changed. The new > preference defaults to "0" (don't block) to maintain backward > compatibility, allowing sites to opt-in to blocking self-checkout > renewals when patrons exceed the fine threshold. > >Test plan: >1. Run prove t/db_dependent/Circulation.t - all tests should pass >2. Run prove t/db_dependent/api/v1/checkouts.t - all tests should pass >3. Verify system preferences are correctly renamed in the admin interface >4. Test self-checkout renewals with FineNoRenewalsBlockSelfCheckRenew > set to 0 (default) - renewals should work even when fines exceed limit >5. Test self-checkout renewals with FineNoRenewalsBlockSelfCheckRenew > set to 1 - renewals should be blocked when fines exceed limit > >Sponsored-by: OpenFifth <https://openfifth.co.uk/> >Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> >--- > C4/Circulation.pm | 6 ++-- > .../data/mysql/atomicupdate/bug_23415.pl | 32 +++++++++++++++++-- > installer/data/mysql/mandatory/sysprefs.sql | 5 +-- > .../admin/preferences/circulation.pref | 9 +++++- > .../en/modules/admin/preferences/opac.pref | 2 +- > opac/opac-user.pl | 2 +- > opac/sco/sco-main.pl | 20 ++++++++++++ > t/db_dependent/Circulation.t | 16 +++++----- > 8 files changed, 73 insertions(+), 19 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 8326735adee..c49ee5571f6 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -3247,7 +3247,7 @@ sub CanBookBeRenewed { > my $balance = $account->balance; > my $non_issues_charges = $account->non_issues_charges; > my $amountoutstanding_for_renewal = >- C4::Context->preference("OPACFineNoRenewalsIncludeCredits") >+ C4::Context->preference("FineNoRenewalsIncludeCredits") > ? $balance > : $non_issues_charges; > >@@ -4993,11 +4993,11 @@ sub _CanBookBeAutoRenewed { > } > } > >- if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) { >+ if ( C4::Context->preference('FineNoRenewalsBlockAutoRenew') ) { > my $fine_no_renewals = C4::Context->preference("FineNoRenewals"); > my $account = $patron->account; > my $amountoutstanding = >- C4::Context->preference("OPACFineNoRenewalsIncludeCredits") >+ C4::Context->preference("FineNoRenewalsIncludeCredits") > ? $account->balance > : $account->non_issues_charges; > if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) { >diff --git a/installer/data/mysql/atomicupdate/bug_23415.pl b/installer/data/mysql/atomicupdate/bug_23415.pl >index 57a3bf96394..f098a7b8da8 100755 >--- a/installer/data/mysql/atomicupdate/bug_23415.pl >+++ b/installer/data/mysql/atomicupdate/bug_23415.pl >@@ -2,8 +2,9 @@ use Modern::Perl; > > return { > bug_number => "23415", >- description => "Rename OPACFineNoRenewals and add new syspref AllowFineOverrideRenewing", >- up => sub { >+ description => >+ "Rename OPACFineNoRenewals and related sysprefs, add new sysprefs AllowFineOverrideRenewing and FineNoRenewalsBlockSelfCheckRenew", >+ up => sub { > my ($args) = @_; > my ( $dbh, $out ) = @$args{qw(dbh out)}; > >@@ -13,7 +14,23 @@ return { > } > ); > >- say $out "Updated system preference 'OPACFineNoRenewals'"; >+ say $out "Updated system preference 'OPACFineNoRenewals' to 'FineNoRenewals'"; >+ >+ $dbh->do( >+ q{ >+ UPDATE IGNORE systempreferences SET variable = "FineNoRenewalsIncludeCredits" WHERE variable = "OPACFineNoRenewalsIncludeCredits"; >+ } >+ ); >+ >+ say $out "Updated system preference 'OPACFineNoRenewalsIncludeCredits' to 'FineNoRenewalsIncludeCredits'"; >+ >+ $dbh->do( >+ q{ >+ UPDATE IGNORE systempreferences SET variable = "FineNoRenewalsBlockAutoRenew" WHERE variable = "OPACFineNoRenewalsBlockAutoRenew"; >+ } >+ ); >+ >+ say $out "Updated system preference 'OPACFineNoRenewalsBlockAutoRenew' to 'FineNoRenewalsBlockAutoRenew'"; > > $dbh->do( > q{ >@@ -23,5 +40,14 @@ return { > ); > > say $out "Added new system preference 'AllowFineOverrideRenewing'"; >+ >+ $dbh->do( >+ q{ >+ INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) >+ VALUES ('FineNoRenewalsBlockSelfCheckRenew', '0', '', 'Block self-checkout renewals if the patron owes more than the value of FineNoRenewals.','YesNo') >+ } >+ ); >+ >+ say $out "Added new system preference 'FineNoRenewalsBlockSelfCheckRenew'"; > }, > }; >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index f004beb8983..e2289aee652 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -508,8 +508,9 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('OpacExportOptions','bibtex,dc,marcxml,marc8,utf8,marcstd,mods,ris,isbd','','Define export options available on OPAC detail page.','multiple'), > ('OPACFallback', 'prog', 'bootstrap|prog', 'Define the fallback theme for the OPAC interface.', 'Themes'), > ('OpacFavicon','','','Enter a complete URL to an image to replace the default Koha favicon on the OPAC','free'), >-('OPACFineNoRenewalsBlockAutoRenew','0','','Block/Allow auto renewals if the patron owe more than FineNoRenewals','YesNo'), >-('OPACFineNoRenewalsIncludeCredits','1',NULL,'If enabled the value specified in FineNoRenewals should include any unapplied account credits in the calculation','YesNo'), >+('FineNoRenewalsBlockAutoRenew','0','','Block/Allow auto renewals if the patron owes more than FineNoRenewals','YesNo'), >+('FineNoRenewalsBlockSelfCheckRenew','0','','Block self-checkout renewals if the patron owes more than the value of FineNoRenewals','YesNo'), >+('FineNoRenewalsIncludeCredits','1',NULL,'If enabled the value specified in FineNoRenewals should include any unapplied account credits in the calculation','YesNo'), > ('OPACFinesTab','1','','If OFF the patron fines tab in the OPAC is disabled.','YesNo'), > ('OPACFRBRizeEditions','0','','If ON, the OPAC will query one or more ISBN web services for associated ISBNs and display an Editions tab on the details pages','YesNo'), > ('OpacHiddenItems','','','This syspref allows to define custom rules for hiding specific items at the OPAC. See https://wiki.koha-community.org/wiki/OpacHiddenItems for more information.','Textarea'), >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 06e9495a4a6..dd94f7c7b6b 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 >@@ -503,11 +503,18 @@ Circulation: > block: block renewing for all the patron's items. > - > - If a patron owes more than the value of <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=FineNoRenewals">FineNoRenewals</a>, >- - pref: OPACFineNoRenewalsBlockAutoRenew >+ - pref: FineNoRenewalsBlockAutoRenew > choices: > 1: block > 0: allow > - their auto-renewals. >+ - >+ - If a patron owes more than the value of <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=FineNoRenewals">FineNoRenewals</a>, >+ - pref: FineNoRenewalsBlockSelfCheckRenew >+ choices: >+ 1: block >+ 0: allow >+ - renewals via the self-checkout system. > - > - If a patron pays off all fines on an overdue item that is accruing fines, > - pref: RenewAccruingItemWhenPaid >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref >index 42df619c0d1..59d8e1ac049 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref >@@ -646,7 +646,7 @@ OPAC: > opacrenew: "'OPACRenew'" > - as branchcode to store in the statistics table for renewals done in the OPAC. > - >- - pref: OPACFineNoRenewalsIncludeCredits >+ - pref: FineNoRenewalsIncludeCredits > choices: > 1: Include > 0: "Don't include" >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index 4b59548d0f1..337f37d2477 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -128,7 +128,7 @@ my $amountoutstanding = $patron->account->balance; > my $no_renewal_amt = C4::Context->preference('FineNoRenewals'); > $no_renewal_amt = undef unless looks_like_number($no_renewal_amt); > my $amountoutstandingfornewal = >- C4::Context->preference("OPACFineNoRenewalsIncludeCredits") >+ C4::Context->preference("FineNoRenewalsIncludeCredits") > ? $amountoutstanding > : $patron->account->outstanding_debits->total_outstanding; > >diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl >index 7741ee8707f..a0cca4a3836 100755 >--- a/opac/sco/sco-main.pl >+++ b/opac/sco/sco-main.pl >@@ -333,6 +333,16 @@ if ( $patron && ( $op eq 'cud-renew' ) ) { > > if ( $patron->checkouts->find( { itemnumber => $item->itemnumber } ) ) { > my ( $status, $renewerror ) = CanBookBeRenewed( $patron, $item->checkout ); >+ >+ # Override FineNoRenewals check if FineNoRenewalsBlockSelfCheckRenew is disabled >+ if ( !$status >+ && $renewerror eq 'too_much_owing' >+ && !C4::Context->preference('FineNoRenewalsBlockSelfCheckRenew') ) >+ { >+ $status = 1; >+ $renewerror = undef; >+ } >+ > if ($status) { > AddRenewal( > { >@@ -361,6 +371,16 @@ if ($patron) { > while ( my $c = $pending_checkouts->next ) { > my $checkout = $c->unblessed_all_relateds; > my ( $can_be_renewed, $renew_error ) = CanBookBeRenewed( $patron, $c ); >+ >+ # Override FineNoRenewals check if FineNoRenewalsBlockSelfCheckRenew is disabled >+ if ( !$can_be_renewed >+ && $renew_error eq 'too_much_owing' >+ && !C4::Context->preference('FineNoRenewalsBlockSelfCheckRenew') ) >+ { >+ $can_be_renewed = 1; >+ $renew_error = undef; >+ } >+ > $checkout->{can_be_renewed} = $can_be_renewed; # In the future this will be $checkout->can_be_renewed > $checkout->{renew_error} = $renew_error; > $checkout->{overdue} = $c->is_overdue; >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index eff67f6cc81..e1418d7bb4f 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -1325,7 +1325,7 @@ subtest "CanBookBeRenewed tests" => sub { > is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); > }; > >- subtest "auto_too_much_owing | OPACFineNoRenewalsBlockAutoRenew & OPACFineNoRenewalsIncludeCredits" => sub { >+ subtest "auto_too_much_owing | FineNoRenewalsBlockAutoRenew & FineNoRenewalsIncludeCredits" => sub { > plan tests => 10; > my $item_to_auto_renew = $builder->build_sample_item( > { >@@ -1352,9 +1352,9 @@ subtest "CanBookBeRenewed tests" => sub { > } > } > ); >- C4::Context->set_preference( 'OPACFineNoRenewalsBlockAutoRenew', '1' ); >- C4::Context->set_preference( 'FineNoRenewals', '10' ); >- C4::Context->set_preference( 'OPACFineNoRenewalsIncludeCredits', '1' ); >+ C4::Context->set_preference( 'FineNoRenewalsBlockAutoRenew', '1' ); >+ C4::Context->set_preference( 'FineNoRenewals', '10' ); >+ C4::Context->set_preference( 'FineNoRenewalsIncludeCredits', '1' ); > my $fines_amount = 5; > my $account = Koha::Account->new( { patron_id => $renewing_borrowernumber } ); > $account->add_debit( >@@ -1408,19 +1408,19 @@ subtest "CanBookBeRenewed tests" => sub { > is( $renewokay, 1, 'Renew, even if renewal is automatic' ); > is( > $error, 'auto_renew', >- 'Can auto renew, FineNoRenewals=10, OPACFineNoRenewalsIncludeCredits=1, patron has 15 debt, 5 credit' >+ 'Can auto renew, FineNoRenewals=10, FineNoRenewalsIncludeCredits=1, patron has 15 debt, 5 credit' > ); > >- C4::Context->set_preference( 'OPACFineNoRenewalsIncludeCredits', '0' ); >+ C4::Context->set_preference( 'FineNoRenewalsIncludeCredits', '0' ); > ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( > $error, 'auto_too_much_owing', >- 'Cannot auto renew, FineNoRenewals=10, OPACFineNoRenewalsIncludeCredits=1, patron has 15 debt, 5 credit' >+ 'Cannot auto renew, FineNoRenewals=10, FineNoRenewalsIncludeCredits=0, patron has 15 debt, 5 credit' > ); > > $dbh->do( 'DELETE FROM accountlines WHERE borrowernumber=?', undef, $renewing_borrowernumber ); >- C4::Context->set_preference( 'OPACFineNoRenewalsIncludeCredits', '1' ); >+ C4::Context->set_preference( 'FineNoRenewalsIncludeCredits', '1' ); > }; > > subtest "auto_account_expired | BlockExpiredPatronOpacActions" => sub { >-- >2.51.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 23415
:
92039
|
92541
|
98442
|
98634
|
98996
|
98997
|
105893
|
105894
|
108392
|
109871
|
109872
|
109876
|
109880
|
116919
|
116920
|
116921
|
116922
|
118175
|
118176
|
118177
|
118178
|
118183
|
119300
|
119301
|
119302
|
119303
|
121472
|
139670
|
139671
|
139672
|
139673
|
139674
|
146415
|
148198
|
148199
|
148200
|
148201
|
148202
|
152776
|
152777
|
152778
|
152779
|
152780
|
152781
|
154595
|
154596
|
154597
|
154598
|
154599
|
154600
|
158302
|
158303
|
158304
|
158305
|
158306
|
158307
|
161964
|
161965
|
161966
|
161967
|
161968
|
161969
|
162171
|
162172
|
162173
|
162174
|
162175
|
162176
|
162177
|
162178
|
162179
|
162180
|
171620
|
171621
|
171622
|
171623
|
171983
|
171984
|
171985
|
171986
|
172906
|
172907
|
172908
|
172909
|
172910
|
185270
|
185271
|
185272
|
185273
|
185274
|
185275
|
186014
|
188331
|
188332
|
188333
|
188334
|
188335
|
188336
|
188337
|
188338
|
188339
|
188340
|
188341
|
188342
|
188708
|
188709
|
188710
|
188711
|
188712
|
188713
|
188714
|
188715
|
188716
|
188717
|
188718
|
188719
|
188720
|
188738
|
188739
|
188740
|
188741
|
188742
|
188743
|
188744
|
188745
|
188746
|
188747
|
188748
|
188749
|
188750
|
188751
|
189721
|
189722
|
189723
|
189724
|
189725
|
189726
|
189727
|
189728
|
189729
|
189730
|
189731
|
189732
|
189733
|
189734
|
189735
|
189736
|
189737
|
189738
|
189739
|
189740
|
189741
|
189742
|
189743
|
189744
|
189745
|
189746
|
189747
|
189748
|
189749
| 189750