[Bug 22547] New: C4::Overdues - UpdateFine is untested
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Bug ID: 22547 Summary: C4::Overdues - UpdateFine is untested Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Target Milestone: --- UpdateFine is not tested at the unit level, it should be to help prevent regressions (and to allow more confidence in refactoring efforts). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 86782 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86782&action=edit Bug 22547: Add tests for C4::Overdues->UpdateFine Add tests for UpdateFine, in particular testing the functionality of MaxFine. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |katrin.fischer@bsz-bw.de, | |kyle@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Question here.. should I write the tests to test all the current behaviour or write them to test what I believe the behaviour should be.. Example questions raised: 1) Should we really be counting 'Sundry'/'Manual Invoice' in the total fines amount outstanding? 2) If the above is correct, should we be counting other fee's in the total fines amount outstanding calculation? 3) We don't explicitly order the accountlines lookup, as such we can't be sure we're actually updating the correct fine for the renewals case... Should I test for this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- That final one.. actually I don't think we record distinct fines after renewal any more.. this was an inadvertent change of functionality at some point I believe... maybe I'm reading the code wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86782|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 86790 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86790&action=edit Bug 22547: Add tests for C4::Overdues->UpdateFine Add tests for UpdateFine, in particular testing the functionality of MaxFine. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Test Plan: * Read the test and confirm it all makes sense.. run the test and confirm it passes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=10030 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22521 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22521 [Bug 22521] Convert fines handling to use 'status' instead of two accounttypes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|C4::Overdues - UpdateFine |C4::Overdues - UpdateFine |is untested |is barely tested -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86790|0 |1 is obsolete| | --- Comment #6 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 87051 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87051&action=edit Bug 22547: Add tests for C4::Overdues->UpdateFine Add tests for UpdateFine, in particular testing the functionality of MaxFine. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22593 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22593 [Bug 22593] Cronjobs/Scripts dealing with accountlines need updating for bug 22008 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|22593 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22593 [Bug 22593] Cronjobs/Scripts dealing with accountlines need updating for bug 22008 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |BLOCKED --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Martin, I am having some doubts about doing these kind of things in the test: $fine->amountoutstanding('50')->store; This obviously is a shortcut for doing the payment via 'nice' Koha calls, but I think it would be better to use the regular calls in order to find integration bugs and not give a false sense of security. I tested a bit with something like: t::lib::Mocks::mock_userenv({ branchcode => $patron->branchcode }); Koha::Account->new({ patron_id => $patron->borrowernumber })->pay({ amount => 50, lines => [ $fine ], }); But it needs more attention.. This is no absolute blocker btw. It is great to have more tests here. But if you could improve now, please do. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22564 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22564 [Bug 22564] accounttype 'Rep' is still referred to but is never set -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|22564 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22564 [Bug 22564] accounttype 'Rep' is still referred to but is never set -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm, I've been debating this a bit today. I think I was really aiming for a strict Unit Test here and as such wanted minimal reliance on thinks like the pay functions. I do agree, we really want to also have a level of integration tests but I'm not sure it's appropriate in this case. I'll ponder how/where to add such an integration tests too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Setting back to SO as I believe these tests are OK as is at the moment.. perhaps we could do further improvements and more formal integration tests down the line. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87051|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 87643 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87643&action=edit Bug 22547: Add tests for C4::Overdues->UpdateFine Add tests for UpdateFine, in particular testing the functionality of MaxFine. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 87644 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87644&action=edit Bug 22547: (QA follow-up) Warn in debug mode Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87643|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 88072 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88072&action=edit Bug 22547: Add tests for C4::Overdues->UpdateFine Add tests for UpdateFine, in particular testing the functionality of MaxFine. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87644|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 88073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88073&action=edit Bug 22547: (QA follow-up) Warn in debug mode Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.05.00 released in| | Status|Pushed to Master |RESOLVED Resolution|--- |FIXED --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Enhancement will not be backported to 18.11.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22547 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org