Bug 3498

Summary: Allow partial fine payments
Product: Koha Reporter: Jane Wagner <jwagner>
Component: PatronsAssignee: Colin Campbell <colin.campbell>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: PATCH-Sent (DO NOT USE) CC: chobbs, chris, colin.campbell, cookie.wolfrom, dgrobani, katrin.fischer, koha.sekjal, nengard, robin, smoreland, tdavis, wizzyrea
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Partial payment screenshot
Partial payment paying one fine screenshot
Partial payment paying lump sum screenshot
Patch adding functionality
screenshot
Template corrections on paycollect.pl
Corrects the structure of the HTML of paycollect.tmpl
Removes the CAN_user_updatecharges_writeoff_charges permission.
Patches reworked to use TT
Bug 3498 Allow Partial Payment of Fines
Selenium test that adds an adult patron
Selenium test that creates manual fines and pays them back
my steps
Bug 3498 - Documentation update to explain what every button does.
Bug 3498 Allow Partial Payment of Fines

Description Chris Cormack 2010-05-21 01:11:25 UTC


---- Reported by jwagner@ptfs.com 2009-08-03 14:23:27 ----

Created an attachment
Partial payment screenshot

Sponsored by East Brunswick Public Library, East Brunswick, NJ, USA.

Allow partial payments of outstanding fines, either against individual fine entries or as a lump payment.  Requires related granular permission development.



---- Additional Comments From jwagner@ptfs.com 2009-08-03 14:23:55 ----

Created an attachment
Partial payment paying one fine screenshot





---- Additional Comments From jwagner@ptfs.com 2009-08-03 14:24:19 ----

Created an attachment
Partial payment paying lump sum screenshot





---- Additional Comments From colin.campbell@ptfs-europe.com 2010-03-29 07:35:18 ----

Created an attachment
Patch adding functionality

Patch adds the partial payment feature. The dependency on granluar permissions in the pay functionality is not in this version. 



---- Additional Comments From jwagner@ptfs.com 2010-05-12 12:51:28 ----

Colin's patch of March 29 did not include the granular permission dependency; the full fix for this bug (in ptfs-harley) can be pulled from the public git repository at http://github.com/Koha-PTFS, branch http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3498





--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:11 UTC  ---

This bug was previously known as _bug_ 3498 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3498
Imported an attachment (id=1365)
Imported an attachment (id=1366)
Imported an attachment (id=1367)
Imported an attachment (id=1368)

Unknown operating system Windows 2000. Setting to default OS "All".
Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was jwagner@ptfs.com.
The original assignee of this bug does not have
   an account here. Reassigning to the default assignee
   for the component, gmcharlt@gmail.com.
   Previous assignee was kohaprogrammers@ptfs.com.
CC member arm@hanover.ca does not have an account here
CC member melanie@salpublib.org does not have an account here
The original submitter of attachment 1365 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 1366 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 1367 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 1368 [details] [review] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Colin Campbell 2010-09-09 19:42:19 UTC
I've rebased the initial patch against 3.2_beta as the branch bug_3498 at my koha repository git@github.com:colinsc/koha.git This version is in production use in the UK.
Comment 2 Robin Sheat 2010-09-20 05:34:57 UTC
This seems to be working for us, too.
Comment 3 Katrin Fischer 2010-12-12 15:14:53 UTC
These look like cool features. What is the status of this bug?
Comment 4 Colin Campbell 2011-01-12 09:47:48 UTC
Status of the branch is that we (PTFS Europe) have been running it in production for some time now and it goes in our new installs as it provides something most of our customers require. I'd be grateful if someone would test with a view to upping the canditure for its inclusion in 3.4.
Comment 5 Katrin Fischer 2011-03-19 20:16:00 UTC
Created attachment 3359 [details]
screenshot

Patch applied without conflicts.

1) I seems the patch relies on a new permission for staff to be allowed to rite-off fines: CAN_user_updatecharges_writeoff_charges
The code for adding this permission is missing from the patch, so I could not test the write-off functionality.

