Summary: | Factorize code for getting orders in C4::Acquisitions | ||
---|---|---|---|
Product: | Koha | Reporter: | Mathieu Saby <mathsabypro> |
Component: | Acquisitions | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED WONTFIX | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
[PATCH] Bug 11206 - Factorize code for getting orders in C4::Acquisitions
Bug 12206: fix HTML in other holdings list in staff search results page |
Description
Mathieu Saby
2013-11-06 09:28:28 UTC
Created attachment 22747 [details] [review] [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 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 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. 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 Created attachment 28192 [details] 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> The content of attachment 28192 [details] has been deleted for the following reason:
wrong bug
|