Bug 22759 - Circulation rules for maxissueqty are applied per branch even for defaults
Summary: Circulation rules for maxissueqty are applied per branch even for defaults
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P1 - high blocker (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 18925
Blocks: 21946
  Show dependency treegraph
 
Reported: 2019-04-23 15:02 UTC by Nick Clemens
Modified: 2020-01-06 20:14 UTC (History)
9 users (show)

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


Attachments
Bug 22759: Unit tests (7.40 KB, patch)
2019-04-23 16:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany (2.25 KB, patch)
2019-04-23 16:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22759: Unit tests (7.44 KB, patch)
2019-04-23 16:40 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany (2.29 KB, patch)
2019-04-23 16:40 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22759: Unit tests (7.49 KB, patch)
2019-04-23 20:59 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22759: Fix wrong check of branch rule in C4::Circulation::TooMany (2.35 KB, patch)
2019-04-23 21:00 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.