From 4042f59f037a5d350fd6d62ceb5b74b3534120c3 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 18 Apr 2016 10:37:18 +0100
Subject: [PATCH] Bug 15009: QA follow-up

This patch adds a test to check the unicity of auth cats, simplify
the GetBudgetAuthCats subroutine and make it return an arrayref of scalar
instead of an arrayref of hashref with only 1 key.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Budgets.pm                                      | 15 ++++++-----
 admin/aqplan.pl                                    |  2 +-
 .../prog/en/includes/budgets-admin-toolbar.inc     |  4 +--
 t/db_dependent/Budgets.t                           | 30 +++++++++++++++++-----
 4 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/C4/Budgets.pm b/C4/Budgets.pm
index fd32010..872c836 100644
--- a/C4/Budgets.pm
+++ b/C4/Budgets.pm
@@ -395,7 +395,14 @@ sub GetBudgetName {
     return $sth->fetchrow_array;
 }
 
-# -------------------------------------------------------------------
+=head2 GetBudgetAuthCats
+
+  my $auth_cats = &GetBudgetAuthCats($budget_period_id);
+
+Return the list of authcat for a given budget_period_id
+
+=cut
+
 sub GetBudgetAuthCats  {
     my ($budget_period_id) = shift;
     # now, populate the auth_cats_loop used in the budget planning button
@@ -408,11 +415,7 @@ sub GetBudgetAuthCats  {
         $authcats{$sort1_authcat}=1 if $sort1_authcat;
         $authcats{$sort2_authcat}=1 if $sort2_authcat;
     }
-    my @auth_cats_loop;
-    foreach (sort keys %authcats) {
-        push @auth_cats_loop,{ authcat => $_ };
-    }
-    return \@auth_cats_loop;
+    return [ sort keys %authcats ];
 }
 
 # -------------------------------------------------------------------
diff --git a/admin/aqplan.pl b/admin/aqplan.pl
index 20a8b00..2e887ce 100755
--- a/admin/aqplan.pl
+++ b/admin/aqplan.pl
@@ -127,7 +127,7 @@ while ( my ($category) = $sth->fetchrow_array ) {
 push( @category_list, 'MONTHS' );
 push( @category_list, 'ITEMTYPES' );
 push( @category_list, 'BRANCHES' );
-push( @category_list, $$_{'authcat'} ) foreach @$auth_cats_loop;
+push( @category_list, $_ ) foreach @$auth_cats_loop;
 
 #reorder the list
 @category_list = sort { $a cmp $b } @category_list;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/budgets-admin-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/budgets-admin-toolbar.inc
index 03c9b45..7172a8d 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/budgets-admin-toolbar.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/budgets-admin-toolbar.inc
@@ -48,7 +48,7 @@
                         [% ELSE %]
                         <li class="disabled">
                         [% END %]
-                        <a href="/cgi-bin/koha/admin/aqplan.pl?budget_period_id=[% budget_period_id %]&amp;authcat=[% auth_cats_loo.authcat %]">Plan by [% auth_cats_loo.authcat %]</a>
+                        <a href="/cgi-bin/koha/admin/aqplan.pl?budget_period_id=[% budget_period_id %]&amp;authcat=[% auth_cats_loo %]">Plan by [% auth_cats_loo %]</a>
                         </li>
                     [% END %]
                 </ul>
@@ -58,4 +58,4 @@
         <div class="btn-group"><a class="btn btn-small" id="newrootbudget" href="/cgi-bin/koha/admin/aqbudgetperiods.pl?op=add_form"><i class="fa fa-plus"></i> New budget</a></div>
     [% END %]
 
-</div>
\ No newline at end of file
+</div>
diff --git a/t/db_dependent/Budgets.t b/t/db_dependent/Budgets.t
index 269b147..675b990 100755
--- a/t/db_dependent/Budgets.t
+++ b/t/db_dependent/Budgets.t
@@ -1,5 +1,5 @@
 use Modern::Perl;
-use Test::More tests => 133;
+use Test::More tests => 134;
 
 BEGIN {
     use_ok('C4::Budgets')
@@ -625,11 +625,16 @@ is( C4::Budgets::GetBudget($budget_id21)->{budget_owner_id},
 
 # Test GetBudgetAuthCats
 
-my $budgetPeriodId = 1;
+my $budgetPeriodId = AddBudgetPeriod({
+    budget_period_startdate   => '2008-01-01',
+    budget_period_enddate     => '2008-12-31',
+    budget_period_description => 'just another budget',
+    budget_period_active      => 0,
+});
 
-my $bdgts = GetBudgets();
+$budgets = GetBudgets();
 my $i = 0;
-for my $budget ( @{$bdgts} )
+for my $budget ( @$budgets )
 {
     $budget->{sort1_authcat} = "sort1_authcat_$i";
     $budget->{sort2_authcat} = "sort2_authcat_$i";
@@ -643,7 +648,20 @@ my $authCat = GetBudgetAuthCats($budgetPeriodId);
 is( scalar @{$authCat}, $i * 2, "GetBudgetAuthCats returns only non-empty sorting categories (no empty authCat in db)" );
 
 $i = 0;
-for my $budget ( @{$bdgts} )
+for my $budget ( @$budgets )
+{
+    $budget->{sort1_authcat} = "sort_authcat_$i";
+    $budget->{sort2_authcat} = "sort_authcat_$i";
+    $budget->{budget_period_id} = $budgetPeriodId;
+    ModBudget( $budget );
+    $i++;
+}
+
+$authCat = GetBudgetAuthCats($budgetPeriodId);
+is( scalar @$authCat, scalar @$budgets, "GetBudgetAuthCats returns distinct authCat" );
+
+$i = 0;
+for my $budget ( @$budgets )
 {
     $budget->{sort1_authcat} = "sort1_authcat_$i";
     $budget->{sort2_authcat} = "";
@@ -657,7 +675,7 @@ $authCat = GetBudgetAuthCats($budgetPeriodId);
 is( scalar @{$authCat}, $i, "GetBudgetAuthCats returns only non-empty sorting categories (empty sort2_authcat on all records)" );
 
 $i = 0;
-for my $budget ( @{$bdgts} )
+for my $budget ( @$budgets )
 {
     $budget->{sort1_authcat} = "";
     $budget->{sort2_authcat} = "";
-- 
2.7.0