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: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-02 07:42 UTC by Emmi Takkinen
Modified: 2020-03-22 17:36 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


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

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 :)