From 30005e36b9e105cf0a572779219967c5f9e67e82 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 Signed-off-by: Martin Renvoize --- 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 FineNoRenewals, - - pref: OPACFineNoRenewalsBlockAutoRenew + - pref: FineNoRenewalsBlockAutoRenew choices: 1: block 0: allow - their auto-renewals. + - + - If a patron owes more than the value of FineNoRenewals, + - 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