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: rel_25_05_candidate
Depends on:
Blocks:
 
Reported: 2019-08-02 07:42 UTC by Emmi Takkinen
Modified: 2024-11-11 16:56 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
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

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
Created attachment 92039 [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
Comment 2 Emmi Takkinen 2019-09-03 10:25:14 UTC
Created attachment 92541 [details] [review]
Bug 23415: Notify patron fines when renewing

In previous patch error message wasn't translatable. Fixed that and made sure loading gif on checkout list works correctly after this patch is applied.
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
Created attachment 98442 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
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
Created attachment 98634 [details] [review]
Bug 23415: Add missing filters
Comment 11 ByWater Sandboxes 2020-02-14 17:54:35 UTC
Created attachment 98996 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 12 ByWater Sandboxes 2020-02-14 17:54:37 UTC
Created attachment 98997 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
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
Created attachment 105893 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Comment 15 Emmi Takkinen 2020-06-16 06:09:51 UTC
Created attachment 105894 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple holds and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has holds. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple holds (at least two)
2. Renew all holds from checkout list
3. Confirmation pop-up is displayed for every hold
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy
Comment 16 Emmi Takkinen 2020-08-17 11:29:08 UTC
Created attachment 108392 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy
Comment 17 Emmi Takkinen 2020-09-11 06:08:30 UTC
Created attachment 109871 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 18 Emmi Takkinen 2020-09-11 06:09:11 UTC
Created attachment 109872 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 19 Emmi Takkinen 2020-09-11 06:09:55 UTC
Created attachment 109876 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Comment 20 Emmi Takkinen 2020-09-11 06:10:39 UTC
Created attachment 109880 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy
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
Created attachment 116919 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 24 Emmi Takkinen 2021-02-16 11:44:02 UTC
Created attachment 116920 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 25 Emmi Takkinen 2021-02-16 11:44:41 UTC
Created attachment 116921 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Comment 26 Emmi Takkinen 2021-02-16 11:45:22 UTC
Created attachment 116922 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy
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
Created attachment 118175 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 29 Emmi Takkinen 2021-03-12 07:06:41 UTC
Created attachment 118176 [details] [review]
Bug 23415: Add missing filters


Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 30 Emmi Takkinen 2021-03-12 07:07:16 UTC
Created attachment 118177 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of  noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Comment 31 Emmi Takkinen 2021-03-12 07:07:48 UTC
Created attachment 118178 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy
Comment 32 Lund University Sandbox 2021-03-12 10:21:34 UTC
Created attachment 118183 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Comment 33 Peter Vashchuk 2021-04-07 14:08:03 UTC
Created attachment 119300 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 34 Peter Vashchuk 2021-04-07 14:08:08 UTC
Created attachment 119301 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 35 Peter Vashchuk 2021-04-07 14:08:13 UTC
Created attachment 119302 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 36 Peter Vashchuk 2021-04-07 14:08:18 UTC
Created attachment 119303 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
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
Created attachment 121472 [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
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
Created attachment 139670 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 42 Emmi Takkinen 2022-08-23 11:57:32 UTC
Created attachment 139671 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 43 Emmi Takkinen 2022-08-23 11:58:02 UTC
Created attachment 139672 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 44 Emmi Takkinen 2022-08-23 11:58:37 UTC
Created attachment 139673 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 45 Emmi Takkinen 2022-08-23 11:59:08 UTC
Created attachment 139674 [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
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
Created attachment 146415 [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
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
Created attachment 148198 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 50 ByWater Sandboxes 2023-03-15 11:11:47 UTC
Created attachment 148199 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 51 ByWater Sandboxes 2023-03-15 11:11:50 UTC
Created attachment 148200 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 52 ByWater Sandboxes 2023-03-15 11:11:52 UTC
Created attachment 148201 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 53 ByWater Sandboxes 2023-03-15 11:11:54 UTC
Created attachment 148202 [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: Sally <sally.healey@cheshiresharedservices.gov.uk>
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
Created attachment 152776 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 57 Emmi Takkinen 2023-06-28 08:20:56 UTC
Created attachment 152777 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 58 Emmi Takkinen 2023-06-28 08:21:28 UTC
Created attachment 152778 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 59 Emmi Takkinen 2023-06-28 08:21:54 UTC
Created attachment 152779 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 60 Emmi Takkinen 2023-06-28 08:22:33 UTC
Created attachment 152780 [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
Comment 61 Emmi Takkinen 2023-06-28 08:22:57 UTC
Created attachment 152781 [details] [review]
Bug 23415: Add fine formatting

To test:
1. Find patron with checkouts and fines over allowed limit.
2. Renew checkouts either from renew page or checkout list.
=> Confirm that fines are formated correctly e.g. "The patron
has a debt of 20.00" not like "The patron has a debt of 20."
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
Created attachment 154595 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Veli-Matti <veli-matti.hakkinen@jyu.fi>
Comment 64 Biblibre Sandboxes 2023-08-18 08:16:16 UTC
Created attachment 154596 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Veli-Matti <veli-matti.hakkinen@jyu.fi>
Comment 65 Biblibre Sandboxes 2023-08-18 08:16:20 UTC
Created attachment 154597 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Veli-Matti <veli-matti.hakkinen@jyu.fi>
Comment 66 Biblibre Sandboxes 2023-08-18 08:16:23 UTC
Created attachment 154598 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Veli-Matti <veli-matti.hakkinen@jyu.fi>
Comment 67 Biblibre Sandboxes 2023-08-18 08:16:26 UTC
Created attachment 154599 [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: Veli-Matti <veli-matti.hakkinen@jyu.fi>
Comment 68 Biblibre Sandboxes 2023-08-18 08:16:29 UTC
Created attachment 154600 [details] [review]
Bug 23415: Add fine formatting

To test:
1. Find patron with checkouts and fines over allowed limit.
2. Renew checkouts either from renew page or checkout list.
=> Confirm that fines are formated correctly e.g. "The patron
has a debt of 20.00" not like "The patron has a debt of 20."

Signed-off-by: Veli-Matti <veli-matti.hakkinen@jyu.fi>
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
Created attachment 158302 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 72 Emmi Takkinen 2023-11-03 07:54:19 UTC
Created attachment 158303 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 73 Emmi Takkinen 2023-11-03 07:54:59 UTC
Created attachment 158304 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 74 Emmi Takkinen 2023-11-03 07:55:25 UTC
Created attachment 158305 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 75 Emmi Takkinen 2023-11-03 07:55:53 UTC
Created attachment 158306 [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
Comment 76 Emmi Takkinen 2023-11-03 07:56:15 UTC
Created attachment 158307 [details] [review]
Bug 23415: Add fine formatting

To test:
1. Find patron with checkouts and fines over allowed limit.
2. Renew checkouts either from renew page or checkout list.
=> Confirm that fines are formated correctly e.g. "The patron
has a debt of 20.00" not like "The patron has a debt of 20."
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
Created attachment 161964 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 80 Emmi Takkinen 2024-02-09 12:20:48 UTC
Created attachment 161965 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 81 Emmi Takkinen 2024-02-09 12:21:31 UTC
Created attachment 161966 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 82 Emmi Takkinen 2024-02-09 12:21:55 UTC
Created attachment 161967 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 83 Emmi Takkinen 2024-02-09 12:22:18 UTC
Created attachment 161968 [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
Comment 84 Emmi Takkinen 2024-02-09 12:22:41 UTC
Created attachment 161969 [details] [review]
Bug 23415: Add fine formatting

To test:
1. Find patron with checkouts and fines over allowed limit.
2. Renew checkouts either from renew page or checkout list.
=> Confirm that fines are formated correctly e.g. "The patron
has a debt of 20.00" not like "The patron has a debt of 20."
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
Created attachment 162171 [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

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 87 Martin Renvoize (ashimema) 2024-02-15 08:03:53 UTC
Created attachment 162172 [details] [review]
Bug 23415: Add missing filters

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 88 Martin Renvoize (ashimema) 2024-02-15 08:03:56 UTC
Created attachment 162173 [details] [review]
Bug 23415: Use OPACFineNoRenewals instead of noissuecharge

Since noissuecharge is mainly fine limit for checkouts it's
more logical to use value in OPACFineNoRenewals.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 89 Martin Renvoize (ashimema) 2024-02-15 08:04:00 UTC
Created attachment 162174 [details] [review]
Bug 23415: Display confirm dialog only once

On detail page when patron has multiple checkouts and they are renewed
from checkouts list, confirmation dialog box is displayed as many times
as patron has checkouts. This patch reworks some code so this dialog box is
displayed only once.

To test:
1. Find patron with multiple checkouts (at least two)
2. Renew all checkouts from checkout list
3. Confirmation pop-up is displayed for every checkout
4. Apply this patch
5. Renew again
=> Confirmation dialog box is displayed only once

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Martin Renvoize (ashimema) 2024-02-15 08:04:05 UTC
Created attachment 162175 [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 91 Martin Renvoize (ashimema) 2024-02-15 08:04:08 UTC
Created attachment 162176 [details] [review]
Bug 23415: Add fine formatting

To test:
1. Find patron with checkouts and fines over allowed limit.
2. Renew checkouts either from renew page or checkout list.
=> Confirm that fines are formated correctly e.g. "The patron
has a debt of 20.00" not like "The patron has a debt of 20."

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Martin Renvoize (ashimema) 2024-02-15 08:04:12 UTC
Created attachment 162177 [details] [review]
Bug 23415: (QA follow-up) Remove OPACFineNoRenewals from syspres.sql

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Martin Renvoize (ashimema) 2024-02-15 08:04:16 UTC
Created attachment 162178 [details] [review]
Bug 23415: Tidy template parameters block

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 94 Martin Renvoize (ashimema) 2024-02-15 08:04:20 UTC
Created attachment 162179 [details] [review]
Bug 23415: (QA follow-up) Spelling and price formatting

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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.