[Bug 20728] New: Remove subroutines GetLastOrder*
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Bug ID: 20728 Summary: Remove subroutines GetLastOrder* Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 20276 There are 2 subroutines from C4::Acquisition that could be removed: - GetLastOrderReceivedFromSubscriptionid - GetLastOrderNotReceivedFromSubscriptionid After bug 20726 only GetLastOrderReceivedFromSubscriptionid will be used (from acqui/neworderempty.pl) and this call could be replaced easily with Koha::Acquisition::Orders The code (+ tests) related to these 2 subroutines could then be removed. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20276 [Bug 20276] GetCourseItem is using the wrong call to get itemnumber -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|20276 |20726 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20276 [Bug 20276] GetCourseItem is using the wrong call to get itemnumber https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20726 [Bug 20726] Display acquisition details on the subscription detail page -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Bug 20728 depends on bug 20726, which changed state. Bug 20726 Summary: Display acquisition details on the subscription detail page https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20726 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=20728 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=20728 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 86693 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86693&action=edit Bug 20728: Proof that there is no cheating -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 86694 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86694&action=edit Bug 20728: Replace the calls by their Koha::Acq::Orders->search equivalent -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 86695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86695&action=edit Bug 20728: Remove the 2 GetLastOrder* subroutines -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86693|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 99655 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99655&action=edit Bug 20728: Proof that there is no cheating 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=20728 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86694|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 99656 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99656&action=edit Bug 20728: Replace the calls by their Koha::Acq::Orders->search equivalent 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=20728 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86695|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 99657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99657&action=edit Bug 20728: Remove the 2 GetLastOrder* subroutines 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=20728 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Signed Off --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Works as expected without regressions. I'm not entirely sure I like that we're moving from a tested method to an untested controller, but I'm struggling to come up with a better alternative. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #7)
Works as expected without regressions.
I'm not entirely sure I like that we're moving from a tested method to an untested controller, but I'm struggling to come up with a better alternative.
Signing off
1 join? :) I agree it can be discussed. But the parameters for the search is basic, IMO it does not deserve its own subroutine. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |katrin.fischer@bsz-bw.de --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Please fix the commit messages :( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #9)
Please fix the commit messages :(
What do you mean? All of them explain what they are doing. With comment 0 and the 3 commit titles, everything is meaningful to me. I can explain a bit more if you want me to clarify something. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
From 0445ff47aea8cbe01e5fbb212aebe4885b65deb3 Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Fri, 15 Mar 2019 19:24:19 -0300 Subject: [PATCH] Bug 20728: Proof that there is no cheating
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> The don't have descriptions or test plans. And I already failed to understand the first one from the subject. comment #0 shoudl be in one of the commit messages. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99655|0 |1 is obsolete| | Attachment #99656|0 |1 is obsolete| | Attachment #99657|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 99877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99877&action=edit Bug 20728: Proof that there is no cheating To make sure the replacing code will acchieve the same things as the actual one, we replace the raw SQL query with the DBIC version of it. Then the tests will show us that they are equivalent. Test plan: Apply only this patch, run the tests, confirm they pass. 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=20728 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 99878 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99878&action=edit Bug 20728: Replace the calls by their Koha::Acq::Orders->search equivalent There are 2 subroutines from C4::Acquisition that could be removed: - GetLastOrderReceivedFromSubscriptionid - GetLastOrderNotReceivedFromSubscriptionid After bug 20726 only GetLastOrderReceivedFromSubscriptionid will be used (from acqui/neworderempty.pl) and this call could be replaced easily with Koha::Acquisition::Orders The code (+ tests) related to these 2 subroutines could then be removed. The parameters for the search is basic and does no really deserve its own subroutine. 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=20728 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 99879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99879&action=edit Bug 20728: Remove the 2 GetLastOrder* subroutines At this point the 2 subroutines are no longer in used. 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=20728 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I have adjusted the commit messages, hope it's better now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=20728 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99877|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102873&action=edit Bug 20728: Proof that there is no cheating To make sure the replacing code will acchieve the same things as the actual one, we replace the raw SQL query with the DBIC version of it. Then the tests will show us that they are equivalent. Test plan: Apply only this patch, run the tests, confirm they pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99878|0 |1 is obsolete| | --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102874 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102874&action=edit Bug 20728: Replace the calls by their Koha::Acq::Orders->search equivalent There are 2 subroutines from C4::Acquisition that could be removed: - GetLastOrderReceivedFromSubscriptionid - GetLastOrderNotReceivedFromSubscriptionid After bug 20726 only GetLastOrderReceivedFromSubscriptionid will be used (from acqui/neworderempty.pl) and this call could be replaced easily with Koha::Acquisition::Orders The code (+ tests) related to these 2 subroutines could then be removed. The parameters for the search is basic and does no really deserve its own subroutine. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99879|0 |1 is obsolete| | --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102875 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102875&action=edit Bug 20728: Remove the 2 GetLastOrder* subroutines At this point the 2 subroutines are no longer in used. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20728 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #20 from Joy Nelson <joy@bywatersolutions.com> --- not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org