From c5ece276bc15717d0a8b700da819766edfc31353 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Mon, 1 Sep 2025 13:22:51 +0300 Subject: [PATCH] Bug 23415: Fix QA issues --- C4/Circulation.pm | 2 +- circ/renew.pl | 7 ++-- .../admin/preferences/circulation.pref | 2 +- .../prog/en/modules/circ/circulation.tt | 3 +- .../prog/en/modules/circ/renew.tt | 6 ++-- .../prog/en/modules/members/moremember.tt | 3 +- koha-tmpl/intranet-tmpl/prog/js/checkouts.js | 27 ++++++++++----- opac/opac-user.pl | 4 +-- t/db_dependent/Circulation.t | 33 +++++++++---------- 9 files changed, 47 insertions(+), 40 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 086bbada80f..04d8f447a77 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3253,7 +3253,7 @@ sub CanBookBeRenewed { { return ( 0, 'overdue' ); } elsif ( $finesblockrenewing && $balance > $finesblockrenewing && !$allowfineoverriderenewing ) { - return ( 0, 'too_much_oweing'); + return ( 0, 'too_much_oweing' ); } } diff --git a/circ/renew.pl b/circ/renew.pl index 5d8446776db..af41f311ae1 100755 --- a/circ/renew.pl +++ b/circ/renew.pl @@ -95,12 +95,11 @@ if ( $op eq 'cud-renew' && $barcode ) { ); } - if ( $error && ($error eq 'too_much_oweing' or $error eq 'auto_too_much_oweing') ) { + if ( $error && ( $error eq 'too_much_oweing' or $error eq 'auto_too_much_oweing' ) ) { if ($override_debt) { $can_renew = 1; - $error = undef; - } - else { + $error = undef; + } else { $balance = $patron->account->balance; $template->param( balance => $balance ); $can_renew = 0; 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 a6c739ea18b..06e9495a4a6 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 @@ -571,7 +571,7 @@ Circulation: - Only allow patrons or staff to renew items if patron has less than - pref: FineNoRenewals class: currency - - [% local_currency %] in fines (leave blank to disable). + - '[% local_currency %] in fines (leave blank to disable).' Lost item policy: - - By default, set the LOST value of an item to diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index 8b10a64f247..1a5c79f8ac0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -1134,7 +1134,8 @@ var amountlimit = "[% Koha.Preference('FineNoRenewals') | html %]"; var fines = "[% fines | $Price %]"; - var MSG_CONFIRM_RENEW = _("The patron has a debt of %s\nAre you sure you want to renew checkout(s)?").format(fines); + var MSG_CONFIRM_RENEW = _("The patron has a debt of %s").format(fines); + MSG_CONFIRM_RENEW += _("Are you sure you want to renew checkout(s)?"); // On-site checkout function toggle_onsite_checkout(){ diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt index 351e0a19c7e..46f7f6fe83c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt @@ -140,11 +140,12 @@ [% END %] [% ELSIF error == "too_much_oweing" %] -
  • The patron has a debt of [% balance | $Price %].
  • - + [% INCLUDE 'csrf-token.inc' %] + +
    [% ELSIF error == "on_reserve" %] @@ -173,7 +174,6 @@

    Item cannot be renewed because it's an onsite checkout

    [% ELSIF error == 'recalled' %]

    This item has been recalled.

    - [% ELSE %] [% error | html %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index d6b0ae44c51..6f59058988f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -815,7 +815,8 @@ var amountlimit = "[% Koha.Preference('FineNoRenewals') | html %]"; var fines = "[% fines | $Price %]"; - var MSG_CONFIRM_RENEW = _("The patron has a debt of %s\n Are you sure you want to renew checkout(s)?").format(fines); + var MSG_CONFIRM_RENEW = _("The patron has a debt of %s").format(fines); + MSG_CONFIRM_RENEW += _("Are you sure you want to renew checkout(s)?"); function uncheck_sibling(me){ nodename=me.getAttribute("name"); diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js index 8770d97c407..52de8b770a0 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js +++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js @@ -472,13 +472,12 @@ function LoadIssuesTable() { span_style = "display: none"; span_class = "renewals-allowed"; - } else if ( - oObj.can_renew_error == "too_much_oweing" - ) { + } else if (oObj.can_renew_error == "too_much_oweing") { msg += "" + __( - "Cannot renew, he patron has a debt of " + parseFloat(oObj.fine).format_price() + "Cannot renew, he patron has a debt of " + + parseFloat(oObj.fine).format_price() ) + ""; @@ -958,13 +957,13 @@ $(document).ready(function () { e.preventDefault(); let refresh_table = true; - if(show_confirm && parseFloat(fines) > parseFloat(amountlimit) ) { + if (show_confirm && parseFloat(fines) > parseFloat(amountlimit)) { var result = confirm(MSG_CONFIRM_RENEW); - if(!result){ + if (!result) { return false; } // Prevent displaying confirm box again - show_confirm = false; + show_confirm = false; } function renew(item_id) { @@ -1002,8 +1001,18 @@ $(document).ready(function () { params.date_due = dueDate; } - - $(this).parent().parent().replaceWith(""); + $(this) + .parent() + .parent() + .replaceWith( + "" + ); const client = APIClient.circulation; return client.checkouts.renew(params).then( diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 3f5958e3b0b..429374fa457 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -125,8 +125,8 @@ if ( $userdebarred || $borr->{'gonenoaddress'} || $borr->{'lost'} ) { } 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 $no_renewal_amt = C4::Context->preference('FineNoRenewals'); +$no_renewal_amt = undef unless looks_like_number($no_renewal_amt); my $amountoutstandingfornewal = C4::Context->preference("OPACFineNoRenewalsIncludeCredits") ? $amountoutstanding diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 8c7cc432639..ab60ab75b00 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -1352,9 +1352,9 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - C4::Context->set_preference('OPACFineNoRenewalsBlockAutoRenew','1'); - C4::Context->set_preference('FineNoRenewals','10'); - C4::Context->set_preference('OPACFineNoRenewalsIncludeCredit','1'); + C4::Context->set_preference( 'OPACFineNoRenewalsBlockAutoRenew', '1' ); + C4::Context->set_preference( 'FineNoRenewals', '10' ); + C4::Context->set_preference( 'OPACFineNoRenewalsIncludeCredit', '1' ); my $fines_amount = 5; my $account = Koha::Account->new( { patron_id => $renewing_borrowernumber } ); $account->add_debit( @@ -1366,10 +1366,9 @@ subtest "CanBookBeRenewed tests" => sub { description => "Some fines" } )->status('RETURNED')->store; - ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrower_obj, $issue ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 1, 'Renew, even if renewal is automatic' ); - is( $error, 'auto_renew', 'Can auto renew, FineNoRenewals=10, patron has 5' ); + is( $error, 'auto_renew', 'Can auto renew, FineNoRenewals=10, patron has 5' ); $account->add_debit( { @@ -1380,10 +1379,9 @@ subtest "CanBookBeRenewed tests" => sub { description => "Some fines" } )->status('RETURNED')->store; - ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrower_obj, $issue ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 1, 'Renew, even if renewal is automatic' ); - is( $error, 'auto_renew', 'Can auto renew, FineNoRenewals=10, patron has 10' ); + is( $error, 'auto_renew', 'Can auto renew, FineNoRenewals=10, patron has 10' ); $account->add_debit( { @@ -1394,10 +1392,9 @@ subtest "CanBookBeRenewed tests" => sub { description => "Some fines" } )->status('RETURNED')->store; - ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrower_obj, $issue ); - is( $renewokay, 0, 'Do not renew, renewal is automatic' ); - is( $error, 'auto_too_much_oweing', 'Cannot auto renew, FineNoRenewals=10, patron has 15' ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_too_much_oweing', 'Cannot auto renew, FineNoRenewals=10, patron has 15' ); $account->add_credit( { @@ -1824,7 +1821,7 @@ subtest "CanBookBeRenewed tests" => sub { "Account line description must not contain 'Lost Items ', but be title, barcode, itemcallnumber" ); - t::lib::Mocks::mock_preference('FineNoRenewals', 0); + t::lib::Mocks::mock_preference( 'FineNoRenewals', 0 ); # Recalls t::lib::Mocks::mock_preference( 'UseRecalls', 1 ); @@ -1904,10 +1901,10 @@ subtest "CanBookBeRenewed tests" => sub { $recall->set_cancelled; # Too much debt - t::lib::Mocks::mock_preference('FineNoRenewals', 1); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); - is( $renewokay, 0, 'Cannot renew, too much debt and FineNoRenewals=1' ); - is( $error, 'too_much_oweing', 'Cannot renew, debt not allowed (returned code is too_much_oweing)'); + t::lib::Mocks::mock_preference( 'FineNoRenewals', 1 ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_1 ); + is( $renewokay, 0, 'Cannot renew, too much debt and FineNoRenewals=1' ); + is( $error, 'too_much_oweing', 'Cannot renew, debt not allowed (returned code is too_much_oweing)' ); C4::Context->dbh->do("DELETE FROM accountlines"); }; -- 2.51.0