Description
Kyle M Hall (khall)
2018-04-17 18:07:57 UTC
Created attachment 74377 [details] [review] Bug 20598: Accruing fines not closed out by longoverdue.pl if WhenLostForgiveFine is not enabled Test Plan: 1) Ensure WhenLostForgiveFine is disabled 2) Create an overdue with a fine 3) Mark it lost with longoverdue.pl 4) Note it is still marked as an accruing fine 5) Apply this patch 6) Repeat steps 1-3 7) Note it is no longer an accruing fine! Created attachment 75932 [details] [review] Bug 20598: Accruing fines not closed out by longoverdue.pl if WhenLostForgiveFine is not enabled Test Plan: 1) Ensure WhenLostForgiveFine is disabled 2) Create an overdue with a fine 3) Mark it lost with longoverdue.pl 4) Note it is still marked as an accruing fine 5) Apply this patch 6) Repeat steps 1-3 7) Note it is no longer an accruing fine! Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and it works. Please provide tests to cover this change. Created attachment 81953 [details] [review] Bug 20598: Accruing fines not closed out by longoverdue.pl if WhenLostForgiveFine is not enabled Test Plan: 1) Ensure WhenLostForgiveFine is disabled 2) Create an overdue with a fine 3) Mark it lost with longoverdue.pl 4) Note it is still marked as an accruing fine 5) Apply this patch 6) Repeat steps 1-3 7) Note it is no longer an accruing fine! Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and it works. Created attachment 81954 [details] [review] Bug 20598: Unit test QA: Looking here Author: root <root@e7279b8b0511> kyle@bywatetsolutions.com bywatet ?? Created attachment 82391 [details] [review] Bug 20598: Accruing fines not closed out by longoverdue.pl if WhenLostForgiveFine is not enabled Test Plan: 1) Ensure WhenLostForgiveFine is disabled 2) Create an overdue with a fine 3) Mark it lost with longoverdue.pl 4) Note it is still marked as an accruing fine 5) Apply this patch 6) Repeat steps 1-3 7) Note it is no longer an accruing fine! Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and it works. Created attachment 82392 [details] [review] Bug 20598: Unit test Not ready yet; _FixOverduesOnReturn needs fixing ! (In reply to Marcel de Rooy from comment #11) > Not ready yet; _FixOverduesOnReturn needs fixing ! See bug 21849 about useless calls of Koha::Account::Offset->new. Circulation.t t::lib::Mocks::mock_preference('WhenLostForgiveFine','0'); t::lib::Mocks::mock_preference('WhenLostChargeReplacementFee','0'); [...] is( $total_due, '15.000000', 'Borrower only charged replacement fee with both WhenLostForgiveFine and WhenLostChargeReplacementFee enabled' ); [...] ok( $total_due == 15, 'Borrower only charged fine with both WhenLostForgiveFine and WhenLostChargeReplacementFee disabled' ); Hmm. Enabled? Replacement fee was zero ? Created attachment 82394 [details] [review] Bug 20598: Accruing fines not closed out by longoverdue.pl if WhenLostForgiveFine is not enabled Test Plan: 1) Ensure WhenLostForgiveFine is disabled 2) Create an overdue with a fine 3) Mark it lost with longoverdue.pl 4) Note it is still marked as an accruing fine 5) Apply this patch 6) Repeat steps 1-3 7) Note it is no longer an accruing fine! Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and it works. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 82395 [details] [review] Bug 20598: Unit test Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 82396 [details] [review] Bug 20598: (QA follow-up) Tiny fixes [1] Correct POD for _FixOverduesOnReturn Is called by AddReturn, AddRenewal and LostItem. Also tested in Circulation.t btw [2] $dbh is not used in _FixOverduesOnReturn [3] Moving all parameters to the first line. [4] Variable $uquery is not used too. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Passing QA. Tested a bit and works. Overall impression however is that things are handled a bit messy.Look at previous comments. Opened a new bug too. Awesome work all! Pushed to master for 18.11 backported to 18.05 for 18.05.07 Pushed to 17.11.x for 17.11.13 |