Summary: | Fix impossible warning in circulation.pl when OverduesBlockCirc set to Block | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Circulation | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | Mason James <mtj> |
Severity: | minor | ||
Priority: | P5 - low | CC: | chris, gmcharlt, kyle.m.hall, kyle, mtj, paul.poulain |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 8422: Fix impossible warning in circulation.pl when OverduesBlockCirc set to Block
[SIGNED-OFF] Bug 8422 - Fix impossible warning in circulation.pl when OverduesBlockCirc set to Block |
Description
David Cook
2012-07-12 01:23:57 UTC
Created attachment 10779 [details] [review] Bug 8422: Fix impossible warning in circulation.pl when OverduesBlockCirc set to Block Created attachment 10829 [details] [review] [SIGNED-OFF] Bug 8422 - Fix impossible warning in circulation.pl when OverduesBlockCirc set to Block Previously, the message Patron has x overdue item(s). Checkout anyway? was shown when syspref OverduesBlockCirc was set to Ask for confirmation or Block. This patch changes the message for Block to: Checkouts are BLOCKED because patron has overdues. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to comment #2) > Created attachment 10829 [details] [review] > [SIGNED-OFF] Bug 8422 - Fix impossible warning in circulation.pl when > OverduesBlockCirc set to Block > > Previously, the message Patron has x overdue item(s). Checkout anyway? was > shown when syspref OverduesBlockCirc was set to Ask for confirmation or > Block. > > This patch changes the message for Block to: Checkouts are BLOCKED because > patron has overdues. > > Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> passing QA... trivial patch passes QA tests OK > Thoughts?
David, my thoughts are that the Circulation would not deserve a complete rewritte... Many things are crappy/ years of code piled, you've found one.
About your patch = it's pushed. Note that you could have kept the [% USERBLOCKEDOVERDUE %] information:
Checkouts are BLOCKED because patron has [% USERBLOCKEDOVERDUE %] overdue items
Is there reason not to do it that way ?
(In reply to comment #4) > > Thoughts? > > David, my thoughts are that the Circulation would not deserve a complete > rewritte... Many things are crappy/ years of code piled, you've found one. > > About your patch = it's pushed. Note that you could have kept the [% > USERBLOCKEDOVERDUE %] information: > Checkouts are BLOCKED because patron has [% USERBLOCKEDOVERDUE %] overdue > items > > Is there reason not to do it that way ? Point well received, Paul. I'm not sure that the template would need a complete re-write, but I figured it was worth flagging and maybe I will come back and fix that up someday. I don't have the programming skill (yet) to work on some of the deeper issues, but I figure that I can help iron out the interface. As for my patch, I thought about keeping the [% > USERBLOCKEDOVERDUE %] information, but I thought that it was potentially misleading. The patron isn't blocked because of the number of overdue items. They're blocked because they have overdue items. I figured that my patch used more precise language. Pushed to 3.8.x will be in 3.8.4 |