2) The 'pay amount' table seems a bit broken. (strange formatting, borders) (see old screenshot: Partial payment paying lump sum)

3)'Pay selected' asks for 1.15 to collect from patron, but the total due is 0.15. It seems the manual credit is not correctly taken into account. After confirming 1.15 the amount was corrected to 0.15. Confirmed again and it worked.
Another problem of the manual credit is, that the entries on the 'pay fines' tab are not cleaned Those paid by credit remain on the list. Only paying them with'Pay' beside the entry makes those go away.
Comment 6 Frédérick Capovilla 2011-04-01 14:53:59 UTC Comment hidden (obsolete)
Comment 7 Frédérick Capovilla 2011-04-04 13:07:44 UTC Comment hidden (obsolete)
Comment 8 Frédérick Capovilla 2011-04-05 16:38:27 UTC Comment hidden (obsolete)
Comment 9 Nicole C. Engard 2011-04-19 15:14:07 UTC
This patch (or series of patches) needs to be updated to update the *.tt files.
Comment 10 Colin Campbell 2011-08-09 10:14:15 UTC Comment hidden (obsolete)
Comment 11 Colin Campbell 2011-08-09 10:18:41 UTC
Reworked original patch to utilize template toolkit.

screens also use circ-menu,tt instead of circ-menu.inc to avoid the need to maintain global variables for borrower attributes
Comment 12 Elliott Davis 2011-08-10 03:47:29 UTC
It looks good to me, I am up for signing off on this except I didn't see a testing procedure.  I know I am new to testing so if this is wrong please let me know.
Comment 13 Nicole C. Engard 2011-08-15 19:39:25 UTC
Elliott,

If you knew how to test it then a testing plan isn't necessary, you can sign off. I do think we want a second sign off as well, so attach your signed off patch and I'll test when I get a chance and give you a second sign off.

Nicole
Comment 14 Elliott Davis 2011-08-15 20:47:27 UTC
Ok sounds good.  I have been testing from origin/new/awaiting_qa/bug_5549 so I'll need to set up a testing branch on master to actually test but it works with my bug branch.
Comment 15 Liz Rea 2011-09-08 20:09:16 UTC Comment hidden (obsolete)
Comment 16 Liz Rea 2011-09-08 20:11:26 UTC
Created attachment 5361 [details]
Selenium test that adds an adult patron

This selenium test adds the patron necessary for the fines test to work.
Comment 17 Liz Rea 2011-09-08 20:12:19 UTC
Created attachment 5362 [details]
Selenium test that creates manual fines and pays them back

This test will use the create patron test for Testpatron.
Comment 18 Liz Rea 2011-09-08 20:29:01 UTC
When attempting to run the tests, make sure your base URL is set to the address of your server in Selenium. :)
Comment 19 Nicole C. Engard 2011-09-08 21:16:01 UTC
Created attachment 5363 [details]
my steps

I selected multiple lines to pay and clicked 'pay amount' it then asked me how much so I put in a small amount. on the next screen it shows the amount paid off of one of the lines I didn't select.

Do we care?? I'm about to test some more.
Comment 20 Nicole C. Engard 2011-09-08 21:19:53 UTC
Also, this is a design issue - but the writeoff all button writes off all lines, as it implies, but the other buttons around it require that you check a box above so it might confuse people - thinking they select the ones they want to write off and then click the button
Comment 21 Nicole C. Engard 2011-09-08 21:21:08 UTC
regarding my screenshot for pay amont - i'm thinking now that pay amount can be clicked without anything selected and it just applies to any line - this too is a design issue we need it to be clear what the three buttons do and why i'd click one over another.

I did pay selected and it did put the money toward one of the lines i selected.
Comment 22 Ian Walls 2011-09-08 21:25:07 UTC
Applying the partial payment value against a fine that is not selected with the checkboxes seems to be incorrect.  This will cause much confusion amongst librarians, I predict.

