http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Francois Charbonnier <francois.charbonnier@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eric.begin@inLibro.com, | |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Francois Charbonnier <francois.charbonnier@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #2 from simith.doliveira@inlibro.com --- To test: Apply patch and update the database. Set up : On the item type administration page, default replacement price and a processing fee should be set up for on or more item types. When set up to 0, no processing fee will be charged to patrons for lost items. The "UseDefaultReplacementCost" system preference can be switched to « Use » or « Don't use ». If the lost item doesn't have a replacement cost catalogued, Koha will charge the default replacement cost. The "ProcessingFeeNote" system preference can be edited with the value to be used as a note alongside the processing fee when charged to the patron. The value will be displayed in the patron page fines tab. These tests apply when : * an item is manually marked as lost * an item is marked lost by the longoverdue.pl script Test scenario 1 : "UseDefaultReplacementCost" is set to « Use » and items.replacementcost = 0 * the item is manually marked lost 1. Set up the default replacement cost for a specific item type. 2. Set up the processing fee for this item type as well. 3. Search for an item of this item type and with no replacement cost. 4. Checkout the item to a patron. 5. Go to the item page. 6. Mark the item as lost. 7. Go to the patron page and open the fines tab. Check that : 8. there is a new entry for the item marked as lost. 9. the value match the default replacement cost for this item type. 10. there is another entry for the processing fee. 11. This entry should display the value from the « ProcessingFeeNote » system preference. Note : If you set up a processing fee to « 0 », there should be no entry for the processing fee. * the item is marked lost by the longoverdue.pl script 1. Set up the default replacement cost for a specific item type. 2. Set up the processing fee for this item type as well. 3. Search for an item of this item type and with no replacement cost. 4. Checkout the item to a patron specifying a 30 days late due date. 5. Run the script longoverdue.pl (/misc/cronjobs/) ./longoverdue.pl --lost 30=2 --charge 2 --confirm (for example) 6. Go to the item page. Check that : 7. the item is marked as lost. 8. Go to the patron page and open the fine tab. Check that : 9. there is a new entry for the item marked as lost. 10. the value match the default replacement cost for this item type. 11. there is another entry for the processing fee. 12. This entry should display the value from the « ProcessingFeeNote » system preference. Note : If you set up a processing fee to « 0 », there should be no entry for the processing fee. Test scenario 2 "UseDefaultReplacementCost" is set to « Use » and items.replacementcost > 0 * the item is manually marked lost 1. Set up the default replacement cost for a specific item type. 2. Set up the processing fee for this item type as well. 3. Search for an item of this item type and with a replacement cost. 4. Checkout the item to a patron. 5. Go to the item page. 6. Mark the item lost. 7. Go to the patron page and open the fine tab. Check that : 8. there is a new entry for the item marked as lost. 9. the value match the item replacement cost (it's Koha's default behaviour). 10. there is another entry for the processing fee.. This entry should display the value from the « ProcessingFeeNote » system preference. Note : If you set up a processing fee to « 0 », there should be no entry for the processing fee. * the item is marked lost by the longoverdue.pl script 1. Set up the default replacement cost for a specific item type. 2. Set up the processing fee for this item type as well. 3. Search for an item of this item type and with a replacement cost. 4. Checkout the item to a patron specifying a 30 days late due date. 5. Run the script longoverdue.pl (/misc/cronjobs/) ./longoverdue.pl --lost 30=2 --charge 2 --confirm (for example) 6. Go to the item page Check that : 7. the item is marked as lost. 8. Go to the patron page and open the fine tab. Check that : 9. there is a new entry for the item marked as lost. 10 the value match the item replacement cost (it's Koha's default behaviour). 11. there is another entry for the processing fee. This entry should display the value from the « ProcessingFeeNote » system preference. Note : If you set up a processing fee to « 0 », there should be no entry for the processing fee. Test scenario 3 : "UseDefaultReplacementCost" is set to « Don't use » Koha's default behaviour apply. It means : If items.replacementcost = 0 : The default replacement cost value for the item type will not be used. The processing fee (if greater than 0) will be charged. If items.replacement >0. The item replacement cost will be used. The processing fee (if greater than 0) will be charged. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34334|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Peggy Thrasher <p.thrasher@dover.nh.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |p.thrasher@dover.nh.gov -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |charles.farmer@inlibro.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com --- Comment #6 from Nicole C. Engard <nengard@gmail.com> --- I want to sign off on this - but can only test the staff client part of things. Everything there works great!! Maybe someone else could test the long over due cron part? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #8 from Charles Farmer <charles.farmer@inlibro.com> --- I read _insert_accountlines again and adapted manualinvoice to take its place. I also removed the *return* instruction at the end of chargelostitem() because, as you said, it wasn't used anywhere (except once in ChargeLostItem.t) Thank you for your time! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36505|0 |1 is obsolete| | --- Comment #10 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 36506 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36506&action=edit Bug 12768 - Replacement cost and processing fee management -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Charles Farmer from comment #8)

