Bug 23415: (follow-up) Rename sysprefs and add self-checkout control
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>
This commit is contained in:
parent
06fd7ed7e4
commit
1eb1cfdbef
|
|
@ -3275,7 +3275,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;
|
||||
|
||||
|
|
@ -5052,11 +5052,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 ) {
|
||||
|
|
|
|||
|
|
@ -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'";
|
||||
},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -295,6 +295,9 @@ INSERT INTO systempreferences ( `variable`, `value` ) VALUES
|
|||
('FilterBeforeOverdueReport','0'),
|
||||
('FilterSearchResultsByLoggedInBranch','0'),
|
||||
('FineNoRenewals','100'),
|
||||
('FineNoRenewalsBlockAutoRenew','0'),
|
||||
('FineNoRenewalsBlockSelfCheckRenew','0'),
|
||||
('FineNoRenewalsIncludeCredits','1'),
|
||||
('FineNotifyAtCheckin','0'),
|
||||
('FinePaymentAutoPopup','0'),
|
||||
('finesCalendar','noFinesWhenClosed'),
|
||||
|
|
@ -516,8 +519,6 @@ INSERT INTO systempreferences ( `variable`, `value` ) VALUES
|
|||
('OpacExportOptions','bibtex,dc,marcxml,marc8,utf8,marcstd,mods,ris,isbd'),
|
||||
('OPACFallback', 'prog'),
|
||||
('OpacFavicon',''),
|
||||
('OPACFineNoRenewalsBlockAutoRenew','0'),
|
||||
('OPACFineNoRenewalsIncludeCredits','1'),
|
||||
('OPACFinesTab','1'),
|
||||
('OPACFRBRizeEditions','0'),
|
||||
('OpacHiddenItems',''),
|
||||
|
|
|
|||
|
|
@ -516,11 +516,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
|
||||
|
|
|
|||
|
|
@ -652,7 +652,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"
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -497,7 +497,7 @@ subtest "GetIssuingCharges tests" => sub {
|
|||
|
||||
my ( $reused_itemnumber_1, $reused_itemnumber_2 );
|
||||
subtest "CanBookBeRenewed tests" => sub {
|
||||
plan tests => 120;
|
||||
plan tests => 121;
|
||||
|
||||
C4::Context->set_preference( 'ItemsDeniedRenewal', '' );
|
||||
|
||||
|
|
@ -1324,7 +1324,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(
|
||||
{
|
||||
|
|
@ -1351,9 +1351,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(
|
||||
|
|
@ -1407,19 +1407,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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue