Attachment 50317 Details for Bug 15009 – Bug 15009: QA follow-up

[patch] Bug 15009: QA follow-up

Bug-15009-QA-follow-up.patch (text/plain), 5.30 KB, created by Jonathan Druart on 2016-04-18 09:41:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-04-18 09:41:06 UTC

Size: 5.30 KB

patch

obsolete

>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

Actions: View | Diff | Splinter Review

Attachments on bug 15009: 43391 | 43392 | 44262 | 49601 | 50293 | 50314 | 50315 | 50316 | 50317