I read _insert_accountlines again and adapted manualinvoice to take its place. I also removed the *return* instruction at the end of chargelostitem() because, as you said, it wasn't used anywhere (except once in ChargeLostItem.t)

Thank you for your time!

Thanks for those fixes! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36506|0 |1 is obsolete| | --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 36552 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36552&action=edit Bug 12768 [QA Followup] - Fix unit tests -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 36554 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36554&action=edit [SIGNED-OFF] Bug 12768 - Replacement cost and processing fee management Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Signed Off |Failed QA Keywords|Academy | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Simith, starting with a code review here: Results of the QA script: FAIL C4/Accounts.pm OK critic FAIL forbidden patterns forbidden pattern: tab char (line 404) forbidden pattern: tab char (line 410) forbidden pattern: tab char (line 409) forbidden pattern: tab char (line 469) forbidden pattern: tab char (line 408) forbidden pattern: tab char (line 407) forbidden pattern: tab char (line 406) OK pod OK valid Also some tests seem to be failing: Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 512 Tests: 60 Failed: 2) Failed tests: 43-44 Non-zero exit status: 2 'Manual' code review: 1) Don't use double as the type, but decimal, as we are already using decimal in all other places where amounts are stored and double should not be used to store monetary values: + defaultreplacecost double(16,4) default NULL, -- default replacement cost + processfee double(16,4) default NULL, -- default text be recorded in the column note when the processing fee is applied + updatedatabase.pl as well (blocker) 2) Please amend POD of manualinvoice with the new type PF and the new parameter. Can you explain what notifyid is used for? (normal) 3) Spotting a translation problem here: + C4::Accounts::chargelostitem($issues, "Lost Item $issues->{'title'} $issues->{'barcode'}"); ... but it was there before your patch. 4) Please include the updated schema files in a separate patch file. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36554|0 |1 is obsolete| | --- Comment #17 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 38365 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38365&action=edit Bug 12768 - Replacement cost and processing fee management -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36555|0 |1 is obsolete| | --- Comment #20 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 38375 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38375&action=edit Bug 12768 [QA Followup] - Fix unit tests -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The tests pass ok for me on current master - can you try again on a database with your patch and all the sample data installed? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38367|0 |1 is obsolete| | --- Comment #25 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39298 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39298&action=edit Bug 12768 - Updated schema files -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38375|0 |1 is obsolete| | --- Comment #27 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39300 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39300&action=edit Bug 12768 [QA Followup] - Fix unit tests -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #29 from Eivin Giske Skaaren <eskaaren@yahoo.no> --- Test results: After applying the patch I was unable to set "Charge", "Default replacement cost" or "Processing fee" for any item. When clicking save the values were not changed but stayed 0.00. I checked out master again to verify and I could then change the value for "Charge". After manually altering the table with the commands in a/installer/data/mysql/atomicupdate/bug_12768-update_schema_files.sql I got it to work. This may have been a local problem with my devbox. Test scenario 1: When setting to "lost" manually this seems to work as advertised. But as a side note: if changing the status for the item from "lost" back to "choose" the fines stays put. So by setting an item to "lost" by mistake and then changing back the fines must also be manually handled. When using the longoverdue.pl script the item is marked as Long overdue (lost) but there is no fine shown for the patron. Test scenario 2: Manual work as advertised. When using the longoverdue.pl script the result is the same as in scenario 1. There is no fine. Perhaps someone else could test the part using the script? If this works for them there might be an issue with my dev env and I would be happy to sign off. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #30 from Maxime Beaulieu <maxime.beaulieu@inlibro.com> --- (In reply to Eivin Giske Skaaren from comment #29)

After applying the patch I was unable to set "Charge", "Default replacement cost" or "Processing fee" for any item. After manually altering the table with the commands in a/installer/data/mysql/atomicupdate/bug_12768-update_schema_files.sql

