[Bug 21681] New: Remove C4::Accounts::getcharges
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Bug ID: 21681 Summary: Remove C4::Accounts::getcharges Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: josef.moravec@gmail.com QA Contact: testopia@bugs.koha-community.org C4::Accounts::getcharges is only used on one place and could be easily rewritten using objects -- 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=21681 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=21681 --- Comment #1 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81269 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81269&action=edit Bug 21681: Remove C4::Accounts::getcharges Test plan: 0) Do not apply the patch 1) Enable ILS-DI 2) Go to <opac_url>/cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_fines=1 use patron with some paid and unpaid fines 3) Save the page for later usage 4) Apply the patch 5) Repeat 2 with same patron and compare the saved result with the new one, they should be same 6) git grep getcharges should return no occurance 7) prove t/db_dependent/Accounts.t -- 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=21681 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |josef.moravec@gmail.com |ity.org | Blocks| |14825 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14825 [Bug 14825] Accounts Rewrite Omnibus -- 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=21681 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21683 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21683 [Bug 21683] Remove accountlines.accountno -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81269|0 |1 is obsolete| | --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81298 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81298&action=edit Bug 21681: Remove C4::Accounts::getcharges Test plan: 0) Do not apply the patch 1) Enable ILS-DI 2) Go to <opac_url>/cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_fines=1 use patron with some paid and unpaid fines 3) Save the page for later usage 4) Apply the patch 5) Repeat 2 with same patron and compare the saved result with the new one, they should be same 6) git grep getcharges should return no occurance 7) prove t/db_dependent/Accounts.t 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=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Signed Off Severity|enhancement |trivial --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Simples.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- With my QA hat on.. I'm less happy with this. 1) We should not use Koha::Account::Lines directly.. the intention is to use Koha::Account as an intermediary for dealing with all account actions. 2) If we do the above, there is a method directly from the Koha::Patron object you have in ILSDI already ($patron->account->*) 3) I don't really believe ILSDI should return ALL accountlines associated to a patron. In reality, the intention is for outstanding debits (and possibly outstanding credits) to be usable. However, that would be a change of the api, and so is probably out of scope. As such, we could add a 'lines' method to Koha::Account which would return all accountlines filtered by the user. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm working on the followup myself -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81306 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81306&action=edit Bug 21681: (follow-up) Don't use Koha::Account::Line directly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #7 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Martin Renvoize from comment #5)
I'm working on the followup myself
Oh, you were really fast! Thanks! I agreee with you, that it does not make much sense to return all accountlines in getpatroninfo method of ilsdi, but it is definetaly out of scope of this report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I just need to add a test for Koha::Account->lines now then i'll get Tomas to do a final QA on it ;) Be really nice to clean up this area. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81322 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81322&action=edit Bug 21681: (follow-up) Add tests for Koha::Account->lines -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=21681 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Depends on| |21694 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I will move the 2 last patches to their own bug report, see bug 21694 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21694 [Bug 21694] Add the Koha::Account->lines method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81306|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=21681 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81322|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=21681 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81298|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 81325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81325&action=edit Bug 21681: Remove C4::Accounts::getcharges Test plan: 0) Do not apply the patch 1) Enable ILS-DI 2) Go to <opac_url>/cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_fines=1 use patron with some paid and unpaid fines 3) Save the page for later usage 4) Apply the patch 5) Repeat 2 with same patron and compare the saved result with the new one, they should be same 6) git grep getcharges should return no occurance 7) prove t/db_dependent/Accounts.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 81326 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81326&action=edit Bug 21681: Add tests for GetPatronInfo Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 81327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81327&action=edit Bug 21681: Improve the account lines fetch Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I have squashed the first 2 patches (wrong manip), sorry Martin! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Loving the code golf here.. nice teamwork! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Yup, tested and all works as expected still, code looks great after that golfing session and Jonthans added ILSDI tests too.. can't ask for more.. lets PQA it as there's now three QA team eyes here ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81325|0 |1 is obsolete| | --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81329 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81329&action=edit Bug 21681: Remove C4::Accounts::getcharges Test plan: 0) Do not apply the patch 1) Enable ILS-DI 2) Go to <opac_url>/cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_fines=1 use patron with some paid and unpaid fines 3) Save the page for later usage 4) Apply the patch 5) Repeat 2 with same patron and compare the saved result with the new one, they should be same 6) git grep getcharges should return no occurance 7) prove t/db_dependent/Accounts.t Co-authored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81326|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81330&action=edit Bug 21681: Add tests for GetPatronInfo Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81327|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81331&action=edit Bug 21681: Improve the account lines fetch Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=21681 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21696 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21696 [Bug 21696] Use Koha::Account->lines from Koha::Account -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|21696 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21696 [Bug 21696] Use Koha::Account->lines from Koha::Account -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21696 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21696 [Bug 21696] Use Koha::Account->lines from Koha::Account -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 18.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Bug 21681 depends on bug 21694, which changed state. Bug 21694 Summary: Add the Koha::Account->lines method https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21694 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Fines and fees |Architecture, internals, | |and plumbing Severity|trivial |enhancement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Master |RESOLVED --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Architectural enhancement, will not be backported to 18.05.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21804 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21681 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|14825 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14825 [Bug 14825] Accounts Rewrite Omnibus -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org