Bug 24008

Summary: Attempting to delete a patron with outstanding credits will warn, but not block the deletion
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jonathan.druart, joy, katrin.fischer, kyle.m.hall, martin.renvoize, testopia, theod, tomascohen, victor
Version: masterKeywords: Manual, release-notes-needed
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
20.05.00
Bug Depends on: 23822    
Bug Blocks:    
Attachments: Bug 24008: Display a warning message when deleting a patron with outstanding credits
Bug 24008: Display warnings in a list
Bug 24008: Remove minus sign when displaying credit value
Bug 24008: Display a warning message when deleting a patron with outstanding credits
Bug 24008: Display warnings in a list
Bug 24008: Remove minus sign when displaying credit value
Bug 24008: Display a warning message when deleting a patron with outstanding credits
Bug 24008: Display warnings in a list
Bug 24008: Remove minus sign when displaying credit value

Description Martin Renvoize 2019-11-11 12:32:28 UTC
+++ This bug was initially created as a clone of Bug #23822 +++

When one tries to delete a patron that has a credit in his account, no warning is produced. 
The deletion is not performed (which is the proper behavior), however one would expect to see a warning similar to the one that is produced when the patron has an unpaid fine, a checked out item, a hold, etc.

The following screencast was performed in ByWaters' sandboxes running 19.05 and shows the issue:
https://screencast-o-matic.com/watch/cq6D6vu4YJ
Comment 1 Katrin Fischer 2019-11-13 14:02:25 UTC
I think we should think if this should block or only warn - holds warn, fines block currently. I could see a credit warning, but not blocking.
Comment 2 Jonathan Druart 2019-12-02 13:27:20 UTC
Created attachment 95924 [details] [review]
Bug 24008: Display a warning message when deleting a patron with outstanding credits

So far nothing is displayed if a librarian removes patron with
outstanding credits.
Note that outstanding debits blocks the deletion.

Test plan:
- Create a patron with outstanding credit
- Create a patron with outstanding debits
- Delete the 2 patrons
=> With credit - You get a warning but do not block the deletion
=> With debits - You get a warning message that blocks the deletion
Comment 3 Katrin Fischer 2019-12-07 10:17:06 UTC
Functionally this works well, but I wonder if we could improve the text a bit:

Patron has 1 hold(s). Deleting patron cancels all their holds.

Are you sure you want to delete the patron Nicole Schneider? This cannot be undone.
Patron has a -9.00 credit.

So now we have one warning on top (holds) an done below. I think that's not ideal for usability. It would be nicer to display them all together, maybe as a list?

Also, should we remove the minus?
Comment 4 Jonathan Druart 2019-12-10 11:27:46 UTC
Created attachment 96142 [details] [review]
Bug 24008: Display warnings in a list
Comment 5 Jonathan Druart 2019-12-10 11:28:54 UTC
(In reply to Katrin Fischer from comment #3)
> Functionally this works well, but I wonder if we could improve the text a
> bit:
> 
> Patron has 1 hold(s). Deleting patron cancels all their holds.
> 
> Are you sure you want to delete the patron Nicole Schneider? This cannot be
> undone.
> Patron has a -9.00 credit.
> 
> So now we have one warning on top (holds) an done below. I think that's not
> ideal for usability. It would be nicer to display them all together, maybe
> as a list?

Done.

> Also, should we remove the minus?


Maybe, no idea.
Comment 6 Martin Renvoize 2019-12-12 12:33:14 UTC
I think it's clearer as a positive amount personally.. it's a 'credit' so it's already identified as such in the sentence rather than needing to state the sign.
Comment 7 Jonathan Druart 2019-12-23 14:36:09 UTC
Created attachment 96604 [details] [review]
Bug 24008: Remove minus sign when displaying credit value
Comment 8 Jonathan Druart 2019-12-23 14:37:05 UTC
Note sure it is expected, but if you have a credit of 5 and a debit of 5, the patron deletion is blocked.
Comment 9 Tomás Cohen Arazi 2019-12-23 14:41:25 UTC
(In reply to Jonathan Druart from comment #8)
> Note sure it is expected, but if you have a credit of 5 and a debit of 5,
> the patron deletion is blocked.

It is expected. Unless AccountAutoReconcile is set or reconcile_balance.pl run, this situations can happen. And credits need to be applied manually. So technically, there's credit on their account.
Comment 10 Martin Renvoize 2020-01-15 16:36:54 UTC
Created attachment 97400 [details] [review]
Bug 24008: Display a warning message when deleting a patron with outstanding credits

So far nothing is displayed if a librarian removes patron with
outstanding credits.
Note that outstanding debits blocks the deletion.

Test plan:
- Create a patron with outstanding credit
- Create a patron with outstanding debits
- Delete the 2 patrons
=> With credit - You get a warning but do not block the deletion
=> With debits - You get a warning message that blocks the deletion

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-01-15 16:36:57 UTC
Created attachment 97401 [details] [review]
Bug 24008: Display warnings in a list

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2020-01-15 16:37:00 UTC
Created attachment 97402 [details] [review]
Bug 24008: Remove minus sign when displaying credit value

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2020-01-15 16:37:39 UTC
Works well, signing off.
Comment 14 Katrin Fischer 2020-01-20 07:11:57 UTC
Created attachment 97603 [details] [review]
Bug 24008: Display a warning message when deleting a patron with outstanding credits

So far nothing is displayed if a librarian removes patron with
outstanding credits.
Note that outstanding debits blocks the deletion.

Test plan:
- Create a patron with outstanding credit
- Create a patron with outstanding debits
- Delete the 2 patrons
=> With credit - You get a warning but do not block the deletion
=> With debits - You get a warning message that blocks the deletion

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2020-01-20 07:12:01 UTC
Created attachment 97604 [details] [review]
Bug 24008: Display warnings in a list

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2020-01-20 07:12:04 UTC
Created attachment 97605 [details] [review]
Bug 24008: Remove minus sign when displaying credit value

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2020-01-20 07:12:31 UTC
I feel styling could be a little improved for the message in general, but that's outside of scope here. Works well.
Comment 18 Martin Renvoize 2020-01-20 14:04:26 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 19 Joy Nelson 2020-02-01 00:04:48 UTC
Enhancement not backported to 19.11.x branch