Bug 10577: Improve semantics of GetBudgetPeriod()

Remove the option to pass zero to this function in
order to get "the" active budget.  This was a problem
in three ways:

- 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.

This patch also improves the DB-dependent tests for budgets by

- wrapping the test in a transaction
- counting budgets correctly

To test:

[1] Apply the patch.
[2] Verify that prove -v t/db_dependent/Budgets.t passes
[3] Verify in the staff interface that:
    - the budget hierarchy displays correctly
    - you can add and modify a budget

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Rescued-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Galen Charlton 2013-07-11 16:41:26 +00:00 committed by Nick Clemens
parent 9a321c128f
commit cc2a62bdee
2 changed files with 8 additions and 28 deletions

View File

@ -442,27 +442,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{

View File

@ -1,6 +1,6 @@
#!/usr/bin/perl
use Modern::Perl;
use Test::More tests => 147;
use Test::More tests => 144;
BEGIN {
use_ok('C4::Budgets')
@ -58,7 +58,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' );
@ -76,8 +75,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',
@ -96,8 +93,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' );
@ -111,7 +106,6 @@ is( DelBudgetPeriod($bpid), 1, 'DelBudgetPeriod returns true' );
$budgetperiods = GetBudgetPeriods();
is( @$budgetperiods, 0, 'GetBudgetPeriods returns the correct number of budget periods' );
#
# Budget :
#