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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/C4/Budgets.pm b/C4/Budgets.pm
index 0cd7c93..ccc3d37 100644 (file)
--- a/C4/Budgets.pm
+++ b/C4/Budgets.pm
@@ -520,72 +520,36 @@ sub GetBudgetHierarchy {
        $debug && warn $query,join(",",@bind_params);
        my $sth = $dbh->prepare($query);
        $sth->execute(@bind_params);
-       my $results = $sth->fetchall_arrayref({});
-       my @res     = @$results;
-       my $i = 0;
-       while (1) {
-               my $depth_cnt = 0;
-               foreach my $r (@res) {
-                       my @child;
-                       # look for children
-                       $r->{depth} = '0' if !defined $r->{budget_parent_id};
-                       foreach my $r2 (@res) {
-                               if (defined $r2->{budget_parent_id}
-                                       && $r2->{budget_parent_id} == $r->{budget_id}) {
-                                       push @child, $r2->{budget_id};
-                                       $r2->{depth} = ($r->{depth} + 1) if defined $r->{depth};
-                               }
-                       }
-                       $r->{child} = \@child if scalar @child > 0;    # add the child
-                       $depth_cnt++ if !defined $r->{'depth'};
-               }
-               last if ($depth_cnt == 0 || $i == 100);
-               $i++;
-       }
 
-       # look for top parents 1st
-       my (@sort, $depth_count);
-       ($i, $depth_count) = 0;
-       while (1) {
-               my $children = 0;
-               foreach my $r (@res) {
-                       if ($r->{depth} == $depth_count) {
-                               $children++ if (ref $r->{child} eq 'ARRAY');
-
-                               # find the parent id element_id and insert it after
-                               my $i2 = 0;
-                               my $parent;
-                               if ($depth_count > 0) {
-
-                                       # add indent
-                                       my $depth = $r->{depth} * 2;
-                                       $r->{budget_code_indent} = $r->{budget_code};
-                                       $r->{budget_name_indent} = $r->{budget_name};
-                                       foreach my $r3 (@sort) {
-                                               if ($r3->{budget_id} == $r->{budget_parent_id}) {
-                                                       $parent = $i2;
-                                                       last;
-                                               }
-                                               $i2++;
-                                       }
-                               } else {
-                                       $r->{budget_code_indent} = $r->{budget_code};
-                                       $r->{budget_name_indent} = $r->{budget_name};
-                               }
-                
-                               if (defined $parent) {
-                                       splice @sort, ($parent + 1), 0, $r;
-                               } else {
-                                       push @sort, $r;
-                               }
-                       }
-
-                       $i++;
-               }    # --------------foreach
-               $depth_count++;
-               last if $children == 0;
-       }
+    my %links;
+    # create hash with budget_id has key
+    while ( my $data = $sth->fetchrow_hashref ) {
+        $links{ $data->{'budget_id'} } = $data;
+    }
 
+    # link child to parent
+    my @first_parents;
+    foreach ( sort keys %links ) {
+        my $child = $links{$_};
+        if ( $child->{'budget_parent_id'} ) {
+            my $parent = $links{ $child->{'budget_parent_id'} };
+            if ($parent) {
+                unless ( $parent->{'children'} ) {
+                    # init child arrayref
+                    $parent->{'children'} = [];
+                }
+                # add as child
+                push @{ $parent->{'children'} }, $child;
+            }
+        } else {
+            push @first_parents, $child;
+        }
+    }
+
+    my @sort = ();
+    foreach my $first_parent (@first_parents) {
+        _add_budget_children(\@sort, $first_parent);
+    }
 
     foreach my $budget (@sort) {
         $budget->{budget_spent}   = GetBudgetSpent( $budget->{budget_id} );
@@ -596,6 +560,18 @@ sub GetBudgetHierarchy {
     return \@sort;
 }
 
+# Recursive method to add a budget and its chidren to an array
+sub _add_budget_children {
+    my $res = shift;
+    my $budget = shift;
+    push @$res, $budget;
+    my $children = $budget->{'children'} || [];
+    return unless @$children; # break recursivity
+    foreach my $child (@$children) {
+        _add_budget_children($res, $child);
+    }
+}
+
 # -------------------------------------------------------------------
 
 sub AddBudget {

diff --git a/acqui/acqui-home.pl b/acqui/acqui-home.pl
index 3481745..3231e62 100755 (executable)
--- a/acqui/acqui-home.pl
+++ b/acqui/acqui-home.pl
@@ -74,8 +74,6 @@ my @budget_loop;
 foreach my $budget ( @{$budget_arr} ) {
     next unless (CanUserUseBudget($loggedinuser, $budget, $userflags));
 
-    $budget->{budget_code_indent} =~ s/\ /\&nbsp\;/g;
-
     $budget->{'budget_branchname'} =
       GetBranchName( $budget->{'budget_branchcode'} );
 

diff --git a/admin/aqbudgets.pl b/admin/aqbudgets.pl
index 6046387..8b5a690 100755 (executable)
--- a/admin/aqbudgets.pl
+++ b/admin/aqbudgets.pl
@@ -139,8 +139,7 @@ if ($op eq 'add_form') {
     my @values;
     my $hier = GetBudgetHierarchy($$period{budget_period_id});
     foreach my $r (@$hier) {
-        $r->{budget_code_indent} =~ s/ /\~/g;    #
-        $labels{"$r->{budget_id}"} = $r->{budget_code_indent};
+        $labels{"$r->{budget_id}"} = $r->{budget_code};
         push @values, $r->{budget_id};
     }
     push @values, '';

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
index d0f8bdb..b97d0d8 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
@@ -315,7 +315,7 @@ var MSG_PARENT_BENEATH_BUDGET = "- " + _("New budget-parent is beneath budget")
     [% END %]
     <td>[% budget.budget_period_active %]</td>
     <td>Budget [% budget.budget_period_description %] [id=[% budget.budget_period_id %]][% UNLESS budget.budget_period_active %] (inactive)[% END %]</td>
-    <td>[% budget.budget_code_indent %]</td>
+    <td>[% budget.budget_code %]</td>
     <td>[% budget.budget_name %]</td>
     <td class="data">
       [% IF budget.budget_parent_id %]