@@ -, +, @@ - Koha doesn't require that there be only one active budget at a time, so the concept of "the" active budget doesn't make sense. - Having the single parameter be either an ID or a flag based on its value is poor function design. - No callers of GetBudgetPeriod() were actually using this modality. - wrapping the test in a transaction - counting budgets correctly - the budget hierarchy displays correctly - you can add and modify a budget --- C4/Budgets.pm | 28 +++++++--------------------- t/db_dependent/Budgets.t | 8 +------- 2 files changed, 8 insertions(+), 28 deletions(-) --- a/C4/Budgets.pm +++ a/C4/Budgets.pm @@ -438,27 +438,13 @@ sub GetBudgetPeriods { sub GetBudgetPeriod { my ($budget_period_id) = @_; my $dbh = C4::Context->dbh; - ## $total = number of records linked to the record that must be deleted - my $total = 0; - ## get information about the record that will be deleted - my $sth; - if ($budget_period_id) { - $sth = $dbh->prepare( qq| - SELECT * - FROM aqbudgetperiods - WHERE budget_period_id=? | - ); - $sth->execute($budget_period_id); - } else { # ACTIVE BUDGET - $sth = $dbh->prepare(qq| - SELECT * - FROM aqbudgetperiods - WHERE budget_period_active=1 | - ); - $sth->execute(); - } - my $data = $sth->fetchrow_hashref; - return $data; + my $sth = $dbh->prepare( qq| + SELECT * + FROM aqbudgetperiods + WHERE budget_period_id=? | + ); + $sth->execute($budget_period_id); + return $sth->fetchrow_hashref; } sub DelBudgetPeriod{ --- a/t/db_dependent/Budgets.t +++ a/t/db_dependent/Budgets.t @@ -1,6 +1,6 @@ #!/usr/bin/perl use Modern::Perl; -use Test::More tests => 146; +use Test::More tests => 143; BEGIN { use_ok('C4::Budgets') @@ -53,7 +53,6 @@ $bpid = AddBudgetPeriod({ budget_period_enddate => '2008-12-31', }); is( $bpid, undef, 'AddBugetPeriod without start date returns undef' ); -is( GetBudgetPeriod(0), undef ,'GetBudgetPeriod(0) returned undef : noactive BudgetPeriod' ); my $budgetperiods = GetBudgetPeriods(); is( @$budgetperiods, 0, 'GetBudgetPeriods returns the correct number of budget periods' ); @@ -71,8 +70,6 @@ is( $budgetperiod->{budget_period_startdate}, $my_budgetperiod->{budget_period_s is( $budgetperiod->{budget_period_enddate}, $my_budgetperiod->{budget_period_enddate}, 'AddBudgetPeriod stores the end date correctly' ); is( $budgetperiod->{budget_period_description}, $my_budgetperiod->{budget_period_description}, 'AddBudgetPeriod stores the description correctly' ); is( $budgetperiod->{budget_period_active}, $my_budgetperiod->{budget_period_active}, 'AddBudgetPeriod stores active correctly' ); -is( GetBudgetPeriod(0), undef ,'GetBudgetPeriod(0) returned undef : noactive BudgetPeriod' ); - $my_budgetperiod = { budget_period_startdate => '2009-01-01', @@ -91,8 +88,6 @@ is( $budgetperiod->{budget_period_startdate}, $my_budgetperiod->{budget_period_s is( $budgetperiod->{budget_period_enddate}, $my_budgetperiod->{budget_period_enddate}, 'ModBudgetPeriod updates the end date correctly' ); is( $budgetperiod->{budget_period_description}, $my_budgetperiod->{budget_period_description}, 'ModBudgetPeriod updates the description correctly' ); is( $budgetperiod->{budget_period_active}, $my_budgetperiod->{budget_period_active}, 'ModBudgetPeriod upates active correctly' ); -isnt( GetBudgetPeriod(0), undef, 'GetBugetPeriods functions correctly' ); - $budgetperiods = GetBudgetPeriods(); is( @$budgetperiods, 1, 'GetBudgetPeriods returns the correct number of budget periods' ); @@ -106,7 +101,6 @@ is( DelBudgetPeriod($bpid), 1, 'DelBudgetPeriod returns true' ); $budgetperiods = GetBudgetPeriods(); is( @$budgetperiods, 0, 'GetBudgetPeriods returns the correct number of budget periods' ); - # # Budget : # --