Bug 23415 - Notify patron fines when renewing
Summary: Notify patron fines when renewing
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Emmi Takkinen
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-02 07:42 UTC by Emmi Takkinen
Modified: 2025-10-31 13:13 UTC (History)
20 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 23415: Notify patron fines when renewing (10.21 KB, patch)
2019-08-07 12:31 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.04 KB, patch)
2019-09-03 10:25 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (10.94 KB, patch)
2020-02-05 10:42 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.12 KB, patch)
2020-02-10 07:10 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.00 KB, patch)
2020-02-14 17:54 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.15 KB, patch)
2020-02-14 17:54 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.68 KB, patch)
2020-06-16 06:08 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.57 KB, patch)
2020-06-16 06:09 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.59 KB, patch)
2020-08-17 11:29 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (10.87 KB, patch)
2020-09-11 06:08 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.15 KB, patch)
2020-09-11 06:09 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.67 KB, patch)
2020-09-11 06:09 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.59 KB, patch)
2020-09-11 06:10 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.25 KB, patch)
2021-02-16 11:43 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.15 KB, patch)
2021-02-16 11:44 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.74 KB, patch)
2021-02-16 11:44 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.59 KB, patch)
2021-02-16 11:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.71 KB, patch)
2021-03-12 07:06 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.15 KB, patch)
2021-03-12 07:06 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.74 KB, patch)
2021-03-12 07:07 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.59 KB, patch)
2021-03-12 07:07 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.64 KB, patch)
2021-03-12 10:21 UTC, Lund University Sandbox
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.75 KB, patch)
2021-04-07 14:08 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.20 KB, patch)
2021-04-07 14:08 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.78 KB, patch)
2021-04-07 14:08 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.69 KB, patch)
2021-04-07 14:08 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.74 KB, patch)
2021-05-28 09:27 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.79 KB, patch)
2022-08-23 11:56 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.33 KB, patch)
2022-08-23 11:57 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.91 KB, patch)
2022-08-23 11:58 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.70 KB, patch)
2022-08-23 11:58 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.98 KB, patch)
2022-08-23 11:59 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.87 KB, patch)
2023-02-09 07:24 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.85 KB, patch)
2023-03-15 11:11 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.38 KB, patch)
2023-03-15 11:11 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.96 KB, patch)
2023-03-15 11:11 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.76 KB, patch)
2023-03-15 11:11 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.93 KB, patch)
2023-03-15 11:11 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.67 KB, patch)
2023-06-28 08:20 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.32 KB, patch)
2023-06-28 08:20 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.67 KB, patch)
2023-06-28 08:21 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.70 KB, patch)
2023-06-28 08:21 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.85 KB, patch)
2023-06-28 08:22 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add fine formatting (1.50 KB, patch)
2023-06-28 08:22 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (11.72 KB, patch)
2023-08-18 08:16 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.37 KB, patch)
2023-08-18 08:16 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.72 KB, patch)
2023-08-18 08:16 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.75 KB, patch)
2023-08-18 08:16 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.90 KB, patch)
2023-08-18 08:16 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 23415: Add fine formatting (1.54 KB, patch)
2023-08-18 08:16 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.97 KB, patch)
2023-11-03 07:53 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.32 KB, patch)
2023-11-03 07:54 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.67 KB, patch)
2023-11-03 07:54 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.68 KB, patch)
2023-11-03 07:55 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.85 KB, patch)
2023-11-03 07:55 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add fine formatting (1.50 KB, patch)
2023-11-03 07:56 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.97 KB, patch)
2024-02-09 12:20 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.32 KB, patch)
2024-02-09 12:20 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (3.67 KB, patch)
2024-02-09 12:21 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.69 KB, patch)
2024-02-09 12:21 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (14.26 KB, patch)
2024-02-09 12:22 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add fine formatting (1.50 KB, patch)
2024-02-09 12:22 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (10.09 KB, patch)
2024-02-15 08:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add missing filters (2.39 KB, patch)
2024-02-15 08:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge (4.67 KB, patch)
2024-02-15 08:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Display confirm dialog only once (2.75 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (14.35 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add fine formatting (1.56 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.43 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (1.08 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Spelling and price formatting (2.89 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix rebase code re-introduction (3.85 KB, patch)
2024-02-15 08:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.54 KB, patch)
2024-09-17 13:11 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (14.26 KB, patch)
2024-09-17 13:11 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.43 KB, patch)
2024-09-17 13:12 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (1.10 KB, patch)
2024-09-17 13:12 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.59 KB, patch)
2024-09-26 15:22 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (14.31 KB, patch)
2024-09-26 15:22 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.48 KB, patch)
2024-09-26 15:22 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (1.15 KB, patch)
2024-09-26 15:22 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.67 KB, patch)
2024-10-17 16:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (14.39 KB, patch)
2024-10-17 16:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.55 KB, patch)
2024-10-17 16:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (1.22 KB, patch)
2024-10-17 16:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed (5.90 KB, patch)
2024-10-17 16:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.44 KB, patch)
2025-08-08 12:18 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.16 KB, patch)
2025-08-08 12:18 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.50 KB, patch)
2025-08-08 12:18 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (914 bytes, patch)
2025-08-08 12:19 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed (5.62 KB, patch)
2025-08-08 12:19 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Add new syspref AllowFineOverrideRenewing (8.07 KB, patch)
2025-08-08 12:20 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Fix QA issues (4.56 KB, patch)
2025-09-01 11:18 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.52 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.19 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.50 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (910 bytes, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed (5.68 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add new syspref AllowFineOverrideRenewing (8.08 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Fix QA issues (13.10 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix critical logic and security issues (9.20 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix spelling: oweing -> owing (14.89 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add Cypress test coverage for FineNoRenewals (8.36 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add API tests for fine override on renewal (4.84 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add REST API support for fine override on renewal (3.61 KB, patch)
2025-10-23 11:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.52 KB, patch)
2025-10-30 22:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.19 KB, patch)
2025-10-30 22:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.50 KB, patch)
2025-10-30 22:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (910 bytes, patch)
2025-10-30 22:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed (5.68 KB, patch)
2025-10-30 22:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add new syspref AllowFineOverrideRenewing (8.08 KB, patch)
2025-10-30 22:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Fix QA issues (13.10 KB, patch)
2025-10-30 22:14 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix critical logic and security issues (9.20 KB, patch)
2025-10-30 22:14 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix spelling: oweing -> owing (14.89 KB, patch)
2025-10-30 22:14 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.52 KB, patch)
2025-10-30 22:16 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.19 KB, patch)
2025-10-30 22:17 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.50 KB, patch)
2025-10-30 22:17 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Tidy template parameters block (910 bytes, patch)
2025-10-30 22:17 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Notify patron fines when renewing (9.52 KB, patch)
2025-10-31 06:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Rename OPACFineNoRenewals (15.48 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql (3.50 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed (5.68 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add new syspref AllowFineOverrideRenewing (8.14 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Fix QA issues (13.16 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix logic and security issues (9.18 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (QA follow-up) Fix spelling: oweing -> owing (14.96 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add Cypress test coverage for FineNoRenewals (8.41 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add API tests for fine override on renewal (4.89 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add REST API support for fine override on renewal (3.67 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Fix FineNoRenewals to use non_issues_charges and improve template (16.58 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: Add debt override support to circulation checkout page (16.76 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 23415: (follow-up) Respect OPACFineNoRenewalsIncludeCredits preference (3.63 KB, patch)
2025-10-31 06:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emmi Takkinen 2019-08-02 07:42:06 UTC
It is possible to renew items for patron who has fines over accepted limit on renew page or from checkout list. There should be alert to notify staff that patron has fines over limit and confirmation to renew.
Comment 1 Emmi Takkinen 2019-08-07 12:31:05 UTC Comment hidden (obsolete)
Comment 2 Emmi Takkinen 2019-09-03 10:25:14 UTC Comment hidden (obsolete)
Comment 3 Christopher Brannon 2019-10-04 18:15:27 UTC
I'm trying this on a sandbox, but I am not seeing any change in either renewal process with the patch applied.
Comment 4 Anneli Österman 2019-11-07 09:16:53 UTC
I also tried this patch on a sandbox and the alert was displayed on moremember.pl-page and renew.pl-page. It was not displayed on circulation.pl-page, where it should also appear.
Comment 5 Katrin Fischer 2019-11-24 13:57:44 UTC
Marking FQA because of Anneli's last comment. Can you please give feedback?
Comment 6 Anneli Österman 2020-02-05 09:46:19 UTC
I tested this again using a sandbox with this bug number (using master) and it worked perfectly well on all three pages.
Comment 7 Katrin Fischer 2020-02-05 10:25:37 UTC
Thx anneli, do you want to add your sign off?
Comment 8 ByWater Sandboxes 2020-02-05 10:42:12 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2020-02-09 12:43:51 UTC
Hi Emmi, there are some fails with the QA tools for missing filters, can you please fix?

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
   FAIL	  filters
		missing_filter at line 1071 (        var amountlimit = "[% Koha.Preference('noissuescharge') %]";)
		missing_filter at line 1072 (        var fines = "[% fines %]";)

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
   FAIL	  filters
		missing_filter at line 890 (        var amountlimit = "[% Koha.Preference('noissuescharge') %]";)
		missing_filter at line 891 (        var fines = "[% fines %]";)
Comment 10 Emmi Takkinen 2020-02-10 07:10:29 UTC Comment hidden (obsolete)
Comment 11 ByWater Sandboxes 2020-02-14 17:54:35 UTC Comment hidden (obsolete)
Comment 12 ByWater Sandboxes 2020-02-14 17:54:37 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2020-03-22 17:36:01 UTC
I am wondering if noissuecharge is the right value here.

We got: 
OPACFineNoRenewals = fine limit for renewals in the OPAC, but nothing for staff specifically.

noissuecharge = fine limit for checkouts (as name says). It's used in this patch set, but there are other complications with this one:

noissuescharge 	
Prevent patrons from checking out books if they have more than EUR in fines.

HoldsInNoissuesCharge 	
[Don't include|Include] hold charges when summing up charges for noissuescharge.
ManInvInNoissuesCharge 	
[Don't include|Include] custom debit type charges when summing up charges for noissuescharge.
RentalsInNoissuesCharge 	
[Don't include|Include]  rental charges when summing up charges for noissuescharge. 
NoIssuesChargeGuarantees 	
Prevent a patron from checking out if the patron has guarantees owing in total more than EUR in fines.

So I think this is not covering it:

+  my $amountlimit = C4::Context->preference("noissuescharge");
+  var fines = "[% fines | html %]

We'd need the amount calculated taking all noissues* prefs into account.

I think I'd prefer not using noissuecharge here, as it only speaks of checkouts and we have a distinct one for renewals. Using OPACFineNoRenewals would make sense to me, as you can still override in the staff interface. So having a 'hint' there would work.


Also a little worried that this adds a confirm step - but as none of the testers complained, I think it's probably ok :)
Comment 14 Emmi Takkinen 2020-06-16 06:08:58 UTC Comment hidden (obsolete)
Comment 15 Emmi Takkinen 2020-06-16 06:09:51 UTC Comment hidden (obsolete)
Comment 16 Emmi Takkinen 2020-08-17 11:29:08 UTC Comment hidden (obsolete)
Comment 17 Emmi Takkinen 2020-09-11 06:08:30 UTC Comment hidden (obsolete)
Comment 18 Emmi Takkinen 2020-09-11 06:09:11 UTC Comment hidden (obsolete)
Comment 19 Emmi Takkinen 2020-09-11 06:09:55 UTC Comment hidden (obsolete)
Comment 20 Emmi Takkinen 2020-09-11 06:10:39 UTC Comment hidden (obsolete)
Comment 21 Emmi Takkinen 2020-09-11 06:11:11 UTC
Rebased patches against current master.
Comment 22 Victoria Faafia 2021-01-20 00:59:37 UTC
Applying: Bug 23415: Notify patron fines when renewing
Using index info to reconstruct a base tree...
M	circ/circulation.pl
M	circ/renew.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
M	koha-tmpl/intranet-tmpl/prog/js/checkouts.js
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/js/checkouts.js
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/checkouts.js
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
Auto-merging circ/renew.pl
Auto-merging circ/circulation.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23415: Notify patron fines when renewing
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-23415-Notify-patron-fines-when-renewing-AHlD9r.patch
Comment 23 Emmi Takkinen 2021-02-16 11:43:19 UTC Comment hidden (obsolete)
Comment 24 Emmi Takkinen 2021-02-16 11:44:02 UTC Comment hidden (obsolete)
Comment 25 Emmi Takkinen 2021-02-16 11:44:41 UTC Comment hidden (obsolete)
Comment 26 Emmi Takkinen 2021-02-16 11:45:22 UTC Comment hidden (obsolete)
Comment 27 Stina Hallin 2021-03-11 13:19:21 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/checkouts.js
Comment 28 Emmi Takkinen 2021-03-12 07:06:12 UTC Comment hidden (obsolete)
Comment 29 Emmi Takkinen 2021-03-12 07:06:41 UTC Comment hidden (obsolete)
Comment 30 Emmi Takkinen 2021-03-12 07:07:16 UTC Comment hidden (obsolete)
Comment 31 Emmi Takkinen 2021-03-12 07:07:48 UTC Comment hidden (obsolete)
Comment 32 Lund University Sandbox 2021-03-12 10:21:34 UTC Comment hidden (obsolete)
Comment 33 Peter Vashchuk 2021-04-07 14:08:03 UTC Comment hidden (obsolete)
Comment 34 Peter Vashchuk 2021-04-07 14:08:08 UTC Comment hidden (obsolete)
Comment 35 Peter Vashchuk 2021-04-07 14:08:13 UTC Comment hidden (obsolete)
Comment 36 Peter Vashchuk 2021-04-07 14:08:18 UTC Comment hidden (obsolete)
Comment 37 Kyle M Hall (khall) 2021-05-07 17:00:56 UTC
I think OPACFineNoRenewals needs to be renamed to FineNoRenewals or something along those lines to prevent confusion in the future.
Comment 38 Emmi Takkinen 2021-05-11 10:14:00 UTC
(In reply to Kyle M Hall from comment #37)
> I think OPACFineNoRenewals needs to be renamed to FineNoRenewals or
> something along those lines to prevent confusion in the future.

Makes sense...maybe it should also be moved from OPAC preferences to Circulation preferences.
Comment 39 Emmi Takkinen 2021-05-28 09:27:11 UTC Comment hidden (obsolete)
Comment 40 Danyon Sewell 2022-07-07 23:58:36 UTC
Patch doesn't apply. Please rebase.
Comment 41 Emmi Takkinen 2022-08-23 11:56:58 UTC Comment hidden (obsolete)
Comment 42 Emmi Takkinen 2022-08-23 11:57:32 UTC Comment hidden (obsolete)
Comment 43 Emmi Takkinen 2022-08-23 11:58:02 UTC Comment hidden (obsolete)
Comment 44 Emmi Takkinen 2022-08-23 11:58:37 UTC Comment hidden (obsolete)
Comment 45 Emmi Takkinen 2022-08-23 11:59:08 UTC Comment hidden (obsolete)
Comment 46 Laura Escamilla 2023-02-08 13:07:35 UTC
The test plan still works but OPACFineNoRenewals is still showing as OPACFineNoRenewals and not FineNoRenewals.



(In reply to Emmi Takkinen from comment #45)
> Created attachment 139674 [details] [review] [review]
> Bug 23415: Rename OPACFineNoRenewals
> 
> OPACFineNoRenewals no longer applies just to OPAC.
> To avoid confusion rename it as FineNoRenewals and
> also rename all occurences in code.
> 
> To test, confirm everything works as described in
> previous test plans.
> 
> Also prove t/db_dependent/Circulation.t
Comment 47 Emmi Takkinen 2023-02-09 07:24:29 UTC Comment hidden (obsolete)
Comment 48 Emmi Takkinen 2023-02-09 07:27:59 UTC
(In reply to Laura Escamilla from comment #46)
> The test plan still works but OPACFineNoRenewals is still showing as
> OPACFineNoRenewals and not FineNoRenewals.
> 
> 
> 
> (In reply to Emmi Takkinen from comment #45)
> > Created attachment 139674 [details] [review] [review] [review]
> > Bug 23415: Rename OPACFineNoRenewals
> > 
> > OPACFineNoRenewals no longer applies just to OPAC.
> > To avoid confusion rename it as FineNoRenewals and
> > also rename all occurences in code.
> > 
> > To test, confirm everything works as described in
> > previous test plans.
> > 
> > Also prove t/db_dependent/Circulation.t

I rebased patch and added new atomicupdate file. Could it be that patch didn't previously apply? Was syspref still named as OPACFineNoRenewals in systempreference page or was it still OPACFineNoRenewals in database and code?
Comment 49 ByWater Sandboxes 2023-03-15 11:11:45 UTC Comment hidden (obsolete)
Comment 50 ByWater Sandboxes 2023-03-15 11:11:47 UTC Comment hidden (obsolete)
Comment 51 ByWater Sandboxes 2023-03-15 11:11:50 UTC Comment hidden (obsolete)
Comment 52 ByWater Sandboxes 2023-03-15 11:11:52 UTC Comment hidden (obsolete)
Comment 53 ByWater Sandboxes 2023-03-15 11:11:54 UTC Comment hidden (obsolete)
Comment 54 Sally 2023-03-15 12:01:20 UTC
Works as described; it shows for me as FineNoRenewals.  I've signed off because all of the functionality is fine. 

There were a couple of formatting issues around the currency in the pop-up box:  

1)  The trailing full stop / period feels a bit odd when it's currency:
Shows:  The patron has a debt of 3.21.
Perhaps:  The patron has a debt of 3.21

2)  If it's a round number, then pennies/cents aren't shown - it looks a bit odd:
Shows:  The patron has a debt of 3.
Perhaps:  The patron has a debt of 3.00

3)  Is there any scope for adding the appropriate currency symbol?
Shows:  The patron has a debt of 3.21
Perhaps:  The patron has a debt of £3.21
And:  The patron has a debt of $3.21
And:  The patron has a debt of €3.21 

The other query was system preferences.

OPACFineNoRenewals has been renamed to FineNoRenewals.
There are two other system preferences:  OPACFineNoRenewalsBlockAutoRenew and OPACFineNoRenewalsIncludeCredits

The descriptions (values) of these have been updated by this bug to refer to FineNoRenewals, but should we also consider renaming the system preferences themselves to:  FineNoRenewalsBlockAutoRenew and  FineNoRenewalsIncludeCredits ?  

I realise this last point might be out of scope of the bug.
Comment 55 Katrin Fischer 2023-04-16 01:40:19 UTC
Can you please rebase?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 23415: Notify patron fines when renewing
Applying: Bug 23415: Add missing filters
Applying: Bug 23415: Use OPACFineNoRenewals instead of noissuecharge
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 23415: Use OPACFineNoRenewals instead of noissuecharge
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-23415-Use-OPACFineNoRenewals-instead-of-noissu-dlItcl.patch

Please also check the fine formatting as noted by Sally in comment#54. There is now also a way to format in JS afaik.
Comment 56 Emmi Takkinen 2023-06-28 08:20:28 UTC Comment hidden (obsolete)
Comment 57 Emmi Takkinen 2023-06-28 08:20:56 UTC Comment hidden (obsolete)
Comment 58 Emmi Takkinen 2023-06-28 08:21:28 UTC Comment hidden (obsolete)
Comment 59 Emmi Takkinen 2023-06-28 08:21:54 UTC Comment hidden (obsolete)
Comment 60 Emmi Takkinen 2023-06-28 08:22:33 UTC Comment hidden (obsolete)
Comment 61 Emmi Takkinen 2023-06-28 08:22:57 UTC Comment hidden (obsolete)
Comment 62 Emmi Takkinen 2023-06-28 08:29:55 UTC
Rebased patches.  

(In reply to Sally from comment #54)
> Works as described; it shows for me as FineNoRenewals.  I've signed off
> because all of the functionality is fine. 
> 
> There were a couple of formatting issues around the currency in the pop-up
> box:  
> 
> 1)  The trailing full stop / period feels a bit odd when it's currency:
> Shows:  The patron has a debt of 3.21.
> Perhaps:  The patron has a debt of 3.21
> 
> 2)  If it's a round number, then pennies/cents aren't shown - it looks a bit
> odd:
> Shows:  The patron has a debt of 3.
> Perhaps:  The patron has a debt of 3.00
> 
These have been added in new patch.

> 3)  Is there any scope for adding the appropriate currency symbol?
> Shows:  The patron has a debt of 3.21
> Perhaps:  The patron has a debt of £3.21
> And:  The patron has a debt of $3.21
> And:  The patron has a debt of €3.21 
> 
This isn't currently possible so I left this out.

> The other query was system preferences.
> 
> OPACFineNoRenewals has been renamed to FineNoRenewals.
> There are two other system preferences:  OPACFineNoRenewalsBlockAutoRenew
> and OPACFineNoRenewalsIncludeCredits
> 
> The descriptions (values) of these have been updated by this bug to refer to
> FineNoRenewals, but should we also consider renaming the system preferences
> themselves to:  FineNoRenewalsBlockAutoRenew and 
> FineNoRenewalsIncludeCredits ?  
> 
> I realise this last point might be out of scope of the bug.
Hmm, I'm not sure if these are still used just in OPAC.
Comment 63 Biblibre Sandboxes 2023-08-18 08:16:13 UTC Comment hidden (obsolete)
Comment 64 Biblibre Sandboxes 2023-08-18 08:16:16 UTC Comment hidden (obsolete)
Comment 65 Biblibre Sandboxes 2023-08-18 08:16:20 UTC Comment hidden (obsolete)
Comment 66 Biblibre Sandboxes 2023-08-18 08:16:23 UTC Comment hidden (obsolete)
Comment 67 Biblibre Sandboxes 2023-08-18 08:16:26 UTC Comment hidden (obsolete)
Comment 68 Biblibre Sandboxes 2023-08-18 08:16:29 UTC Comment hidden (obsolete)
Comment 69 Veli-Matti Häkkinen 2023-08-18 08:17:37 UTC
Tested and works as planned.
Comment 70 Martin Renvoize (ashimema) 2023-10-23 07:36:24 UTC
Sorry guys, came to QA but we need a rebase first.
Comment 71 Emmi Takkinen 2023-11-03 07:53:51 UTC Comment hidden (obsolete)
Comment 72 Emmi Takkinen 2023-11-03 07:54:19 UTC Comment hidden (obsolete)
Comment 73 Emmi Takkinen 2023-11-03 07:54:59 UTC Comment hidden (obsolete)
Comment 74 Emmi Takkinen 2023-11-03 07:55:25 UTC Comment hidden (obsolete)
Comment 75 Emmi Takkinen 2023-11-03 07:55:53 UTC Comment hidden (obsolete)
Comment 76 Emmi Takkinen 2023-11-03 07:56:15 UTC Comment hidden (obsolete)
Comment 77 Emmi Takkinen 2023-11-03 07:56:56 UTC
Rebased patches. Setting this back to "Signed Off".
Comment 78 Kyle M Hall (khall) 2024-02-09 11:52:15 UTC
Applying: Bug 23415: Rename OPACFineNoRenewals
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref).
error: could not build fake ancestor
Patch failed at 0001 Bug 23415: Rename OPACFineNoRenewals
Comment 79 Emmi Takkinen 2024-02-09 12:20:22 UTC Comment hidden (obsolete)
Comment 80 Emmi Takkinen 2024-02-09 12:20:48 UTC Comment hidden (obsolete)
Comment 81 Emmi Takkinen 2024-02-09 12:21:31 UTC Comment hidden (obsolete)
Comment 82 Emmi Takkinen 2024-02-09 12:21:55 UTC Comment hidden (obsolete)
Comment 83 Emmi Takkinen 2024-02-09 12:22:18 UTC Comment hidden (obsolete)
Comment 84 Emmi Takkinen 2024-02-09 12:22:41 UTC Comment hidden (obsolete)
Comment 85 Emmi Takkinen 2024-02-09 12:22:55 UTC
Rebased patches.
Comment 86 Martin Renvoize (ashimema) 2024-02-15 08:03:48 UTC Comment hidden (obsolete)
Comment 87 Martin Renvoize (ashimema) 2024-02-15 08:03:53 UTC Comment hidden (obsolete)
Comment 88 Martin Renvoize (ashimema) 2024-02-15 08:03:56 UTC Comment hidden (obsolete)
Comment 89 Martin Renvoize (ashimema) 2024-02-15 08:04:00 UTC Comment hidden (obsolete)
Comment 90 Martin Renvoize (ashimema) 2024-02-15 08:04:05 UTC Comment hidden (obsolete)
Comment 91 Martin Renvoize (ashimema) 2024-02-15 08:04:08 UTC Comment hidden (obsolete)
Comment 92 Martin Renvoize (ashimema) 2024-02-15 08:04:12 UTC Comment hidden (obsolete)
Comment 93 Martin Renvoize (ashimema) 2024-02-15 08:04:16 UTC Comment hidden (obsolete)
Comment 94 Martin Renvoize (ashimema) 2024-02-15 08:04:20 UTC Comment hidden (obsolete)
Comment 95 Martin Renvoize (ashimema) 2024-02-15 08:04:24 UTC
Created attachment 162180 [details] [review]
Bug 23415: (QA follow-up) Fix rebase code re-introduction

Looks like some code was re-introduced during rebases, this patch
switches it back to calling the newly introduced 'renew' function rather
than doing the post inline. I also compared the two posts and updated
the function to match the updated post from upstream.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Martin Renvoize (ashimema) 2024-02-15 08:07:39 UTC
This is really close.. but.. May I ask why you chose to handle this at the controller level rather than adding/moving the check into CanBookBeRenewed?

I see you do handle it in the _CanBookBeAutoRenewed version, but I think I am confused that it's not there in parent function.
Comment 97 Emmi Takkinen 2024-02-21 06:59:41 UTC
(In reply to Martin Renvoize from comment #96)
> This is really close.. but.. May I ask why you chose to handle this at the
> controller level rather than adding/moving the check into CanBookBeRenewed?
> 
> I see you do handle it in the _CanBookBeAutoRenewed version, but I think I
> am confused that it's not there in parent function.

Hmm, do you mean why there is a line my 
>$fine_no_renewals = C4::Context->preference("FineNoRenewals"); 
(and everything after that line) in _CanBookBeAutoRenewed? Those have been added in different bug, I just renamed syspref from "OPACFineNoRenewals" -> "FineNoRenewals".
Comment 98 Martin Renvoize (ashimema) 2024-06-25 09:57:44 UTC
I'm afraid this needs a rebase
Comment 99 Emmi Takkinen 2024-07-08 11:02:52 UTC
It seems that last patch is partially dated.
Comment 100 Lucy Vaux-Harvey 2024-09-16 11:24:16 UTC
It would be great to keep this moving along as so much work has been done already. I currently have a library asking for this functionality.
Comment 101 Emmi Takkinen 2024-09-17 07:26:35 UTC
Code, especially in checkouts.js, has changed so much since last rebase that it's kinda tricky to sort out where everything goes. So this probably needs a lot of rework. I'll probably need to provide whole new patches for this since I don't see a point to just rebase these patches and then creating a new patch to address changes made to the code.
Comment 102 Katrin Fischer 2024-09-17 07:54:58 UTC
I am sorry to hear this Emmi, I hope we didn't make it harder.
Comment 103 Emmi Takkinen 2024-09-17 13:11:07 UTC
Created attachment 171620 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Comment 104 Emmi Takkinen 2024-09-17 13:11:38 UTC
Created attachment 171621 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 105 Emmi Takkinen 2024-09-17 13:12:11 UTC
Created attachment 171622 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 106 Emmi Takkinen 2024-09-17 13:12:41 UTC
Created attachment 171623 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 107 Emmi Takkinen 2024-09-17 13:19:52 UTC
(In reply to Katrin Fischer from comment #102)
> I am sorry to hear this Emmi, I hope we didn't make it harder.

It actually wasn't as bad as I feared it would be :D

I fixed and squashed some patches together, so that this is easier to maintain. However I kept the changes made to OPACFineNoRenewals syspref separate from other changes (again, changes are easier to maintain this way imo). This could need a new test round and sign offs.
Comment 108 Lucy Vaux-Harvey 2024-09-17 14:44:26 UTC
Thank you for looking at this again Emmi and all.
Comment 109 Shi Yao Wang 2024-09-26 15:22:41 UTC
Created attachment 171983 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 110 Shi Yao Wang 2024-09-26 15:22:44 UTC
Created attachment 171984 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 111 Shi Yao Wang 2024-09-26 15:22:48 UTC
Created attachment 171985 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 112 Shi Yao Wang 2024-09-26 15:22:51 UTC
Created attachment 171986 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 113 Martin Renvoize (ashimema) 2024-10-17 16:07:27 UTC
Created attachment 172906 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Martin Renvoize (ashimema) 2024-10-17 16:07:31 UTC
Created attachment 172907 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 115 Martin Renvoize (ashimema) 2024-10-17 16:07:35 UTC
Created attachment 172908 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 116 Martin Renvoize (ashimema) 2024-10-17 16:07:39 UTC
Created attachment 172909 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 117 Martin Renvoize (ashimema) 2024-10-17 16:07:43 UTC
Created attachment 172910 [details] [review]
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed

This moves the basic calculation for too_much_oweing out of the
controller and into the class method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 118 Martin Renvoize (ashimema) 2024-10-17 16:08:26 UTC
Sorry it took so long to come back to this.. all looking good now.

Passing QA
Comment 119 Katrin Fischer 2024-10-24 14:21:21 UTC
1) Translations (follow-up please)

+        var MSG_CONFIRM_RENEW = _("The patron has a debt of %s\nAre you sure you want to renew checkout(s)?").format(fines);

The \n is likely to cause issues in translations (haven't checked with Weblate, but was definitely an issue with Pootle. Please split into separate strings.

2) too_much_oweing (question, not blocker)

Wondering if that should be "owing"? If so, we could fix it globally on a separate bug.
Comment 120 Katrin Fischer 2024-10-24 14:50:58 UTC
Sorry, I was about to push this, but it needs some more work:

3) xt/find-missing-op-in-forms.t 
1..1
not ok 1 - The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478)
#   Failed test 'The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478)'
#   at xt/find-missing-op-in-forms.t line 43.
#          got: '1'
#     expected: '0'
# $VAR1 = 'koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt:193';
# Looks like you failed 1 test of 1.

4) xt/find-missing-csrf.t

5) xt/yaml_valid.t
Comment 121 Emmi Takkinen 2024-10-25 09:33:04 UTC
After latest patch, this no longer works as intended. If patron has fines over FineNoRenewals limit, renewing loans is prevented with message "too_much_oweing". Clicking "Renew all" displays pop up, but clicking ok just displays "There are no items to be renewed." pop up. 

So this got me thinking, which way is more preferable? Either we allow renewing items and overwrite FineNoRenewals limit like these patches did previously. Or we don't allow overwriting that limit and prevent it with error "too_much_oweing".
Comment 122 Katrin Fischer 2024-10-25 09:43:52 UTC
I have to admit I got a bit confused about the before and after, but could something like AllowFineOverride be helpful here?
Or a confirmation with an option to override.
Comment 123 Emmi Takkinen 2024-10-25 10:47:04 UTC
(In reply to Katrin Fischer from comment #122)
> I have to admit I got a bit confused about the before and after, but could
> something like AllowFineOverride be helpful here?
> Or a confirmation with an option to override.

Hmm, AllowFineOverride would probably be the way to go here. But do we then need to change its description to something like this:

"..staff to manually override and check out items or renew holds to patrons who have more in fines than set in the noissuescharge or FineNoRenewals system preference." 

Of course there's also possibility that some libraries want to override check out limit, but not holds limit.
Comment 124 Katrin Fischer 2024-10-25 11:12:12 UTC
We could do another pref... but before considering that it would maybe be good to get some libraries opinions on the use case and if that is really required.
Comment 125 Lucy Vaux-Harvey 2024-10-29 11:40:24 UTC
Hi, the library that I am currently working with would need the ability for staff to override from the alert message. I think it might need to be a separate system preference in order to give libraries flexibility over which transactions could be overridden.
Comment 126 Martin Renvoize (ashimema) 2025-01-07 14:30:18 UTC
Do you think you'll be able to get back to this one Emmi?
Comment 127 Emmi Takkinen 2025-01-20 06:13:14 UTC
(In reply to Martin Renvoize (ashimema) from comment #126)
> Do you think you'll be able to get back to this one Emmi?

I'm not currently able to work with this. Feel free to take this one if you (or someone else) have time :)
Comment 128 Katrin Fischer 2025-04-25 18:16:13 UTC
(In reply to Emmi Takkinen from comment #127)
> (In reply to Martin Renvoize (ashimema) from comment #126)
> > Do you think you'll be able to get back to this one Emmi?
> 
> I'm not currently able to work with this. Feel free to take this one if you
> (or someone else) have time :)

Sadly, this wasn't adopted and hasn't moved forward in the last month. Removing rel_25_05_candidate as we will use this now to track bugs still eligible for this version past soft feature freeze (depending on size etc.)

Emmi, I don't want to remove you as Assignee if you want to come back in future, please decide yourself how you want to handle it!
Comment 129 Martin Renvoize (ashimema) 2025-05-08 16:07:47 UTC
I would be lovely to get this moving again.. but I'm not sure I can commit any more time here either :(
Comment 130 Emmi Takkinen 2025-08-08 12:18:04 UTC
Created attachment 185270 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 131 Emmi Takkinen 2025-08-08 12:18:28 UTC
Created attachment 185271 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 132 Emmi Takkinen 2025-08-08 12:18:55 UTC
Created attachment 185272 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 133 Emmi Takkinen 2025-08-08 12:19:16 UTC
Created attachment 185273 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 134 Emmi Takkinen 2025-08-08 12:19:43 UTC
Created attachment 185274 [details] [review]
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed

This moves the basic calculation for too_much_oweing out of the
controller and into the class method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 135 Emmi Takkinen 2025-08-08 12:20:03 UTC
Created attachment 185275 [details] [review]
Bug 23415: Add new syspref AllowFineOverrideRenewing

This patch adds new syspref AllowFineOverrideRenewing. It allows
staff to renew items for patrons whose fines are over limit set
in syspref FineNoRenewals. Otherwise renewing is prevented. If
renewing items is allowed staff is still required to confirm
if they really want to renew items for the patron.

To test:
1. Apply this patch, update database and restart all services.
2. Find patron with fines over limit set in FineNoRenewals and
renewable items.
=> Note that renewing is prevented with message "Cannot renew,
he patron has a debt of..."
3. Set syspref AllowFineOverrideRenewing as "Allow".
=> Note that items can now be renewed and renewing them displays
confirmation pop-up.

Sponsored-by: Koha-Suomi Oy
Comment 136 Emmi Takkinen 2025-08-08 12:23:27 UTC
Rebased patches and added a new one with a proposal of a new syspref. I didn't yet fix issues Katrin pointed in her comments, will do that after returning from vacation. I'll set this as Needs Signoff to get some feed back, but note that this isn't ready to be pushed to main yet.
Comment 137 Emmi Takkinen 2025-09-01 09:45:15 UTC
(In reply to Katrin Fischer from comment #119)
> 2) too_much_oweing (question, not blocker)
> 
> Wondering if that should be "owing"? If so, we could fix it globally on a
> separate bug.

It should be owing imo. And yes, I'd rather left them as they are for now and fix them in a separate bug.
Comment 138 Emmi Takkinen 2025-09-01 11:18:30 UTC
Created attachment 186014 [details] [review]
Bug 23415: Fix QA issues
Comment 139 Emmi Takkinen 2025-09-01 11:19:28 UTC
Fixed QA issues, qa-tool and tests under xt should now be happy so this is ready for testing.
Comment 140 Martin Renvoize (ashimema) 2025-10-23 09:52:39 UTC
Sorry Emmi, I hit sha1 issues trying to apply this one.
Comment 141 Martin Renvoize (ashimema) 2025-10-23 09:59:00 UTC
Worked around it
Comment 142 Martin Renvoize (ashimema) 2025-10-23 11:25:09 UTC
Created attachment 188331 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 143 Martin Renvoize (ashimema) 2025-10-23 11:25:12 UTC
Created attachment 188332 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 144 Martin Renvoize (ashimema) 2025-10-23 11:25:15 UTC
Created attachment 188333 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 145 Martin Renvoize (ashimema) 2025-10-23 11:25:19 UTC
Created attachment 188334 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 146 Martin Renvoize (ashimema) 2025-10-23 11:25:22 UTC
Created attachment 188335 [details] [review]
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed

This moves the basic calculation for too_much_oweing out of the
controller and into the class method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 147 Martin Renvoize (ashimema) 2025-10-23 11:25:26 UTC
Created attachment 188336 [details] [review]
Bug 23415: Add new syspref AllowFineOverrideRenewing

This patch adds new syspref AllowFineOverrideRenewing. It allows
staff to renew items for patrons whose fines are over limit set
in syspref FineNoRenewals. Otherwise renewing is prevented. If
renewing items is allowed staff is still required to confirm
if they really want to renew items for the patron.

To test:
1. Apply this patch, update database and restart all services.
2. Find patron with fines over limit set in FineNoRenewals and
renewable items.
=> Note that renewing is prevented with message "Cannot renew,
he patron has a debt of..."
3. Set syspref AllowFineOverrideRenewing as "Allow".
=> Note that items can now be renewed and renewing them displays
confirmation pop-up.

Sponsored-by: Koha-Suomi Oy
Comment 148 Martin Renvoize (ashimema) 2025-10-23 11:25:29 UTC
Created attachment 188337 [details] [review]
Bug 23415: Fix QA issues
Comment 149 Martin Renvoize (ashimema) 2025-10-23 11:25:33 UTC
Created attachment 188338 [details] [review]
Bug 23415: (QA follow-up) Fix critical logic and security issues

This commit addresses several critical issues found in QA review:

1. Fixed critical logic bug in C4::Circulation::CanBookBeRenewed
   - Removed incorrect check of AllowFineOverrideRenewing preference
   - The function now always returns 'too_much_oweing' error when
     patron balance exceeds FineNoRenewals limit
   - AllowFineOverrideRenewing should only control UI override
     capability, not the core renewal check

2. Added permission check in circ/renew.pl
   - Override is now only allowed when both override_debt parameter
     is set AND AllowFineOverrideRenewing preference is enabled
   - Prevents security issue where staff could bypass the preference
     by crafting POST requests

3. Fixed template to conditionally display override button
   - Override button in renew.tt now only shows when
     AllowFineOverrideRenewing is enabled
   - Prevents confusion when override is not permitted

4. Added comprehensive test coverage
   - Tests verify CanBookBeRenewed behavior with AllowFineOverrideRenewing
     both enabled and disabled
   - Confirms error is always returned regardless of preference setting
   - Increased test count from 116 to 120

5. Fixed minor issues
   - Fixed typo: "he patron" -> "the patron" in checkouts.js
   - Fixed typo: OPACFineNoRenewalsIncludeCredit ->
     OPACFineNoRenewalsIncludeCredits in test

All tests pass successfully.
Comment 150 Martin Renvoize (ashimema) 2025-10-23 11:25:36 UTC
Created attachment 188339 [details] [review]
Bug 23415: (QA follow-up) Fix spelling: oweing -> owing

Standardizes error code spelling across the codebase:
- Changes 'too_much_oweing' to 'too_much_owing'
- Changes 'auto_too_much_oweing' to 'auto_too_much_owing'

The word "owing" is the correct spelling in English.

Files updated:
- C4/Circulation.pm
- C4/SIP/Sip/MsgType.pm
- circ/renew.pl
- koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
- koha-tmpl/intranet-tmpl/prog/js/checkouts.js
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
- misc/cronjobs/automatic_renewals.pl
- opac/opac-reserve.pl
- opac/opac-user.pl
- t/db_dependent/Circulation.t

All tests pass successfully.
Comment 151 Martin Renvoize (ashimema) 2025-10-23 11:25:40 UTC
Created attachment 188340 [details] [review]
Bug 23415: Add Cypress test coverage for FineNoRenewals

This commit adds comprehensive end-to-end tests for the fine-based
renewal blocking functionality using Cypress.

Test coverage includes:
1. Blocking renewal when patron fines exceed FineNoRenewals limit
2. Displaying override button when AllowFineOverrideRenewing is enabled
3. Hiding override button when AllowFineOverrideRenewing is disabled
4. Successful override when permission is enabled
5. Allowing renewal when fines are below the limit

The test creates test data including:
- A patron with checkouts
- Account fines that exceed the FineNoRenewals threshold
- Tests both blocking and override scenarios

Tests verify the UI behavior matches the business logic:
- Error messages are displayed correctly
- Override buttons appear based on preference setting
- Override functionality works as expected
- Security is maintained (no override without permission)

To run these tests:
  npx cypress run --spec "t/cypress/integration/Circulation/FineNoRenewals_spec.ts"
Comment 152 Martin Renvoize (ashimema) 2025-10-23 11:25:44 UTC
Created attachment 188341 [details] [review]
Bug 23415: Add API tests for fine override on renewal

This commit adds comprehensive REST API tests to verify the
x-koha-override: debt_limit functionality for renewals.

Tests cover:
- Renewal blocked due to excessive fines
- Override fails when AllowFineOverrideRenewing is disabled
- Override succeeds when AllowFineOverrideRenewing is enabled
- Both /renewal and /renewals endpoints

Test plan:
1. Run: prove t/db_dependent/api/v1/checkouts.t
2. All 110 tests should pass
Comment 153 Martin Renvoize (ashimema) 2025-10-23 11:25:47 UTC
Created attachment 188342 [details] [review]
Bug 23415: Add REST API support for fine override on renewal

This commit adds support for the AllowFineOverrideRenewing functionality
to the REST API renewal endpoints using the standard x-koha-override pattern.

Changes:
1. Added x-koha-override header parameter to both renewal endpoints:
   - POST /checkouts/{checkout_id}/renewal
   - POST /checkouts/{checkout_id}/renewals

2. Updated Koha::REST::V1::Checkouts::renew() to:
   - Check for 'debt_limit' in the overrides stash
   - Check AllowFineOverrideRenewing preference when override is requested
   - Allow renewal override only when both:
     * x-koha-override header contains 'debt_limit' AND
     * AllowFineOverrideRenewing preference is enabled
   - Follows the established x-koha-override pattern used by other endpoints

3. Updated OpenAPI specification (swagger) to document the new header
   using the standard pattern with enum values

Usage:
  curl -X POST /api/v1/checkouts/123/renewal \
    -H "x-koha-override: debt_limit"

The override only works for too_much_owing errors and requires
the AllowFineOverrideRenewing system preference to be enabled.

This implementation follows the same pattern as holds and other
endpoints that support overrides, maintaining consistency across
the REST API.
Comment 154 Martin Renvoize (ashimema) 2025-10-23 11:28:17 UTC Comment hidden (obsolete)
Comment 155 David Nind 2025-10-25 02:52:55 UTC
I attempted to test, but have Failed QA because:

1. Running the cypress tests failed: npx cypress run --spec "t/cypress/integration/Circulation/FineNoRenewals_spec.ts"

2. If AllowFineOverrideRenewing is set to "Don't allow", on the checkouts list on the patron's page the "Renew all" action button under the table is still active, although it does renew any items when you click it (there are pop-up windows "The patron has a debt of 8.00Are you sure you want to renew checkout(s)?", click OK, "There are no items to be renewed").


Testing notes (using KTD):

1. I attempted to follow the individual; patch test plans, but gave up.

2. I followed the obsoleted test plan in comment #154, and everything seemed to work as described (I didn't test the API things).

3. Tests:
   . prove t/db_dependent/Circulation.t
   . npx cypress run --spec "t/cypress/integration/Circulation/FineNoRenewals_spec.ts"
   . prove t/db_dependent/api/v1/checkouts.t

4. Notes on system preference changes (for updating the documentation):
   - OPACFineNoRenewals: no longer exists
   - FineNoRenewals: replaces OPACFineNoRenewals, now located under Circulation > Checkout policy, wording is now: 
      Only allow patrons or staff to renew items if patron has less than [5] USD in fines (leave blank to disable).
   - OPACFineNoRenewalsBlockAutoRenew: text updated with FineNoRenewals system preference link
   - OPACFineNoRenewalsIncludeCredits: text updated with FineNoRenewals system preference link
   - AllowFineOverrideRenewing: new preference, text is:
      [Don't allow|Allow] staff to manually override and renew items for patrons who have more in fines than set in the FineNoRenewals system preference.
Comment 156 Martin Renvoize (ashimema) 2025-10-30 17:29:05 UTC
Thanks for testing David, I was meaning to revisit the test plan as I wasn't happy with how verbose it ended up relying on AI to generate it... but I'm glad it helped.

I've found and fixed the issue with the Cypress test (caching in the tests themselves), but whilst doing so I also found a few more edge cases in the implementation to test (namely, renewing from the checkouts list tables)

I'll be submitting a new patchset soon, rebased and with a few follow-ups.

Thanks again
Comment 157 David Nind 2025-10-30 18:03:02 UTC
(In reply to Martin Renvoize (ashimema) from comment #156)
> Thanks for testing David, I was meaning to revisit the test plan as I wasn't
> happy with how verbose it ended up relying on AI to generate it... but I'm
> glad it helped.
> 
> I've found and fixed the issue with the Cypress test (caching in the tests
> themselves), but whilst doing so I also found a few more edge cases in the
> implementation to test (namely, renewing from the checkouts list tables)
> 
> I'll be submitting a new patchset soon, rebased and with a few follow-ups.
> 
> Thanks again

+10
Comment 158 Martin Renvoize (ashimema) 2025-10-30 22:12:07 UTC
Created attachment 188708 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 159 Martin Renvoize (ashimema) 2025-10-30 22:12:12 UTC
Created attachment 188709 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 160 Martin Renvoize (ashimema) 2025-10-30 22:12:29 UTC
Created attachment 188710 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 161 Martin Renvoize (ashimema) 2025-10-30 22:12:54 UTC
Created attachment 188711 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 162 Martin Renvoize (ashimema) 2025-10-30 22:13:13 UTC
Created attachment 188712 [details] [review]
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed

This moves the basic calculation for too_much_oweing out of the
controller and into the class method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 163 Martin Renvoize (ashimema) 2025-10-30 22:13:40 UTC
Created attachment 188713 [details] [review]
Bug 23415: Add new syspref AllowFineOverrideRenewing

This patch adds new syspref AllowFineOverrideRenewing. It allows
staff to renew items for patrons whose fines are over limit set
in syspref FineNoRenewals. Otherwise renewing is prevented. If
renewing items is allowed staff is still required to confirm
if they really want to renew items for the patron.

To test:
1. Apply this patch, update database and restart all services.
2. Find patron with fines over limit set in FineNoRenewals and
renewable items.
=> Note that renewing is prevented with message "Cannot renew,
he patron has a debt of..."
3. Set syspref AllowFineOverrideRenewing as "Allow".
=> Note that items can now be renewed and renewing them displays
confirmation pop-up.

Sponsored-by: Koha-Suomi Oy
Comment 164 Martin Renvoize (ashimema) 2025-10-30 22:14:05 UTC
Created attachment 188714 [details] [review]
Bug 23415: Fix QA issues
Comment 165 Martin Renvoize (ashimema) 2025-10-30 22:14:29 UTC
Created attachment 188715 [details] [review]
Bug 23415: (QA follow-up) Fix critical logic and security issues

This commit addresses several critical issues found in QA review:

1. Fixed critical logic bug in C4::Circulation::CanBookBeRenewed
   - Removed incorrect check of AllowFineOverrideRenewing preference
   - The function now always returns 'too_much_oweing' error when
     patron balance exceeds FineNoRenewals limit
   - AllowFineOverrideRenewing should only control UI override
     capability, not the core renewal check

2. Added permission check in circ/renew.pl
   - Override is now only allowed when both override_debt parameter
     is set AND AllowFineOverrideRenewing preference is enabled
   - Prevents security issue where staff could bypass the preference
     by crafting POST requests

3. Fixed template to conditionally display override button
   - Override button in renew.tt now only shows when
     AllowFineOverrideRenewing is enabled
   - Prevents confusion when override is not permitted

4. Added comprehensive test coverage
   - Tests verify CanBookBeRenewed behavior with AllowFineOverrideRenewing
     both enabled and disabled
   - Confirms error is always returned regardless of preference setting
   - Increased test count from 116 to 120

5. Fixed minor issues
   - Fixed typo: "he patron" -> "the patron" in checkouts.js
   - Fixed typo: OPACFineNoRenewalsIncludeCredit ->
     OPACFineNoRenewalsIncludeCredits in test

All tests pass successfully.
Comment 166 Martin Renvoize (ashimema) 2025-10-30 22:14:50 UTC
Created attachment 188716 [details] [review]
Bug 23415: (QA follow-up) Fix spelling: oweing -> owing

Standardizes error code spelling across the codebase:
- Changes 'too_much_oweing' to 'too_much_owing'
- Changes 'auto_too_much_oweing' to 'auto_too_much_owing'

The word "owing" is the correct spelling in English.

Files updated:
- C4/Circulation.pm
- C4/SIP/Sip/MsgType.pm
- circ/renew.pl
- koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
- koha-tmpl/intranet-tmpl/prog/js/checkouts.js
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
- misc/cronjobs/automatic_renewals.pl
- opac/opac-reserve.pl
- opac/opac-user.pl
- t/db_dependent/Circulation.t

All tests pass successfully.
Comment 167 Martin Renvoize (ashimema) 2025-10-30 22:16:57 UTC
Created attachment 188717 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 168 Martin Renvoize (ashimema) 2025-10-30 22:17:05 UTC
Created attachment 188718 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 169 Martin Renvoize (ashimema) 2025-10-30 22:17:29 UTC
Created attachment 188719 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 170 Martin Renvoize (ashimema) 2025-10-30 22:17:56 UTC
Created attachment 188720 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 171 Martin Renvoize (ashimema) 2025-10-31 06:57:58 UTC
Created attachment 188738 [details] [review]
Bug 23415: Notify patron fines when renewing

It is possible to renew items for patron who has fines over accepted
limit on renew page or from checkout list. This patch adds confirmation
alerts to these renew actions when patron has fines over
"noissuescharge" syspref.

Test plan:
1. Have patron with checkouts and fines over allowed limit
2. Renew checkouts either from renew page or checkout list
=> renew is successful
3. Apply patch
4. Repeat steps 1 and 2
=> On renew page an alert is displayed and user has to confirm
renew
=> When renew is done from checkout list confirmation pop-up
is displayed

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 172 Martin Renvoize (ashimema) 2025-10-31 06:58:02 UTC
Created attachment 188739 [details] [review]
Bug 23415: Rename OPACFineNoRenewals

OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.

To test, confirm everything works as described in
previous test plans.

Also prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 173 Martin Renvoize (ashimema) 2025-10-31 06:58:06 UTC
Created attachment 188740 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 174 Martin Renvoize (ashimema) 2025-10-31 06:58:09 UTC
Created attachment 188741 [details] [review]
Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed

This moves the basic calculation for too_much_oweing out of the
controller and into the class method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 175 Martin Renvoize (ashimema) 2025-10-31 06:58:13 UTC
Created attachment 188742 [details] [review]
Bug 23415: Add new syspref AllowFineOverrideRenewing

This patch adds new syspref AllowFineOverrideRenewing. It allows
staff to renew items for patrons whose fines are over limit set
in syspref FineNoRenewals. Otherwise renewing is prevented. If
renewing items is allowed staff is still required to confirm
if they really want to renew items for the patron.

To test:
1. Apply this patch, update database and restart all services.
2. Find patron with fines over limit set in FineNoRenewals and
renewable items.
=> Note that renewing is prevented with message "Cannot renew,
the patron has a debt of..."
3. Set syspref AllowFineOverrideRenewing as "Allow".
=> Note that items can now be renewed and renewing them displays
confirmation pop-up.

Sponsored-by: Koha-Suomi Oy
Signed-of-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 176 Martin Renvoize (ashimema) 2025-10-31 06:58:16 UTC
Created attachment 188743 [details] [review]
Bug 23415: Fix QA issues

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 177 Martin Renvoize (ashimema) 2025-10-31 06:58:20 UTC
Created attachment 188744 [details] [review]
Bug 23415: (QA follow-up) Fix logic and security issues

This commit addresses several issues found in QA review:

1. Fixed logic bug in C4::Circulation::CanBookBeRenewed
   - Removed incorrect check of AllowFineOverrideRenewing preference
   - The function now always returns 'too_much_oweing' error when
     patron balance exceeds FineNoRenewals limit
   - AllowFineOverrideRenewing should only control UI override
     capability, not the core renewal check
2. Added permission check in circ/renew.pl
   - Override is now only allowed when both override_debt parameter
     is set AND AllowFineOverrideRenewing preference is enabled
   - Prevents security issue where staff could bypass the preference
     by crafting POST requests
3. Fixed template to conditionally display override button
   - Override button in renew.tt now only shows when
     AllowFineOverrideRenewing is enabled
   - Prevents confusion when override is not permitted
4. Added test coverage
   - Tests verify CanBookBeRenewed behavior with AllowFineOverrideRenewing
     both enabled and disabled
   - Confirms error is always returned regardless of preference setting
5. Fixed minor issues
   - Fixed typo: "he patron" -> "the patron" in checkouts.js
   - Fixed typo: OPACFineNoRenewalsIncludeCredit ->
     OPACFineNoRenewalsIncludeCredits in test

All tests should pass successfully.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 178 Martin Renvoize (ashimema) 2025-10-31 06:58:24 UTC
Created attachment 188745 [details] [review]
Bug 23415: (QA follow-up) Fix spelling: oweing -> owing

Standardizes error code spelling across the codebase:
- Changes 'too_much_oweing' to 'too_much_owing'
- Changes 'auto_too_much_oweing' to 'auto_too_much_owing'

The word "owing" is the correct spelling in English.

Files updated:
- C4/Circulation.pm
- C4/SIP/Sip/MsgType.pm
- circ/renew.pl
- koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
- koha-tmpl/intranet-tmpl/prog/js/checkouts.js
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
- misc/cronjobs/automatic_renewals.pl
- opac/opac-reserve.pl
- opac/opac-user.pl
- t/db_dependent/Circulation.t

All tests should pass successfully.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 179 Martin Renvoize (ashimema) 2025-10-31 06:58:28 UTC
Created attachment 188746 [details] [review]
Bug 23415: Add Cypress test coverage for FineNoRenewals

This commit adds end-to-end tests for the fine-based
renewal blocking functionality using Cypress.

Test coverage includes:
1. Blocking renewal when patron fines exceed FineNoRenewals limit
2. Displaying override button when AllowFineOverrideRenewing is enabled
3. Hiding override button when AllowFineOverrideRenewing is disabled
4. Successful override when permission is enabled
5. Allowing renewal when fines are below the limit

The test creates test data including:
- A patron with checkouts
- Account fines that exceed the FineNoRenewals threshold
- Tests both blocking and override scenarios

Tests verify the UI behavior matches the business logic:
- Error messages are displayed correctly
- Override buttons appear based on preference setting
- Override functionality works as expected
- Security is maintained (no override without permission)

To run these tests:
  npx cypress run --spec "t/cypress/integration/Circulation/FineNoRenewals_spec.ts"

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 180 Martin Renvoize (ashimema) 2025-10-31 06:58:31 UTC
Created attachment 188747 [details] [review]
Bug 23415: Add API tests for fine override on renewal

This commit adds REST API tests to verify the
x-koha-override: debt_limit functionality for renewals.

Tests cover:
- Renewal blocked due to excessive fines
- Override fails when AllowFineOverrideRenewing is disabled
- Override succeeds when AllowFineOverrideRenewing is enabled
- Both /renewal and /renewals endpoints

Test plan:
1. Run: prove t/db_dependent/api/v1/checkouts.t
2. All 110 tests should pass

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 181 Martin Renvoize (ashimema) 2025-10-31 06:58:35 UTC
Created attachment 188748 [details] [review]
Bug 23415: Add REST API support for fine override on renewal

This commit adds support for the AllowFineOverrideRenewing functionality
to the REST API renewal endpoints using the standard x-koha-override pattern.

Changes:
1. Added x-koha-override header parameter to both renewal endpoints:
   - POST /checkouts/{checkout_id}/renewal
   - POST /checkouts/{checkout_id}/renewals
2. Updated Koha::REST::V1::Checkouts::renew() to:
   - Check for 'debt_limit' in the overrides stash
   - Check AllowFineOverrideRenewing preference when override is requested
   - Allow renewal override only when both:
     * x-koha-override header contains 'debt_limit' AND
     * AllowFineOverrideRenewing preference is enabled
   - Follows the established x-koha-override pattern used by other endpoints
3. Updated OpenAPI specification (swagger) to document the new header
   using the standard pattern with enum values

Usage:
  curl -X POST /api/v1/checkouts/123/renewal \
    -H "x-koha-override: debt_limit"

The override only works for too_much_owing errors and requires
the AllowFineOverrideRenewing system preference to be enabled.

This implementation follows the same pattern as holds and other
endpoints that support overrides, maintaining consistency across
the REST API.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 182 Martin Renvoize (ashimema) 2025-10-31 06:58:39 UTC
Created attachment 188749 [details] [review]
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

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 183 Martin Renvoize (ashimema) 2025-10-31 06:58:43 UTC
Created attachment 188750 [details] [review]
Bug 23415: Add debt override support to circulation checkout page

This patch implements fine override functionality for the checkouts
table on the circulation page (circ/circulation.pl), completing the
debt override feature for all renewal workflows.

Changes:
- Added override_debt checkbox to checkouts-table.inc when
  AllowFineOverrideRenewing preference is enabled
- Updated /svc/renew to accept and process override_debt parameter,
  matching the pattern used for override_limit
- Enhanced circulation-api-client.js to pass override_debt parameter
  through the renewal API call
- Implemented specific CSS classes for renewal error types
  (renewals-allowed-too_many, renewals-allowed-too_much_owing, etc.)
  to enable selective checkbox control
- Modified override checkbox handlers to selectively enable only
  relevant renewal checkboxes based on the specific blocking error:
  * override_limit enables checkboxes for 'too_many' and 'on_reserve'
  * override_debt enables checkboxes for 'too_much_owing' and
    'auto_too_much_owing'
- Added AllowFineOverrideRenewing JavaScript variable to circulation.tt

This provides clear visual feedback to staff about which renewals will
be affected by each override option, improving usability and reducing
confusion when multiple items have different blocking reasons.
Comment 184 Martin Renvoize (ashimema) 2025-10-31 06:58:47 UTC
Created attachment 188751 [details] [review]
Bug 23415: (follow-up) Respect OPACFineNoRenewalsIncludeCredits preference

This patch fixes a regression introduced in the previous commits where
we changed FineNoRenewals checking to use non_issues_charges instead
of balance. While this change was correct for focusing on blocking
charges, we inadvertently removed the handling of the
OPACFineNoRenewalsIncludeCredits preference.

The OPACFineNoRenewalsIncludeCredits preference controls whether
unapplied account credits should be included when calculating if a
patron exceeds the FineNoRenewals threshold. When this preference is
enabled, the patron's net balance (debits minus credits) should be
used instead of just their outstanding debits.

This patch restores the preference handling for both:
1. Manual renewals in CanBookBeRenewed()
2. Auto renewals in _CanBookBeAutoRenewed()

When OPACFineNoRenewalsIncludeCredits = 1:
  Use balance (all charges net of credits)
When OPACFineNoRenewalsIncludeCredits = 0:
  Use non_issues_charges (only blocking charges, no credits)

This ensures that patrons with sufficient credits can renew their
items when the preference is enabled, while still maintaining the
focus on non-issuing charges introduced in the previous commits.

Test plan:
1. Run prove t/db_dependent/Circulation.t
2. All tests should pass, particularly the
   "auto_too_much_owing | OPACFineNoRenewalsBlockAutoRenew &
   OPACFineNoRenewalsIncludeCredits" subtest

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 185 Martin Renvoize (ashimema) 2025-10-31 07:06:48 UTC
Checks are all happy at this stage, but I'd like a user-facing signoff and then I think we can upgrade my SO lines to a QA stamp
Comment 186 Andrew Fuerste-Henry 2025-10-31 13:13:30 UTC
These sysprefs should get their names adjusted to remove the "OPAC" so they match the new name of FineNoRenewals: OPACFineNoRenewalsIncludeCredits, OPACFineNoRenewalsBlockAutoRenew.

These patches also block renewals via the web-based selfcheck when the patron has fines over FineNoRenewals, which is a change in behavior. OPACFineNoRenewals does not currently block renewals via the web-based selfcheck. This needs a syspref to allow the choice of how self-check renewals are handled so users can maintain current behavior if desired.

That said, I really like these changes and will be happy to help sign it off!