This has failed QA until one of two things happens:

a) the partial payment is assessed only against selected fines

or

b) it is somehow made clear to users that their selections do not matter when making a partial payment.

I would prefer a), but given the state of the underlying Accounts code, b) may be the quicker route to go.
Comment 23 Liz Rea 2011-09-09 14:30:30 UTC
We had this back when we had harley (it's one of the very few things that our librarians liked about it). 

I do not recall any confusion on their part about how the payments/writeoffs were handled. During my testing, it seemed clear to me what each button did, without excessive explanation.

That said, I'll work up a patch for the help file that attempts to explain how everything works, which should satisfy Ian's (b).
Comment 24 Liz Rea 2011-09-09 15:10:22 UTC Comment hidden (obsolete)
Comment 25 Liz Rea 2011-09-12 15:12:17 UTC
Created attachment 5406 [details] [review]
Bug 3498 Allow Partial Payment of Fines

Allow partial payment of outstanding fines
against either individual fine entries or as a lump payment

Sponsered by East Brunswick Public Library, East Brunswick, NJ, USA

NB: Adds a version of the include circ-menu.inc as circ-menu.tt
this has the same functionality as the old include but does not
require all the borrowers attributes to be passed as global
scoped variables

Signed-off-by: Liz Rea <lrea@nekls.org>

Bug 3498 - Documentation update to explain what every button does.

Help file updated to the following:
Pay and Writeoff Fines

Each line item can be paid in full, partially paid, or written off.
Pay a fine in full

    Click "Pay" next to the fine you want to pay in full
    The full amount of the fine will be populated for you in the "Collect From Patron" box
    Click "Confirm"
    The fine will be removed from outstanding fines, and displayed as fully paid.

Pay a partial fine

    Click "Pay" next to the fine you want to partially pay
    Enter the amount you are collecting from the patron in the "Collect From Patron" box
    Click "Confirm"
    The fine will be updated to show the original Amount, and the current Amount Outstanding

Writeoff a single fine

    Click "Writeoff" next to the fine you wish to writeoff.
    The fine will be removed from outstanding fines, and displayed as fully paid.

Pay an amount towards all fines

    Click the "Pay Amount" button
    Enter the amount you are collecting from the patron in "Collect from Patron." The sum of all fines is shown in "Total Amount Outstanding"
    Click "Confirm"
    The fine totals will be updated with the payment applied to oldest fines first.

Writeoff All fines

    Click the "Writeoff All" button
    All fines will be removed from outstanding fines, and displayed as written off.

Pay Selected fines

    Check the selection boxes next to the fines you wish to pay, click "Pay Selected"
    Enter an amount to pay towards the fines.
    Click "Confirm"
    The fine totals will be updated with the payment applied to the oldest selected fines first.

Bug 3498 - adding parens to TT IF Statements for style

pay.tt

Bug 3498 - Add parens to TT IF statements for style

paycollect.tt
Comment 26 Ian Walls 2011-09-12 15:25:07 UTC
Thank you to Elliott, Nicole and Liz for a most thorough testing of this patch, and for putting up with my pesky style and documentation demands.  I'm happy to pass this patch through QA.

Bug 6427 can now be re-evaluated in the context of this fix.
Comment 27 Chris Cormack 2011-09-12 20:41:27 UTC
Pushed, please test
Comment 28 Sharon Moreland 2011-10-05 20:19:36 UTC
Hi all,
I tried to reverse payment of a partial payment and it didn't account as expected.  Can anyone duplicate?
http://screencast.com/t/BK6RKtQvnrel
Comment 29 Owen Leonard 2011-10-13 19:06:04 UTC
*** Bug 2169 has been marked as a duplicate of this bug. ***
Comment 30 Jared Camins-Esakov 2012-12-31 00:16:58 UTC
There have been no further reports of problems so I am marking this bug resolved.