The update worked fine on my end and I was able to edit the values. Have you run the installer/data/mysql/updatedatabase.pl script?

When using the longoverdue.pl script the item is marked as Long overdue (lost) but there is no fine shown for the patron.

When using the longoverdue.pl script the result is the same as in scenario 1. There is no fine.

Did you run longoverdue.pl with the -c option? --charge | -c This specifies what lost value triggers Koha to charge the account for the lost item. Replacement costs are not charged if this is not specified. Also, the patron can not be charged multiple times for losing the same item. Even if you writeoff / pay the fine, it's still there in the DB. C4/Accounts.pm 368 # first make sure the borrower hasn't already been charged for this item 369 my $sth1=$dbh->prepare("SELECT * from accountlines 370 WHERE borrowernumber=? AND itemnumber=? and accounttype='L'"); You will need to remove the lost status from the item and run something like this in MySQL if you want to test multiple times with the same borrower and item combination. DELETE from accountlines WHERE borrowernumber=#BORROWERNUMBER# AND itemnumber=#ITEMNUMBER# and accounttype='L'; -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Eivin Giske Skaaren <eskaaren@yahoo.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39300|0 |1 is obsolete| | --- Comment #32 from Eivin Giske Skaaren <eskaaren@yahoo.no> --- Created attachment 39999 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39999&action=edit Bug 12768 [QA Followup] - Fix unit tests Signed-off-by: Eivin Giske Skaaren <eivin@sysmystic.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Eivin Giske Skaaren <eskaaren@yahoo.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39299|0 |1 is obsolete| | --- Comment #34 from Eivin Giske Skaaren <eskaaren@yahoo.no> --- Created attachment 40001 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40001&action=edit Bug 12768 - Replacement cost and processing fee management Signed-off-by: Eivin Giske Skaaren <eivin@sysmystic.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Eivin Giske Skaaren <eskaaren@yahoo.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 42158 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42158&action=edit Bug 12768: Use the Price TT plugin to display prices -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #39 from Peggy Thrasher <p.thrasher@dover.nh.gov> --- It may be out of scope for this bug, but it would be great if this could be set on a per branch basis. My consortium includes a public library (which does charge a processing fee, and school libraries (who do not). Thanks for considering this Peggy -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Eivin Giske Skaaren <eivin@sysmystic.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #41 from Eivin Giske Skaaren <eivin@sysmystic.com> --- I am setting this to signed off again even though there are no tests written for the C4::Circulation::LostItem subroutine. The test seems to need to mock several tables and someone experienced writing that type of tests for Koha should probably do it if it really is needed. I have tested thoroughly manually in GUI, console and mysql client. Peggy: This patch has already consumed considerable time and perhaps it is better to not increase its scope at this stage. My suggestion is that you file a new bug with the wanted enhancements and refer to this one. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #43 from Eivin Giske Skaaren <eivin@sysmystic.com> --- Hi Jonathan, Perhaps a clarification: Yes I did look at that example but to develop tests for C4::Circulation::LostItem seemed more complex. Anyway I am not the original dev of this patch and will need to talk to the sponsor before putting more time into it. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #44 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 43546 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43546&action=edit Bug 12768: Fixed tests 44 and 45 of t/db_dependant/Circulation.t -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 --- Comment #46 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Charles Farmer from comment #45)

(In reply to Jonathan Druart from comment #38)

...

It does not seem to work if item-level_itypes is set to biblio. You should also provide tests for C4::Circulation::LostItem (I suspect that the problem comes from the query executed in this subroutine).

When you say "provide tests for LostItem", do you mean that a separate file should be written under t/db_dependent/Circulation/ for that purpose, or would you accept if the tests were added to everything else in Circulation.t ?

The last patch could do the job, if the itemtype type you used (BK) was created (tests should create their own data). But thinking about it, I am wondering why you pass the processfee to chargelostitem and don't retrieve it in the chargelostitem subroutine. It also a bit weird to pass an issue, the name of the sub is chargelost*item* :) Kyle, could I get your opinion on that please? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle.m.hall@gmail.com --- Comment #48 from Kyle M Hall <kyle.m.hall@gmail.com> ---

Kyle, could I get your opinion on that please?

I agree. This change makes the LostItem far more complicated. Using the plural form "issues" adds even more confusion. Please restore the prototype for LostItem to its original form and fetch the processing fee from the itemtype within chargelostitem. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12768 Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.hedstrom.mace@sub.s | |u.se -- You are receiving this mail because: You are watching all bug changes.