Bug 18941

Summary: C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: AcquisitionsAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, mtj, mtompset
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
Bug 18941 - Unit tests
Bug 18941 - Unit tests
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
[SIGNED-OFF] Bug 18941 - Unit tests
[SIGNED-OFF] Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
Bug 18941 - Unit tests
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets

Description Nick Clemens (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 2017-07-19 18:54:56 UTC
Created attachment 65125 [details] [review]
Bug 18941 - Unit tests
Comment 5 Nick Clemens (kidclamp) 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