[Bug 20562] New: issue_id is not stored in accountlines for rental fees
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Bug ID: 20562 Summary: issue_id is not stored in accountlines for rental fees Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org To recreate: 1 - Assign a charge to an item type 2 - Checkout an item of that type to a patron 3 - View the accountlines table for that patron SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##; 4 - Note there is no issue_id -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|kyle@bywatersolutions.com |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 74019 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74019&action=edit Bug 20562: issue_id is not stored in accountlines for rental fees To recreate: 1 - Assign a charge to an item type 2 - Checkout an item of that type to a patron 3 - View the accountlines table for that patron SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##; 4 - Note there is no issue_id Test Plan: 1) Apply this patch 2) prove t/db_dependent/Circulation/issue.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nick@bywatersolutions.com |kyle@bywatersolutions.com CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74019|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 74020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74020&action=edit Bug 20562: issue_id is not stored in accountlines for rental fees To recreate: 1 - Assign a charge to an item type 2 - Checkout an item of that type to a patron 3 - View the accountlines table for that patron SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##; 4 - Note there is no issue_id Test Plan: 1) Apply this patch 2) prove t/db_dependent/Circulation/issue.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74020|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 74021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74021&action=edit Bug 20562: issue_id is not stored in accountlines for rental fees Test Plan: 1) Apply this patch 2) Assign a charge to an item type 3) Checkout an item of that type to a patron 4) View the accountlines table for that patron SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##; 5) Note there is an issue_id Or 1) Apply this patch 2) prove t/db_dependent/Circulation/issue.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Is it a regression of it never worked? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #4)
Is it a regression of it never worked?
I would think not, but I suppose you could argue it either way philosophically. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20572 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20572 [Bug 20572] Refactor AddIssuingCharges parameter to hashref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 M. Tompsett <mtompset@hotmail.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=20562 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74021|0 |1 is obsolete| | --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 74102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74102&action=edit Bug 20562: issue_id is not stored in accountlines for rental fees Test Plan: 1) Apply this patch 2) Assign a charge to an item type 3) Checkout an item of that type to a patron 4) View the accountlines table for that patron SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##; 5) Note there is an issue_id Or 1) Apply this patch 2) prove t/db_dependent/Circulation/issue.t Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |josef.moravec@gmail.com |y.org | CC| |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=20562 Josef Moravec <josef.moravec@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=20562 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74102|0 |1 is obsolete| | --- Comment #7 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 74240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74240&action=edit Bug 20562: issue_id is not stored in accountlines for rental fees Test Plan: 1) Apply this patch 2) Assign a charge to an item type 3) Checkout an item of that type to a patron 4) View the accountlines table for that patron SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##; 5) Note there is an issue_id Or 1) Apply this patch 2) prove t/db_dependent/Circulation/issue.t Signed-off-by: Mark Tompsett <mtompset@hotmail.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=20562 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 74250 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74250&action=edit Bug 20562: Pass the Koha::Checkout object to AddIssuingCharge We do not need to pass all those parameters, just the checkout object is enough. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Could you take a look at the follow-up? It does not seem useful to add one more parameter when it's possible to remove one :) See bug 20591 to understand why I removed the DBI->last_insert_id call -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20591 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 --- Comment #10 from Josef Moravec <josef.moravec@gmail.com> --- Comment on attachment 74250 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74250 Bug 20562: Pass the Koha::Checkout object to AddIssuingCharge Review of attachment 74250: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20562&attachment=74250) ----------------------------------------------------------------- Reading the code - does make sense. Testing - does work. Thanks Jonathan! ::: C4/Circulation.pm @@ +3174,2 @@
+ # FIXME What if checkout does not exist?
Then we should throw an exception. I think this will need some refactoring during the move to Koha::Objects in future. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74250|0 |1 is obsolete| | --- Comment #11 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 74253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74253&action=edit Bug 20562: Pass the Koha::Checkout object to AddIssuingCharge We do not need to pass all those parameters, just the checkout object is enough. 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=20562 Josef Moravec <josef.moravec@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=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 18.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- commit dc5d3b9abd34b0e80794fff7029809b7438bcefa Bug 20562: issue_id is not stored in accountlines for rental fees breaks t/db_dependent/Circulation/Returns.t # Failed test 'No account lines should exist for this issue_id, patron should not have been charged' # at t/db_dependent/Circulation/Returns.t line 330. # got: '1' # expected: '0' # Looks like you failed 1 test of 6. t/db_dependent/Circulation/Returns.t .. 4/5 # Failed test 'Handle ids duplication' # at t/db_dependent/Circulation/Returns.t line 333. # Looks like you failed 1 test of 5. Please fix ASAP -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #13)
commit dc5d3b9abd34b0e80794fff7029809b7438bcefa Bug 20562: issue_id is not stored in accountlines for rental fees
breaks t/db_dependent/Circulation/Returns.t
# Failed test 'No account lines should exist for this issue_id, patron should not have been charged' # at t/db_dependent/Circulation/Returns.t line 330. # got: '1' # expected: '0' # Looks like you failed 1 test of 6. t/db_dependent/Circulation/Returns.t .. 4/5 # Failed test 'Handle ids duplication' # at t/db_dependent/Circulation/Returns.t line 333. # Looks like you failed 1 test of 5.
Please fix ASAP
I'm trying to understand what this is actually testing. Can you give me any context? The unit tests aren't telling me much. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Ah, the unit test assumed that his was implemented already! That still doesn't explain the expected behavior of the test. It seems like that particular test should expect an accountline to exist and *not* for none to exist! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74240|0 |1 is obsolete| | Attachment #74253|0 |1 is obsolete| | --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 74584 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74584&action=edit Bug 20562: Fix unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74584|0 |1 is obsolete| | --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 74586 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74586&action=edit Bug 20562: Fix Returns.t test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74253|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74240|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Pushed to stable for 17.11.6 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #19 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Patches does not apply on 17.05.x. Since its an important impact, I prefer not to rebase myself. Could you provide rebased patches ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20562 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org