Bug 22759

Summary: Circulation rules for maxissueqty are applied per branch even for defaults
Product: Koha Reporter: Nick Clemens <nick>
Component: CirculationAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: blocker    
Priority: P1 - high CC: chris, gmcharlt, julian.maurice, katrin.fischer, kyle.m.hall, m.de.rooy, martin.renvoize, tomascohen, wizzyrea
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.05.00
Bug Depends on: 18925    
Bug Blocks: 21946    
Attachments: Bug 22759: Unit tests
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany
Bug 22759: Unit tests
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany
Bug 22759: Unit tests
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany

Description Nick Clemens 2019-04-23 15:02:00 UTC
To test:
1 - Find a title in the catalog, add 8 items, all of a single itemtype, 4 for branch A, four for branch B
2 - Remove any specific circ rules
3 - Set a default circ rule for the itemtype used above, limiting the max issue quantity to 3
4 - Find a patron and checkout the items from branch A
5 - You should be allowed ot checkout 3, but the fourth should trigger 'too many'
6 - Now checkout the items from branch B, again you can checkout 3 and not four
7 - You shouldn't be allowed 3 from each library :-(
Comment 1 Nick Clemens 2019-04-23 16:22:45 UTC
Created attachment 88530 [details] [review]
Bug 22759: Unit tests

To test:
1 - Apply only this patch
2 - prove -v t/db_dependent/Circulation/TooMany.t
3 - It fails
4 - Apply second path
5 - repeat
6 - All green!
Comment 2 Nick Clemens 2019-04-23 16:22:47 UTC
Created attachment 88531 [details] [review]
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany

The condition in TooMany applied branch limits only if the rule didn't have a branchcode, this was wrong

To test:
 1 - Find a title in the catalog, add 8 items, all of a single itemtype, 4 for branch A, four for branch B
 2 - Remove any specific circ rules
 3 - Set a default circ rule for the itemtype used above, limiting the max issue quantity to 3
 4 - Find a patron and checkout the items from branch A
 5 - You should be allowed to checkout 3, but the fourth should trigger 'too many'
 6 - Now checkout the items from branch B, again you can checkout 3 and not four
 7 - Apply this patch
 8 - Assuming you have 6 items out now, try one from either branch, you should trigger 'too many' and have 6
 9 - Return two from branch B and one from branch A
10 - Now you have three items issued total
11 - An item from either branch should be refused now
12 - Test with different values of 'CircControl' and confirm your expectations are met
Comment 3 Liz Rea 2019-04-23 16:40:42 UTC
Created attachment 88538 [details] [review]
Bug 22759: Unit tests

To test:
1 - Apply only this patch
2 - prove -v t/db_dependent/Circulation/TooMany.t
3 - It fails
4 - Apply second path
5 - repeat
6 - All green!

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
this is much much better.
Comment 4 Liz Rea 2019-04-23 16:40:52 UTC
Created attachment 88540 [details] [review]
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany

The condition in TooMany applied branch limits only if the rule didn't have a branchcode, this was wrong

To test:
 1 - Find a title in the catalog, add 8 items, all of a single itemtype, 4 for branch A, four for branch B
 2 - Remove any specific circ rules
 3 - Set a default circ rule for the itemtype used above, limiting the max issue quantity to 3
 4 - Find a patron and checkout the items from branch A
 5 - You should be allowed to checkout 3, but the fourth should trigger 'too many'
 6 - Now checkout the items from branch B, again you can checkout 3 and not four
 7 - Apply this patch
 8 - Assuming you have 6 items out now, try one from either branch, you should trigger 'too many' and have 6
 9 - Return two from branch B and one from branch A
10 - Now you have three items issued total
11 - An item from either branch should be refused now
12 - Test with different values of 'CircControl' and confirm your expectations are met

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 5 Katrin Fischer 2019-04-23 20:59:56 UTC
Created attachment 88565 [details] [review]
Bug 22759: Unit tests

To test:
1 - Apply only this patch
2 - prove -v t/db_dependent/Circulation/TooMany.t
3 - It fails
4 - Apply second path
5 - repeat
6 - All green!

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2019-04-23 21:00:00 UTC
Created attachment 88566 [details] [review]
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany

The condition in TooMany applied branch limits only if the rule didn't have a branchcode, this was wrong

To test:
 1 - Find a title in the catalog, add 8 items, all of a single itemtype, 4 for branch A, four for branch B
 2 - Remove any specific circ rules
 3 - Set a default circ rule for the itemtype used above, limiting the max issue quantity to 3
 4 - Find a patron and checkout the items from branch A
 5 - You should be allowed to checkout 3, but the fourth should trigger 'too many'
 6 - Now checkout the items from branch B, again you can checkout 3 and not four
 7 - Apply this patch
 8 - Assuming you have 6 items out now, try one from either branch, you should trigger 'too many' and have 6
 9 - Return two from branch B and one from branch A
10 - Now you have three items issued total
11 - An item from either branch should be refused now
12 - Test with different values of 'CircControl' and confirm your expectations are met

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Nick Clemens 2019-04-24 11:50:11 UTC
Awesome work all!

Pushed to master for 19.05
Comment 8 Martin Renvoize 2019-04-26 14:46:29 UTC
Bug in enhancement not in 18.11.x series.