Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
Summary: C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-14 11:06 UTC by Nick Clemens
Modified: 2019-10-14 19:58 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets (1.22 KB, patch)
2017-07-14 11:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18941 - Unit tests (2.50 KB, patch)
2017-07-14 11:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18941 - Unit tests (2.50 KB, patch)
2017-07-19 18:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets (1.28 KB, patch)
2017-07-19 18:54 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18941 - Unit tests (2.57 KB, patch)
2017-08-03 02:12 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets (1.35 KB, patch)
2017-08-03 02:12 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18941 - Unit tests (2.63 KB, patch)
2017-08-10 19:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets (1.41 KB, patch)
2017-08-10 19:01 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2017-07-14 11:06:11 UTC
When closing a budget/creating new budgets at the end of the year fund codes are duplicated.

When importing orders from a staged file we retrieve the budget using only the fund code, this is problematic if we have multiple budgets.

When we really see the effect is a library that creates a new inactive budget for upcoming year, this budget's fund end up being implicitly selected by DB order.

We instead need to explicitly select active funds.
Comment 1 Nick Clemens 2017-07-14 11:29:59 UTC
Created attachment 65049 [details] [review]
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets

To test:
1 - Create an active budget
2 - Create an inactive budget
3 - Ensure they each have a fund with the same code
4 - Set MarcFieldsToOrder to get the budget_code from a marc field
5 - Stage a file using the duplicated code
6 - Add to a basket from the staged file
7 - Add the items
8 - Note funds are encumbered from the inactive budget
9 - Apply patch
10 - Repeat 5-8 with a new basket
11 - Note the active budget is now used
Comment 2 Nick Clemens 2017-07-14 11:31:23 UTC
Created attachment 65050 [details] [review]
Bug 18941 - Unit tests
Comment 3 Mark Tompsett 2017-07-14 18:54:52 UTC
I have two problems.
1) this adds a lot of fields because of the JOIN.
   -- should change the * to aqbudget.* at least
2) if I apply the unit tests but not the fix, I expected a failure.
   They still passed. How do I know the condition is fixed?

Marking Failed QA.
Comment 4 Nick Clemens 2017-07-19 18:54:56 UTC
Created attachment 65125 [details] [review]
Bug 18941 - Unit tests
Comment 5 Nick Clemens 2017-07-19 18:54:59 UTC
Created attachment 65126 [details] [review]
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets

To test:
1 - Create an active budget
2 - Create an inactive budget
3 - Ensure they each have a fund with the same code
4 - Set MarcFieldsToOrder to get the budget_code from a marc field
5 - Stage a file using the duplicated code
6 - Add to a basket from the staged file
7 - Add the items
8 - Note funds are encumbered from the inactive budget
9 - Apply patch
10 - Repeat 5-8 with a new basket
11 - Note the active budget is now used
Comment 6 Mark Tompsett 2017-08-03 02:12:29 UTC
Created attachment 65441 [details] [review]
[SIGNED-OFF] Bug 18941 - Unit tests

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 7 Mark Tompsett 2017-08-03 02:12:32 UTC
Created attachment 65442 [details] [review]
[SIGNED-OFF] Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets

To test:
1 - Create an active budget
2 - Create an inactive budget
3 - Ensure they each have a fund with the same code
4 - Set MarcFieldsToOrder to get the budget_code from a marc field
5 - Stage a file using the duplicated code
6 - Add to a basket from the staged file
7 - Add the items
8 - Note funds are encumbered from the inactive budget
9 - Apply patch
10 - Repeat 5-8 with a new basket
11 - Note the active budget is now used

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 8 Jonathan Druart 2017-08-10 19:00:58 UTC
Created attachment 65847 [details] [review]
Bug 18941 - Unit tests

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Jonathan Druart 2017-08-10 19:01:02 UTC
Created attachment 65848 [details] [review]
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets

To test:
1 - Create an active budget
2 - Create an inactive budget
3 - Ensure they each have a fund with the same code
4 - Set MarcFieldsToOrder to get the budget_code from a marc field
5 - Stage a file using the duplicated code
6 - Add to a basket from the staged file
7 - Add the items
8 - Note funds are encumbered from the inactive budget
9 - Apply patch
10 - Repeat 5-8 with a new basket
11 - Note the active budget is now used

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2017-08-10 19:30:44 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 11 Fridolin Somers 2017-09-29 09:52:27 UTC
Pushed to 17.05.x, will be in 17.05.05.
Comment 12 Katrin Fischer 2017-10-01 21:38:10 UTC
These patches have been pushed to 16.11.x and will be in 16.11.13.
Comment 13 Mason James 2017-11-23 22:07:17 UTC
Pushed to 16.05.x, for 16.05.18 release