[Bug 11206] New: Factorize code for getting orders in C4::Acquisitions
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 Bug ID: 11206 Summary: Factorize code for getting orders in C4::Acquisitions Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: mathieu.saby@univ-rennes2.fr QA Contact: testopia@bugs.koha-community.org A lot of subs are used in C4::Acquisitions for doing basically the same thing : searching orders. Code can be factorized, to improve maintainability, build stronger unit tests, and make further improvements easier (like using DBIx::Class, or Bug 10758 ( Show bibliographic information of deleted records in acquisitions). First draft patch attached here. Fully testable, but can maybe be improved, and UT are not written yet. To test : make a full acquisition workflow, search orders, display uncertain prices, export baskets, basketgroups and late orders in CSV or PDF when possible. M. Saby -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |mathieu.saby@univ-rennes2.f |ity.org |r -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 --- Comment #1 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Created attachment 22747 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22747&action=edit [PATCH] Bug 11206 - Factorize code for getting orders in C4::Acquisitions This patch adds a new sub C4::Acquisition::_get_orders_with_params This sub is now called by - GetOrder - GetOrders - GetCancelledOrders - GetOrdersByBiblionumber - GetOrderFromItemnumber - SearchOrder It also makes changes to acqui/lateorders-export.pl, .../prog/en/modules/acqui/csv/lateorders.tt and .../prog/en/modules/acqui/lateorders.tt to take into account some variable renaming Next step : use it in GetLateOrders -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Mathieu, Not sure it is a good idea to factorize like that. Now, some routines did 1, 2 or 3 join, with your patch, they will always do 8 join. That will increase the load in acquisition pages. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 --- Comment #3 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- (In reply to Jonathan Druart from comment #2)
Mathieu, Not sure it is a good idea to factorize like that. Now, some routines did 1, 2 or 3 join, with your patch, they will always do 8 join. That will increase the load in acquisition pages.
In fact, what first disturbed me was that the same fields were not named in the same way in GetOrder and GetCancelledOrders and GetOrders (publishercode, rrp...). And working on that, I tried to factorize all the code. But you are right, in terms of performance it is not a good solution. Maybe the 2 subs which could be safely factorized are GetCancelledOrders and GetOrders, as they differ only by the exclusion of an order status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 --- Comment #4 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- In fact, the 1st step is adding UT to current master. Then, I will maybe work again on that bug, and more surely on bug 10758. Setting this one in discussion See Bug 11224 for new UTs Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 --- Comment #5 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 28192 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28192&action=edit Bug 12206: fix HTML in other holdings list in staff search results page To test: [1] View, in the staff interface, a record with alternate holdings (as controlled by the AlternateHoldingsField system preference). [2] Verify that the HTML list is closed correctly. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11206 --- Comment #6 from Chris Cormack <chris@bigballofwax.co.nz> --- The content of attachment 28192 has been deleted for the following reason: wrong bug -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org