The method C4::Budgets::GetBudgetHierarchy() retreives from database budgets in an array containing a tree of budgets (parent -> children -> children, ...). The code generating this tree with the SQL results needs optimization because when a lot of budgets exists, it can run during several minutes. Have a look at code resume : while (1) { foreach my $r (@res) { foreach my $r2 (@res) { } } } With 1000 budgets there can be 1000x1000x1000 loops ...
Created attachment 25147 [details] [review] Bug 11714 - GetBudgetHierarchy needs optimization The method C4::Budgets::GetBudgetHierarchy() retreives from database budgets in an array containing a tree of budgets (parent -> children -> children, ...). The code generating this tree with the SQL results needs optimization because when a lot of budgets exists, it can run during several minutes. This patch rewites the code using a recurive method. Test plan : - Create a active budget "MyBudget" with 1000 - Click "Add found" on this budget - Create a found "Parent" with 1000, set you has owner - Click "Add child found" on found "Parent" - Create a found "Child" with 100, set you has owner - Click "Add child found" on found "Child" - Create a found "Grand-child" with 10, set you has owner | - Create a new acquisition basket - Add a new order with "Child budget" - Select "Child" found and set all costs to 2 - Save order - Add a new order with "Grand-Child budget" - Select "Child" found and set all costs to 2 - Save order - Close basket - Preform the receive of the two orders | - Go to founds of "MyBudget" => You see a table with 3 founds - in "Fund filters", select no library and uncheck "Show my funds only" and click on "Go" => You see a table with "Parent" found - Click on small arrow left of the fund code of "Parent" => You see a new line with "Child" found - Click on small arrow left of the fund code of "Child" => You see a new line with "Grand-Child" found | => You see in "Grand-Child" row "Base-level spent" = 2 and "Total sublevels spent" = 2 => You see in "Child" row "Base-level spent" = 2 and "Total sublevels spent" = 4 This confirms the founds are used in a hierarchie.
Created attachment 25305 [details] [review] [SIGNED-OFF] Bug 11714 - GetBudgetHierarchy needs optimization The method C4::Budgets::GetBudgetHierarchy() retreives from database budgets in an array containing a tree of budgets (parent -> children -> children, ...). The code generating this tree with the SQL results needs optimization because when a lot of budgets exists, it can run during several minutes. This patch rewites the code using a recurive method. Test plan : - Create a active budget "MyBudget" with 1000 - Click "Add found" on this budget - Create a found "Parent" with 1000, set you has owner - Click "Add child found" on found "Parent" - Create a found "Child" with 100, set you has owner - Click "Add child found" on found "Child" - Create a found "Grand-child" with 10, set you has owner | - Create a new acquisition basket - Add a new order with "Child budget" - Select "Child" found and set all costs to 2 - Save order - Add a new order with "Grand-Child budget" - Select "Child" found and set all costs to 2 - Save order - Close basket - Perform the receive of the two orders | - Go to founds of "MyBudget" => You see a table with 3 founds - in "Fund filters", select no library and uncheck "Show my funds only" and click on "Go" => You see a table with "Parent" found - Click on small arrow left of the fund code of "Parent" => You see a new line with "Child" found - Click on small arrow left of the fund code of "Child" => You see a new line with "Grand-Child" found | => You see in "Grand-Child" row "Base-level spent" = 2 and "Total sublevels spent" = 2 => You see in "Child" row "Base-level spent" = 2 and "Total sublevels spent" = 4 This confirms the founds are used in a hierarchie. I followed the test plan. The behavior was as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Please provide unit tests.
I don't reproduce the issue on master. Certainly fixed with bug 12168. *** This bug has been marked as a duplicate of bug 12168 ***
(In reply to Jonathan Druart from comment #4) > I don't reproduce the issue on master. Certainly fixed with bug 12168. > > *** This bug has been marked as a duplicate of bug 12168 *** Sorry, Fridolin wanted to optimize the routine, not fixing a bug.
Created attachment 29624 [details] [review] Bug 11714 - GetBudgetHierarchy needs optimization The method C4::Budgets::GetBudgetHierarchy() retreives from database budgets in an array containing a tree of budgets (parent -> children -> children, ...). The code generating this tree with the SQL results needs optimization because when a lot of budgets exists, it can run during several minutes. This patch rewites the code using a recurive method. Test plan : - Create a active budget "MyBudget" with 1000 - Click "Add found" on this budget - Create a found "Parent" with 1000, set you has owner - Click "Add child found" on found "Parent" - Create a found "Child" with 100, set you has owner - Click "Add child found" on found "Child" - Create a found "Grand-child" with 10, set you has owner | - Create a new acquisition basket - Add a new order with "Child budget" - Select "Child" found and set all costs to 2 - Save order - Add a new order with "Grand-Child budget" - Select "Child" found and set all costs to 2 - Save order - Close basket - Perform the receive of the two orders | - Go to founds of "MyBudget" => You see a table with 3 founds - in "Fund filters", select no library and uncheck "Show my funds only" and click on "Go" => You see a table with "Parent" found - Click on small arrow left of the fund code of "Parent" => You see a new line with "Child" found - Click on small arrow left of the fund code of "Child" => You see a new line with "Grand-Child" found | => You see in "Grand-Child" row "Base-level spent" = 2 and "Total sublevels spent" = 2 => You see in "Child" row "Base-level spent" = 2 and "Total sublevels spent" = 4 This confirms the founds are used in a hierarchie.
I rebased the patch and added some changes in order to fix conflicts with bug 12168 and bug 11578.
This is not a major anymore, back to normal.
I've applied the patch aginst master 3.17.00.014 The totals shown in the "Period allocated" row are wrong. Instead of being: 1,000.00 4.00 996.00 they are: 1,110.00 10.00 1,100.00 So I pass the patch to "Failed QA" status.
Created attachment 30925 [details] [review] Bug 11714: A fund is a top-level fund if it does not have any parent
I've applied the patches against master 3.17.00.014 I pass the patches to "Signed Off" status.
Created attachment 30942 [details] [review] Bug 11714 - GetBudgetHierarchy needs optimization
Created attachment 30943 [details] [review] Bug 11714: A fund is a top-level fund if it does not have any parent
Created attachment 31103 [details] [review] [PASSED QA] Bug 11714 - GetBudgetHierarchy needs optimization The method C4::Budgets::GetBudgetHierarchy() retreives from database budgets in an array containing a tree of budgets (parent -> children -> children, ...). The code generating this tree with the SQL results needs optimization because when a lot of budgets exists, it can run during several minutes. This patch rewites the code using a recurive method. Test plan : - Create a active budget "MyBudget" with 1000 - Click "Add found" on this budget - Create a found "Parent" with 1000, set you has owner - Click "Add child found" on found "Parent" - Create a found "Child" with 100, set you has owner - Click "Add child found" on found "Child" - Create a found "Grand-child" with 10, set you has owner | - Create a new acquisition basket - Add a new order with "Child budget" - Select "Child" found and set all costs to 2 - Save order - Add a new order with "Grand-Child budget" - Select "Child" found and set all costs to 2 - Save order - Close basket - Perform the receive of the two orders | - Go to founds of "MyBudget" => You see a table with 3 founds - in "Fund filters", select no library and uncheck "Show my funds only" and click on "Go" => You see a table with "Parent" found - Click on small arrow left of the fund code of "Parent" => You see a new line with "Child" found - Click on small arrow left of the fund code of "Child" => You see a new line with "Grand-Child" found | => You see in "Grand-Child" row "Base-level spent" = 2 and "Total sublevels spent" = 2 => You see in "Child" row "Base-level spent" = 2 and "Total sublevels spent" = 4 This confirms the founds are used in a hierarchie. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 31104 [details] [review] [PASSED QA] Bug 11714: A fund is a top-level fund if it does not have any parent Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patches pushed to master. Thanks Fridolin and Jonathan!