Description
Martin Renvoize (ashimema)
2019-11-11 12:32:28 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. 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 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? Created attachment 96142 [details] [review] Bug 24008: Display warnings in a list (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. 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. Created attachment 96604 [details] [review] Bug 24008: Remove minus sign when displaying credit value Note sure it is expected, but if you have a credit of 5 and a debit of 5, the patron deletion is blocked. (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. 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> Created attachment 97401 [details] [review] Bug 24008: Display warnings in a list Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 97402 [details] [review] Bug 24008: Remove minus sign when displaying credit value Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Works well, signing off. 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> 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> 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> I feel styling could be a little improved for the message in general, but that's outside of scope here. Works well. Nice work everyone! Pushed to master for 20.05 Enhancement not backported to 19.11.x branch |