Created attachment 4743 [details] sample accountlines data The accountlines table has a manager_id field but this field is not populated when you pay fines (or in some other instances). This field should be populated with the person who is logged in at the time the fine is paid or at the time any change is made to the account so you can track who's doing what with money in your library.
Created attachment 4745 [details] another sample with account type
Created attachment 4754 [details] [review] patch This patch actually helps bug 2696 along a bit by putting the item number back in the database when a payment is made - making it possible to print that was paid for on the account page. Nicole
This patch touches two C4 modules; could I get a second signoff? I'm not comfortable marking this as "passed QA" when only me and one of my fellow ByWaterians have checked it.
I did some additional testing on the bug and it seems there is still no manager_id saved in accountlines when you do a 'writeoff'. To reproduce: - create manual invoice. Example. Sundry, 0.25 - writeoff fine from pay fines page Paying fines the way described on the bugs was working perfectly. I am signing off on this as the bug only mentions paying fines and I see no problems there. The writeoff problem can is a separate bug and can be fixed in a follow up.
Created attachment 4991 [details] [review] [SIGNED-OFF] Bugs 6634: manager_id not recorded for payments and rental charges Manager_id, the borrowernumber of the staff person doing the action, was not recorded for payments, rental charges, or rental charges on renewals. This patch implements that. This patch also adds the recording of itemnumber in a payment, by taking the itemnumber of the account that it's paying off. This addresses bug 2696, though it does not make any change to the display logic of the Payments line in the staff client. To test: 1. Check out an item with an issuing charge 2. Renew the item 3. Pay one of the rental charges on the item 4. Verify in koha.accountlines that each of the three above operations recorded your borrowernumber in accountlines.manager_id Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Files separate bug 6718 for writeoff problem.
Thank you, Katrin, for your thorough testing. The writeoff issue can be covered with a separate bug report. Marking this one as Passed QA.
Pushed, please test
I just tested and the manager_id is NULL (i've checked, the code is here). I added a manual debt => manager_id=1, it's OK I added a payement through "pay fine" > tested both pay amount and "pay" on the left of the debt line in both cases, I got manager_ID=NULL
a patch has been pushed, but it fixes only a part of the problem
I am removing the patch sent status as more work is needed on this bug.
This was broken again with the patch for bug 5713
Created attachment 7478 [details] [review] patch to populate manager_id and itemnumber when paying fines New patch to populate manager_id and itemnumber when paying a fine and also added the same for partial fines. Based on 3.6.0, so let me know if it doesn't apply on 3.6.x.
Does anyone else get the behavior that if you create three manual invoices and pay one, that the other two have their outstanding balance zeroed out?
Turns out that a writeoff uses a smallint value for the account no, the account no is used to clear the amountoustanding. The account no is supposed to increment, but writeoff assigns the maximum value to the smallint, it fails to increment, and the next accountline is assigned the same (maximum) value for account no. The next time you apply a payment, all of the accountlines with that account no receive the same processing. It's only a writeoff that affects this, and I am using 3.5, not sure if this manifests in 3.6.
Sorry to jump in on your bug, I am going to write the "Writeoff" issue up as a separate bug. Thatcher suggested it was similar to bug#3498, but I think it is different enough that it needs separate documentation.
Created attachment 7527 [details] [review] [SIGNED-OFF] Bug 6634 manager_id and itemnumber not populated when paying fines Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Tested successfully in master following this procedure (Comment 5): 1. Check out an item with an issuing charge 2. Renew the item 3. Pay one of the rental charges on the item 4. Verify in koha.accountlines that each of the three above operations recorded your borrowernumber in accountlines.manager_id manager_id was correctly populated for all transactions.
QA comment small patch, code is OK. Will push it, but my tests shows that there is one case that is still not covered: when paying fines with the button "Pay Selected" or "Pay Amount", the columns are still NULL (manager_id and itemnumber) When paying from "Pay" or "Pay Amount", it's OK though I see in the sub recordpayement that the same kind of fix should be made: my $usth = $dbh->prepare( "INSERT INTO accountlines (borrowernumber, accountno,date,amount,description,accounttype,amountoutstanding) VALUES (?,?,now(),?,'Payment,thanks','Pay',?)" The sub chargelostitem has also the same problem: my $sth2=$dbh->prepare("INSERT INTO accountlines (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber) VALUES (?,?,now(),?,?,'L',?,?)"); (itemnumber is OK, but manager_id is not)
The 1st patch has been pushed, status resetted to "assigned" to address the 2 other cases identified
Created attachment 8676 [details] [review] Fix 3 other cases of manager_id not populated Sorry for missing them at first and taking this long to send a new patch!
If there were still instances where this was broken I must not have tested all the possible cases. Can you describe what other operations need testing?
(In reply to comment #22) > If there were still instances where this was broken I must not have tested > all the possible cases. Can you describe what other operations need testing? (not sure you were answering my QA comment, but thinking so) I haven't tested, it's just that i've checked the code, and some "INSERT" clauses where lacking the manager_id & itemnumber parameters. Maybe that's some dead/unreachable code, but that's a code we have in recordpayement and chargelostitem subs
I have tested the payment of fines and Paul has verified in koha.accountlines.manager_id that each of the operations recorded the borrowernumber and the manager_id details. This patch can be signed off.
Adds managerid recording to recordpayment(), chargelostitem(), and recordpayment_selectaccts() subroutines. Marking Passed QA.
New follow-up pushed (it's not an ENH, chris_c, I think those patches should to ported to 3.8)
Pushed to 3.8.x, will be in 3.8.3