Added syspref CircPayFinesRedirect that allows the pay fines page to appear if the borrower has outstanding fines when an empty barcode is entered from within circulations. If CircAutoPrintQuickSlip (added in bug 7077) is enabled then CircPayFinesRedirect will redirect after the quick slip window has opened (so it does not interfere with existing behavior).
Created attachment 9445 [details] [review] Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fine Added attachment (had to submit bug first to get bug number for comment)
There is currently an error where if you set CircPayFinesRedirect to 'do nothing' and have CircAutoPrintQuickSlip set to 'clear screen' then koha will throw an error about an undefined PrintIssueSlip function in circ/circulation.pl, this has been reported in bug 7001
Created attachment 9446 [details] [review] Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fine Forgot to recreate patch after adding in 'return false' to jquery bind.
Chris, Make sure you proof your patches for spelling, particularly of syspref names. Is the syspref supposed to be CircPayFinesRedirect or CircPayFindsRedirect? I think the first, but before I changed all instances of the second to the first I wanted to check that I understood correctly.
Created attachment 9574 [details] [review] Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fine Strange as I was able to test it fine and looking at my database I see "circpayfinesredirect" in the systempreference table and not "circpayfindsredirect". Sorry this happened and thanks for pointing it out, corrected patch attached.
Chris and Jared, I was asked to add an additional functionality after printing: Automaticalle clear the screen. This would be in conflict with Chris' solution. So I think we could implement both using a more flexible approach. I propose to drop the preference 'circpayfinesredirect' Instead, expand the choices of 'CircAutoPrintQuickSlip' with two more options: "open a print quick slip window and clear the screen" (that's what I need to have) "open a print quick slip window and redirect to outstanding fines (if any)" (that's what Chris needs to have) The values then can be evaluated in circulation.tt (near the comment '// listen submit to trigger qslip on empty checkout'). This can be expanded for any needs.
Opened Bug 8320 as replacement
I quite like your suggestion Marc Veron, I think having all the related options immediately visible in the same syspref is an improvement.
Just need to make sure the pref works with printing without the windows too. IE printing direct to a printqueue. And then that would be fine
I have done some more work since my last patch, I will upload it shortly. In order to get this to work with print spooling I have moved the CircPayFinesRedirect code from the javascript and in to perl. In order to make this work with correctly CircAutoPrintQuickSlip I may also have move the relevant code in to perl (so any redirects happen after the code dealing with print spooling).
Created attachment 10528 [details] [review] Allows pay fines redirect to work with print spooling. Scratch that last comment, by removing the return statement from the javascript for quickslip and moving the code for CircPayFinesRedirect in to the perl code I am able to get the functionality you desired Marc Veron :) Attached is my work so far, there are 4 combinations. CircPayFinesRedirect: do nothing, CircAutoPrintQuickSlip: clear the screen -> clears the screen CircPayFinesRedirect: redirect and CircAutoPrintQuickSlip: clear the screen -> redirects if borrower has fines, otherwise clears the screen CircPayFinesRedirect: do nothing and CircAutoPrintQuickSlip: open quickslip window -> opens quick print slip and then clears the screen CircPayFinesRedirect: redirect and CircAutoPrintQuickSlip: open quickslip window -> opens quick print slip and then redirects if the borrower has fines, otherwise clears the screen (after opening print slip window). It may be wise to combine these in to a single syspref or change the wording to explain the options better.
Created attachment 10531 [details] [review] Allows pay fines redirect to work with print spooling. Attached proper patch, somehow the old patch got attached rather than the new one.
Chris, Thanks for taking care of the additional functionality I need, see 3) below. I tried out the patch and found the following: -------------- - If a patron has a credit (instead of fines), the redirect is done as well. This should not be. -------------- - CircAutoPrintQuickSlip should have 3 possibilities: 1) Clear the screen (EXISTS) 2) Open a print quick slip window [...and do not change the screen] (EXISTS, behaviour until now, we should not change it) 3) Open a print quick slip window and clear the screen (NEW and needs to be an explicit choice) - CircPayFinesRedirect (if set and if there are fines) should override the options above and redirect to pay fines screen. If patron has no fines or if patron has a credit, it should leave the options above as set. Marc
Sounds like this requires a follow-up? Please change the status if I'm incorrect.
Created attachment 12567 [details] [review] Fixed error in redirecting when user had positive balance Sorry for the delay in getting this up here. I have fixed the issue Marc pointed out regarding, redirecting if the user had a positive balance. I will look in to your comments about QuickAutoPrintQuickSlip as I agree that behaviour shouldn't change. Which of these is preferable for QuickAutoPrintQuickSlip: - add in 2), so have 3 options - remove option 3 and add option 2 back in, so only have options 1) and 2) ~ Chris Hall
Chris, I think we should have an additional option for CircAutoPrintQuickSlip: - Open a print quick slip window and clear the screen ---- The overall behaviour triggered by hitting an empty barcode field would be: Use case 1): Patron has no outstanding fines - Independent of Case CircPayFinesRedirect: - Case CircAutoPrintQuickSlip = Clear the screen => Clear the screen - Case CircAutoPrintQuickSlip = Open a print quick slip window => Print, when print is done go back to the borrowers screen (as is) - Case Open a print quick slip window and clear the screen => Print, when print is done go back to the borrowers screen and clear it Use case 2): Patron has outstanding fines - Case CircPayFinesRedirect is off - do the same as above - Case CircPayFinesRedirect is on - Case CircAutoPrintQuickSlip = Clear the screen => Open the pay fines screen. (When done go back to the borrowers screen.) - Case CircAutoPrintQuickSlip = Open a print quick slip window => Print. Open the pay fines screen. (When done go back to the borrowers screen.) - Case Open a print quick slip window and clear the screen => Print. Open the pay fines screen. (When done go back to the borrowers screen.) What do you think about? Marc
Created attachment 12670 [details] [review] Correctly implements PrintAndClear as an explicit option. Hey Marc, Thank you for catching and for your very clear instructions. Attached is a patch that implements the combinations you requested :) Thanks again, ~Chris Hall
... by saying 'thank you for catching' I meant 'thank you for catching that change of CircAutoPrintQuickSlip behavior' ~Chris
Created attachment 12692 [details] [review] Correctly implements PrintAndClear as an explicit option. Fixed some conflicts against master in updatedatabase and sysprefs.
Created attachment 12693 [details] [review] [SIGNED-OFF] Bug 8058 allow empty barcode in circulation to redirect to pay fines screen Also adds the option for printing the quickslip and then clearing the screen. Signed-off-by: Marc Veron <veron@veron.ch> Works as expected (see use cases in comment #16)
*** Bug 8320 has been marked as a duplicate of this bug. ***
QA comment: trying to QA the patch, when I updatedatabase.pl, I get a strange REST IS (0) that comes from + print "REST IS (@result)\n"; which should probably not be here. Otherwise, the patch passes koha-qa.pl, and there are some conflicts that I think are easy to solve, but please, do them (in case I made something wrong) (not tested by me yet)
Created attachment 15649 [details] [review] Bug 8058 allow empty barcode in circulation to redirect to pay fines screen Also adds the option for printing the quickslip and then clearing the screen. Rebased against current master, Removed accidentaly debugging statement. I also found an error in sysprefs.sql where instead of "PrintAndClear" (which is used everywhere else) I had "ClearAndPrint", this has been corrected. I removed the sign off as I have modified the patch since it was applied, I hope this is correct. For testing (after applying the patch): We need 3 patrons: one with no fines, one with outstanding fines, and one with negative fines (positive balance) Go through each of the 6 combinations of CircAutoPrintQuickSlip (Clear, Print, PrintAndClear) and CircPayFinesRedirect (No Action, Redirect) Visit each of the testing patron's check out pages and enter an empty barcode Expected behavior can be seen in comment #16 and is also summed up below. CircPayFinesRedirect should have no affects for patrons that do not have outstanding fines, in this case behavior is determined by CircAutoPrintQuickSlip. The 'interesting' cases are when a patron has outstanding fines and CircPayFinesRedirect is set to redirect: CircAutoPrintQuickSlip set to Clear : redirects (no clearing) CircAutoPrintQuickSlip set to Print : printing window opens, other window redirects CircAutoPrintQuickSlip set to PrintAndClear : printing window opens, other window redirects
How does this patch relate functionality wise to bug 11040?
Still valid?