From 44ef2cd45bbed15260d46d7ea7f23b3b5c099095 Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Fri, 7 Feb 2014 17:59:01 +0100 Subject: [PATCH] [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 Signed-off-by: Kyle M Hall --- C4/Budgets.pm | 104 ++++++++------------ acqui/acqui-home.pl | 2 - admin/aqbudgets.pl | 3 +- .../prog/en/modules/admin/aqbudgets.tt | 2 +- 4 files changed, 42 insertions(+), 69 deletions(-) diff --git a/C4/Budgets.pm b/C4/Budgets.pm index e0e8f72..935e8d5 100644 --- a/C4/Budgets.pm +++ b/C4/Budgets.pm @@ -499,72 +499,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} ); @@ -575,6 +539,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 8cffc26..349a9cd 100755 --- a/acqui/acqui-home.pl +++ b/acqui/acqui-home.pl @@ -95,8 +95,6 @@ my @budget_loop; foreach my $budget ( @{$budget_arr} ) { next unless (CanUserUseBudget($loggedinuser, $budget, $userflags)); - $budget->{budget_code_indent} =~ s/\ /\ \;/g; - $budget->{'budget_branchname'} = GetBranchName( $budget->{'budget_branchcode'} ); diff --git a/admin/aqbudgets.pl b/admin/aqbudgets.pl index 1a66c9c..0ca05f8 100755 --- a/admin/aqbudgets.pl +++ b/admin/aqbudgets.pl @@ -141,8 +141,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 b0c4344..73509ce 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt @@ -314,7 +314,7 @@ var MSG_PARENT_BENEATH_BUDGET = "- " + _("New budget-parent is beneath budget") [% END %] [% budget.budget_period_active %] Budget [% budget.budget_period_description %] [id=[% budget.budget_period_id %]][% UNLESS budget.budget_period_active %] (inactive)[% END %] - [% budget.budget_code_indent %] + [% budget.budget_code %] [% budget.budget_name %] [% IF budget.budget_parent_id %] -- 1.7.2.5