Bugzilla – Attachment 87009 Details for
Bug 10577
C4::Budgets::GetBudgetPeriod has inappropriate overloading of its behavior
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10577: Improve semantics of GetBudgetPeriod()
Bug-10577-Improve-semantics-of-GetBudgetPeriod.patch (text/plain), 4.68 KB, created by
Martin Renvoize (ashimema)
on 2019-03-26 11:40:54 UTC
(
hide
)
Description:
Bug 10577: Improve semantics of GetBudgetPeriod()
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2019-03-26 11:40:54 UTC
Size:
4.68 KB
patch
obsolete
>From 07cfa3e72f4c3c667b383d3c7f24c328f53c425b Mon Sep 17 00:00:00 2001 >From: Galen Charlton <gmc@esilibrary.com> >Date: Thu, 11 Jul 2013 16:41:26 +0000 >Subject: [PATCH] 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> >--- > C4/Budgets.pm | 28 +++++++--------------------- > t/db_dependent/Budgets.t | 8 +------- > 2 files changed, 8 insertions(+), 28 deletions(-) > >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index 9c53236f7d..493e2b7b1e 100644 >--- a/C4/Budgets.pm >+++ b/C4/Budgets.pm >@@ -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{ >diff --git a/t/db_dependent/Budgets.t b/t/db_dependent/Budgets.t >index db9787fb4e..339d5bd6e4 100755 >--- a/t/db_dependent/Budgets.t >+++ b/t/db_dependent/Budgets.t >@@ -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 : > # >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10577
:
19578
|
86563
|
86564
|
86565
|
86566
|
87009
|
87010
|
87080
|
87081
|
87082