Bug 8058 - Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fines
Summary: Circulation empty barcode field redirect to pay fines screen if borrower has ...
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Chris Hall
QA Contact: Paul Poulain
URL:
Keywords:
: 8320 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-05-07 22:42 UTC by Chris Hall
Modified: 2017-07-03 08:54 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fine (4.96 KB, patch)
2012-05-07 22:43 UTC, Chris Hall
Details | Diff | Splinter Review
Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fine (4.99 KB, patch)
2012-05-07 23:06 UTC, Chris Hall
Details | Diff | Splinter Review
Circulation empty barcode field redirect to pay fines screen if borrower has outstanding fine (4.97 KB, patch)
2012-05-14 20:46 UTC, Chris Hall
Details | Diff | Splinter Review
Allows pay fines redirect to work with print spooling. (4.97 KB, patch)
2012-06-27 01:21 UTC, Chris Hall
Details | Diff | Splinter Review
Allows pay fines redirect to work with print spooling. (6.81 KB, patch)
2012-06-27 01:45 UTC, Chris Hall
Details | Diff | Splinter Review
Fixed error in redirecting when user had positive balance (6.38 KB, patch)
2012-09-28 06:02 UTC, Chris Hall
Details | Diff | Splinter Review
Correctly implements PrintAndClear as an explicit option. (9.76 KB, patch)
2012-10-04 00:53 UTC, Chris Hall
Details | Diff | Splinter Review
Correctly implements PrintAndClear as an explicit option. (9.84 KB, patch)
2012-10-05 03:10 UTC, Chris Hall
Details | Diff | Splinter 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. (9.98 KB, patch)
2012-10-05 06:10 UTC, Marc Véron
Details | Diff | Splinter 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. (10.10 KB, patch)
2013-02-25 04:47 UTC, Chris Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Hall 2012-05-07 22:42:45 UTC
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).
Comment 1 Chris Hall 2012-05-07 22:43:52 UTC Comment hidden (obsolete)
Comment 2 Chris Hall 2012-05-07 22:48:12 UTC
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
Comment 3 Chris Hall 2012-05-07 23:06:07 UTC Comment hidden (obsolete)
Comment 4 Jared Camins-Esakov 2012-05-13 12:16:30 UTC
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.
Comment 5 Chris Hall 2012-05-14 20:46:01 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2012-06-26 13:43:49 UTC
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.
Comment 7 Marc Véron 2012-06-26 15:02:31 UTC
Opened Bug 8320 as replacement
Comment 8 Chris Hall 2012-06-26 21:49:14 UTC
I quite like your suggestion Marc Veron, I think having all the related options immediately visible in the same syspref is an improvement.
Comment 9 Chris Cormack 2012-06-26 21:52:51 UTC
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
Comment 10 Chris Hall 2012-06-26 23:21:59 UTC
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).
Comment 11 Chris Hall 2012-06-27 01:21:58 UTC Comment hidden (obsolete)
Comment 12 Chris Hall 2012-06-27 01:45:57 UTC Comment hidden (obsolete)
Comment 13 Marc Véron 2012-06-27 05:12:47 UTC
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
Comment 14 Owen Leonard 2012-07-20 14:11:41 UTC
Sounds like this requires a follow-up? Please change the status if I'm incorrect.
Comment 15 Chris Hall 2012-09-28 06:02:54 UTC Comment hidden (obsolete)
Comment 16 Marc Véron 2012-09-28 08:36:29 UTC
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
Comment 17 Chris Hall 2012-10-04 00:53:36 UTC Comment hidden (obsolete)
Comment 18 Chris Hall 2012-10-04 00:56:10 UTC
... by saying 'thank you for catching'
I meant 
'thank you for catching that change of CircAutoPrintQuickSlip behavior'

~Chris
Comment 19 Chris Hall 2012-10-05 03:10:56 UTC Comment hidden (obsolete)
Comment 20 Marc Véron 2012-10-05 06:10:31 UTC Comment hidden (obsolete)
Comment 21 Marc Véron 2012-10-05 06:14:24 UTC
*** Bug 8320 has been marked as a duplicate of this bug. ***
Comment 22 Paul Poulain 2012-11-28 14:41:23 UTC
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)
Comment 23 Chris Hall 2013-02-25 04:47:31 UTC
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
Comment 24 Mark Tompsett 2014-04-19 06:00:46 UTC
How does this patch relate functionality wise to bug 11040?
Comment 25 Marc Véron 2017-07-03 08:54:50 UTC
Still valid?