Bug 12134

Summary: Members are restricted even if the debarment is ended
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: PatronsAssignee: Kyle M Hall (khall) <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: fridolin.somers, gmcharlt, gwilliams, koha.aixmarseille, kyle.m.hall, kyle, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 2720    
Bug Blocks: 12292, 13107, 13108    
Attachments: Bug 12134 - Members are restricted even if the debarment is ended
Bug 12134 - Members are restricted even if the debarment is ended
Bug 12134: Add unit tests to reflect the change in IsDebarred
[PASSED QA] Bug 12134 - Members are restricted even if the debarment is ended
[PASSED QA] Bug 12134: Add unit tests to reflect the change in IsDebarred
Bug 12134: (follow-up) Add unit tests to reflect the change in IsDebarred

Description Jonathan Druart 2014-04-24 15:21:59 UTC
Introduced by bug 2720.
Comment 1 Kyle M Hall (khall) 2014-04-24 15:24:25 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-04-24 15:38:40 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-04-24 15:38:45 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-04-24 15:42:15 UTC
Good candidate for 3.14.6!
Comment 5 Martin Renvoize (ashimema) 2014-04-25 08:54:10 UTC
Created attachment 27581 [details] [review]
[PASSED QA] Bug 12134 - Members are restricted even if the debarment is ended

Test Plan:
1) Modify borrowers.debarred for a borrower and set the date in the past
2) Try to check out an item to that patron
3) Note you cannot
4) Apply this patch
5) Repeat step 2
6) Note you can now check out items the patron

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2014-04-25 08:54:41 UTC
Created attachment 27582 [details] [review]
[PASSED QA] Bug 12134: Add unit tests to reflect the change in IsDebarred

Before Kyle's patch, the test failed.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2014-04-25 08:55:30 UTC
Passes tests, code makes sense and fixes an important bug.
Comment 8 Jonathan Druart 2014-04-25 10:49:34 UTC
Created attachment 27607 [details] [review]
Bug 12134: (follow-up) Add unit tests to reflect the change in IsDebarred

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 9 Galen Charlton 2014-04-28 17:15:20 UTC
Pushed to master, along with a follow-up that repairs one of the new tests.  Thanks, Jonathan and Kyle!

I think serious thought should be given to removing borrowers.debarred, by the way, in favor of consulting borrower_debarments directly.  borrowers.debarred was retained as a performance measure, but I'd like to see proof that querying borrower_debarments is too slow to be worth the risk of bugs like this one.
Comment 10 Fridolin Somers 2014-04-30 14:28:49 UTC
Patches pushed to 3.14.x, will be in 3.14.06