gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 23415: Fix FineNoRenewals to use non_issues_charges and improve template 

This commit addresses two issues:

1. Fixes FineNoRenewals logic to properly use non_issues_charges
   Both CanBookBeRenewed() and _CanBookBeAutoRenewed() were incorrectly
   using the patron's total balance when checking against FineNoRenewals.
   They now use non_issues_charges, which returns only charges that
   should restrict checkouts (e.g., overdues, lost items) as defined by
   debit types with restricts_checkouts=1. This ensures charges like
   rental fees don't inappropriately block renewals.
2. Improves renewal template for AllowFineOverrideRenewing
   - Adds special handling for auto_too_much_owing error to check
     AllowFineOverrideRenewing preference instead of
     AllowRenewalLimitOverride
   - Adds missing id="barcode" attribute to input field for Cypress tests
   - Adds .dialog CSS class to error/success divs for consistent styling
   - Fixes HTML structure with proper <ul><li> wrapping
   - Adds btn btn-default classes to override button
3. Fixes Cypress test issues
   - Corrects data structure access: items[0] instead of item
   - Uses external_id instead of barcode (API field name)
   - Improves form submission selector specificity
   - Uses REPLACE INTO for system preference setup

Test plan:
1. Run t/cypress/integration/Circulation/FineNoRenewals_spec.ts
2. Verify 4 out of 5 tests pass (one remaining template comparison issue)
3. Manually test that patrons with non-issue charges (rental fees) can
   renew when those charges alone don't exceed FineNoRenewals
4. Verify that patrons with issue-blocking charges (overdues) are
   properly blocked from renewal when exceeding FineNoRenewals

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:
Martin Renvoize 2025-10-30 13:34:08 +00:00
parent 018e65c811
commit 4b88b1de3d
No known key found for this signature in database
GPG Key ID: 2D86E0EAEC83711B
3 changed files with 66 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
C4/Circulation.pm
View File

@ -3271,7 +3271,9 @@ sub CanBookBeRenewed {
my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
my $restricted = $patron->is_debarred;
my $hasoverdues = $patron->has_overdues;
my $balance = $patron->account->balance;
my $account = $patron->account;
my $balance = $account->balance;
my $non_issues_charges = $account->non_issues_charges;
if ( $restricted and $restrictionblockrenewing ) {
return ( 0, 'restriction' );
@ -3279,7 +3281,7 @@ sub CanBookBeRenewed {
|| ( $issue->is_overdue and $overduesblockrenewing eq 'blockitem' ) )
{
return ( 0, 'overdue' );
} elsif ( $finesblockrenewing && $balance > $finesblockrenewing ) {
} elsif ( $finesblockrenewing && $non_issues_charges > $finesblockrenewing ) {
return ( 0, 'too_much_owing' );
}
@ -5047,12 +5049,10 @@ sub _CanBookBeAutoRenewed {
}
if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
my $fine_no_renewals = C4::Context->preference("FineNoRenewals");
my $amountoutstanding =
C4::Context->preference("OPACFineNoRenewalsIncludeCredits")
? $patron->account->balance
: $patron->account->outstanding_debits->total_outstanding;
if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
my $fine_no_renewals = C4::Context->preference("FineNoRenewals");
my $account = $patron->account;
my $non_issues_charges = $account->non_issues_charges;
if ( $non_issues_charges and $non_issues_charges > $fine_no_renewals ) {
return "auto_too_much_owing";
}
}

48
koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
View File

@ -36,7 +36,7 @@
[% SET aside = Koha.Preference('CircSidebar') ? 'circ-nav' : '' %]
[% WRAPPER 'main-container.inc' aside=aside %]
[% IF error %]
<div class="alert alert-warning">
<div class="dialog alert alert-warning">
<h1>Cannot renew:</h1>
[% IF error == "no_item" %]
@ -128,26 +128,44 @@
[% END %]
[% ELSIF error == "auto_renew" or error == "auto_too_much_owing" %]
<p>[% INCLUDE 'biblio-title.inc' biblio=item.biblio link = 1 %] ( [% item.barcode | html %] ) has been scheduled for automatic renewal. </p>
[% SET override_limit = error == "auto_renew" ? "AUTO_RENEW" : "TOO_MUCH" %]
[% IF Koha.Preference('AllowRenewalLimitOverride') %]
<form method="post" action="/cgi-bin/koha/circ/renew.pl">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="barcode" value="[% item.barcode | html %]" />
<input type="hidden" name="override_limit" value="[% override_limit | html %]" />
<input type="hidden" name="op" value="cud-renew" />
<button type="submit" class="btn btn-default approve"><i class="fa fa-check"></i> Override limit and renew</button>
</form>
[% IF error == "auto_too_much_owing" %]
[% IF balance %]
<ul>
<li>The patron has a debt of [% balance | $Price %] and renewals are restricted at [% Koha.Preference('FineNoRenewals') | $Price %].</li>
</ul>
[% END %]
[% IF Koha.Preference('AllowFineOverrideRenewing') %]
<form method="post" action="/cgi-bin/koha/circ/renew.pl">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="barcode" value="[% item.barcode | html %]" />
<input type="hidden" name="override_debt" value="1" />
<input type="hidden" name="op" value="cud-renew" />
<button type="submit" class="btn btn-default approve"><i class="fa fa-check"></i> Override and renew</button>
</form>
[% END %]
[% ELSE %]
[% SET override_limit = "AUTO_RENEW" %]
[% IF Koha.Preference('AllowRenewalLimitOverride') %]
<form method="post" action="/cgi-bin/koha/circ/renew.pl">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="barcode" value="[% item.barcode | html %]" />
<input type="hidden" name="override_limit" value="[% override_limit | html %]" />
<input type="hidden" name="op" value="cud-renew" />
<button type="submit" class="btn btn-default approve"><i class="fa fa-check"></i> Override limit and renew</button>
</form>
[% END %]
[% END %]
[% ELSIF error == "too_much_owing" %]
<li>The patron has a debt of [% balance | $Price %].</li>
<ul>
<li>The patron has a debt of [% balance | $Price %] and renewals are restricted at [% Koha.Preference('FineNoRenewals') | $Price %].</li>
</ul>
[% IF Koha.Preference('AllowFineOverrideRenewing') %]
<form method="post" action="/cgi-bin/koha/circ/renew.pl">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="barcode" value="[% item.barcode | html %]" />
<input type="hidden" name="override_debt" value="1" />
<input type="hidden" name="op" value="cud-renew" />
<button type="submit" class="approve"><i class="fa fa-check"></i>Renew checkout(s)</button>
<button type="submit" class="btn btn-default approve"><i class="fa fa-check"></i> Override and renew</button>
</form>
[% END %]
[% ELSIF error == "on_reserve" %]
@ -189,7 +207,7 @@
[% END %]
[% IF date_due %]
<div class="alert alert-info">
<div class="dialog message alert alert-info">
<h1>Item renewed:</h1>
<p>
[% INCLUDE 'biblio-title.inc' biblio=item.biblio link = 1 %] (
@ -219,7 +237,7 @@
<input value="1" name="unseen" id="unseen" type="checkbox" />
</div>
[% ELSE %]
<input name="barcode" size="14" class="barcode focus" type="text" placeholder="Enter item barcode" />
<input name="barcode" id="barcode" size="14" class="barcode focus" type="text" placeholder="Enter item barcode" />
[% END %]
<input type="hidden" name="op" value="cud-renew" />
<div id="show-circ-settings">

56
t/cypress/integration/Circulation/FineNoRenewals_spec.ts
View File

@ -13,12 +13,15 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
patron = objects.patron;
checkout = objects.checkout;
// Set FineNoRenewals to 5
// Disable auto_renew on the checkout to get too_much_owing error instead of auto_too_much_owing
cy.task("query", {
sql: "UPDATE systempreferences SET value = ? WHERE variable = ?",
values: ["5", "FineNoRenewals"],
sql: "UPDATE issues SET auto_renew = 0 WHERE issue_id = ?",
values: [checkout.checkout_id],
});
// Set FineNoRenewals to 5
cy.set_syspref("FineNoRenewals", "5");
// Add a fine of 10 to the patron (exceeds limit)
fine_amount = 10;
cy.task("query", {
@ -27,7 +30,7 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
values: [
patron.patron_id,
fine_amount,
objects.item.item_id,
objects.items[0].item_id,
],
});
@ -40,23 +43,19 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
cy.task("deleteSampleObjects", [this.testObjects]);
// Reset system preferences
cy.task("query", {
sql: "UPDATE systempreferences SET value = '100' WHERE variable = 'FineNoRenewals'",
});
cy.task("query", {
sql: "UPDATE systempreferences SET value = '0' WHERE variable = 'AllowFineOverrideRenewing'",
});
cy.set_syspref("FineNoRenewals", "100");
cy.set_syspref("AllowFineOverrideRenewing", "0");
});
it("should block renewal when patron has fines over FineNoRenewals limit", function () {
const barcode = this.testObjects.item.barcode;
const barcode = this.testObjects.items[0].external_id;
// Visit renewal page
cy.visit("/cgi-bin/koha/circ/renew.pl");
// Enter barcode
cy.get("#barcode").type(barcode);
cy.get("form").submit();
cy.get("#barcode").closest("form").submit();
// Should see error message about patron debt
cy.get(".dialog.alert").should("be.visible");
@ -68,21 +67,20 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
it("should show override button when AllowFineOverrideRenewing is enabled", function () {
// Enable AllowFineOverrideRenewing
cy.task("query", {
sql: "UPDATE systempreferences SET value = '1' WHERE variable = 'AllowFineOverrideRenewing'",
});
cy.set_syspref("AllowFineOverrideRenewing", "1");
const barcode = this.testObjects.item.barcode;
const barcode = this.testObjects.items[0].external_id;
// Visit renewal page
cy.visit("/cgi-bin/koha/circ/renew.pl");
// Enter barcode
cy.get("#barcode").type(barcode);
cy.get("form").submit();
cy.get("#barcode").closest("form").submit();
// Should see error message about patron debt
cy.get(".dialog.alert").should("be.visible");
cy.get(".dialog.alert li").should(
"contain",
`The patron has a debt of`
@ -94,24 +92,22 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
);
cy.get('.dialog.alert form button[type="submit"].approve').should(
"contain",
"Renew checkout(s)"
"Override and renew"
);
});
it("should NOT show override button when AllowFineOverrideRenewing is disabled", function () {
// Ensure AllowFineOverrideRenewing is disabled
cy.task("query", {
sql: "UPDATE systempreferences SET value = '0' WHERE variable = 'AllowFineOverrideRenewing'",
});
cy.set_syspref("AllowFineOverrideRenewing", "0");
const barcode = this.testObjects.item.barcode;
const barcode = this.testObjects.items[0].external_id;
// Visit renewal page
cy.visit("/cgi-bin/koha/circ/renew.pl");
// Enter barcode
cy.get("#barcode").type(barcode);
cy.get("form").submit();
cy.get("#barcode").closest("form").submit();
// Should see error message about patron debt
cy.get(".dialog.alert").should("be.visible");
@ -128,18 +124,16 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
it("should allow renewal after override when AllowFineOverrideRenewing is enabled", function () {
// Enable AllowFineOverrideRenewing
cy.task("query", {
sql: "UPDATE systempreferences SET value = '1' WHERE variable = 'AllowFineOverrideRenewing'",
});
cy.set_syspref("AllowFineOverrideRenewing", "1");
const barcode = this.testObjects.item.barcode;
const barcode = this.testObjects.items[0].external_id;
// Visit renewal page
cy.visit("/cgi-bin/koha/circ/renew.pl");
// Enter barcode
cy.get("#barcode").type(barcode);
cy.get("form").submit();
cy.get("#barcode").closest("form").submit();
// Click override button
cy.get('.dialog.alert form button[type="submit"].approve').click();
@ -160,17 +154,17 @@ describe("Circulation - FineNoRenewals and AllowFineOverrideRenewing", () => {
cy.task("query", {
sql: `INSERT INTO accountlines (borrowernumber, amountoutstanding, debit_type_code, status, interface, itemnumber)
VALUES (?, 3.00, 'OVERDUE', 'UNRETURNED', 'test', ?)`,
values: [patron.patron_id, this.testObjects.item.item_id],
values: [patron.patron_id, this.testObjects.items[0].item_id],
});
const barcode = this.testObjects.item.barcode;
const barcode = this.testObjects.items[0].external_id;
// Visit renewal page
cy.visit("/cgi-bin/koha/circ/renew.pl");
// Enter barcode
cy.get("#barcode").type(barcode);
cy.get("form").submit();
cy.get("#barcode").closest("form").submit();
// Should see success message (no error about fines)
cy.get(".dialog.message").should("be.visible");